Bug 480279 - Review Request: gnome-globalmenu - centralized menu bar
Review Request: gnome-globalmenu - centralized menu bar
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
: 503237 (view as bug list)
Depends On:
Blocks: RussianFedoraRemix
  Show dependency treegraph
 
Reported: 2009-01-16 01:22 EST by Feng Yu
Modified: 2010-05-20 11:37 EDT (History)
19 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-10-29 19:55:40 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Patch to incorporate all suggestions I made in comment #35 (7.06 KB, patch)
2009-05-29 16:45 EDT, Christoph Wickert
no flags Details | Diff

  None (edit)
Description Feng Yu 2009-01-16 01:22:34 EST
Spec URL: http://gnome2-globalmenu.googlecode.com/svn/tags/v0.7.2/gnome-globalmenu.spec
SRPM URL: http://gnome2-globalmenu.googlecode.com/files/gnome-globalmenu-0.7.2-1.fc10.src.rpm

Description: Global Menu is the centralized menu bar. Refer to http://gnome2-globalmenu.googlecode.com/ . This src rpm will build the gtk module, gnome-panel applet, and xfce panel plugin.
Comment 1 Feng Yu 2009-01-18 21:57:53 EST
Comparing to the old request-for-review package of global menu, this new release no longer requires a patched GTK.

After installation, add the 'Global Menu' applet to gnome panel, right click on it, goto references and click 'enable global menu for GTK applications'.
Comment 2 François Kooman 2009-02-14 06:36:12 EST
- build it in Mock to find all the missing build requirements, there are quite some I found out when trying to rebuild the SRPM without having all the required build requirements installed {libwnck-devel, libnotify-devel???, gnome-menus-devel, xfce4-panel-devel}
- run "rpmlint" on the spec, the srpm and the generated RPMs to find more problems.
- The BuildRoot doesn't follow Guidelines.
- The description lines are too long, wrap them

Cool there is a package for this! :)
Comment 3 Armin 2009-02-14 14:35:32 EST
ok, my turn to be picky:

SPEC:
- in line 2: put 2 % in the comment  (#%%define)
- yea, all descriptions are just too long.  Cut them to about 80 characters per line
- There should be a free line between %files. (xfce-panel and gnome-panel are squished together.

I will try to build it now and see what I get.
Comment 4 Armin 2009-02-14 14:58:30 EST
add %defattr to the three %files sections following %files common.

deps I got here:
- intltool
- libwnck-devel
- many more...

build it with mock: http://fedoraproject.org/wiki/Projects/Mock

and you have to add a version to your last changelog. (non of your changelogs have versions =S )
Comment 5 Feng Yu 2009-02-14 18:52:16 EST
I don't have sufficient harddisk for doing mock.

I'll post the updated srpm + spec files for 0.7.4 once it is released.

But would you take a look into this template file?

http://code.google.com/p/gnome2-globalmenu/source/browse/trunk/gnome-globalmenu.spec.in
Comment 6 Arkady L. Shane 2009-02-17 14:47:35 EST
Hello!

I have built package (ftp://mirror.yandex.ru/fedora/russianfedora/russianfedora/free/fedora/updates/10/SRPMS/gnome-globalmenu-0.7.3-3.fc10.src.rpm) for Fedora 10. It passes rpmlint tests for spec and binaries. You can take it.


And this 0.7.3 version does not compiles on fedora 9:

 gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -g -o .libs/GlobalMenu.PanelApplet GlobalMenu_PanelApplet-applet.o GlobalMenu_PanelApplet-main.o GlobalMenu_PanelApplet-gtkextra-gconfdialog.o GlobalMenu_PanelApplet-switcher.o GlobalMenu_PanelApplet-template.o GlobalMenu_PanelApplet-gnomemenuhelper.o GlobalMenu_PanelApplet-task-utils-c.o -pthread  -L/lib -lpanel-applet-2 -lgnomeui-2 -lSM -lICE -lbonoboui-2 -lgnomevfs-2 -lgconf-2 -lgnomecanvas-2 -lgnome-2 -lpopt -lart_lgpl_2 -lbonobo-2 -lbonobo-activation -lORBit-2 -lgthread-2.0 -lrt -lnotify -ldbus-glib-1 -ldbus-1 -lgnome-menu ../libgnomenu/.libs/libgnomenu.so -lwnck-1 -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0 
../libgnomenu/.libs/libgnomenu.so: undefined reference to `g_markup_parse_context_pop'
../libgnomenu/.libs/libgnomenu.so: undefined reference to `g_markup_parse_context_push'
collect2: ld returned 1 exit status
Comment 7 Bernie Innocenti 2009-02-18 07:00:05 EST
I needed two patches in order to build on rawhide.

I have submitted the first one here:
  http://code.google.com/p/gnome2-globalmenu/issues/detail?id=348

The second one is too much of a kludge.  It seems that  libxfce4panel-1.0.pc does
no longer provide the externalplugindir variable.  We should check with upstream.


--- configure.ac.orig	2009-02-17 18:51:23.913429581 +0100
+++ configure.ac	2009-02-17 18:49:26.570430365 +0100
@@ -111,7 +111,8 @@ AS_IF([test "x$with_gnome_panel" != xno]
 	[PKG_CHECK_MODULES(GNOME_PANEL, 
 		[	libpanelapplet-2.0 >= $PANEL_REQUIRED, 
 			libnotify >= $NOTIFY_REQUIRED,
-			libgnome-menu >= $GMENU_REQUIRED
+			libgnome-menu >= $GMENU_REQUIRED,
+			libgnomeui-2.0
 		],
 		[
 		with_gnome_panel=true;

--- configure.ac.orig	2009-02-17 19:10:03.997555188 +0100
+++ configure.ac	2009-02-17 19:11:23.179429466 +0100
@@ -141,8 +141,8 @@ AS_IF([test "x$with_xfce4_panel" != xno]
 		with_xfce4_panel=true;
 		AC_SUBST(XFCE4_PANEL_CFLAGS)
 		AC_SUBST(XFCE4_PANEL_LIBS)
-		XFCE4_PANEL_PLUGIN_DIR="$($PKG_CONFIG --variable=externalplugindir libxfce4panel-1.0)"
-		XFCE4_PANEL_DESKTOP_DIR="$($PKG_CONFIG --variable=desktopdatadir libxfce4panel-1.0)"
+		XFCE4_PANEL_PLUGIN_DIR=/usr/libexec/xfce4/panel-plugins
+		XFCE4_PANEL_DESKTOP_DIR=/usr/share/xfce4/panel-plugins
 		AC_SUBST(XFCE4_PANEL_PLUGIN_DIR)
 		AC_SUBST(XFCE4_PANEL_DESKTOP_DIR)
 		],
Comment 8 Alexey Torkhov 2009-03-08 03:22:17 EDT
I see 0.7.4 was released, could you update the package?
Comment 9 Feng Yu 2009-03-08 03:30:15 EDT
The tarball is buildable with rpmbuild -ta -- I am uploading the files.
Comment 11 Alexey Torkhov 2009-03-08 04:21:48 EDT
Do you need a sponsor? If so, FE-NEEDSPONSOR bug should be blocked.

- Package fails to build in mock. Need to add intltool, libwnck-devel, libXres-devel to BuildRequires.

- Use %{?rhel} macro to check rhel version:
https://fedoraproject.org/wiki/Packaging/DistTag#Conditionals

- rpmlint is saying enormous number of errors and warnings that should be fixed:
gnome-globalmenu.src: W: no-version-in-last-changelog
gnome-globalmenu-common.x86_64: E: postin-without-ldconfig /usr/lib64/libgnomenu-0.7.4.so.2.0.0
gnome-globalmenu-common.x86_64: E: library-without-ldconfig-postun /usr/lib64/libgnomenu-0.7.4.so.2.0.0
gnome-globalmenu-common.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/libgnomenu.pc
gnome-globalmenu-common.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/gnome-globalmenu.schemas
gnome-globalmenu-common.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libgnomenu.so
gnome-globalmenu-common.x86_64: E: description-line-too-long This package contains shared data and libraries of various Global Menu packages.
gnome-globalmenu-common.x86_64: W: no-version-in-last-changelog
gnome-globalmenu-debuginfo.x86_64: W: no-version-in-last-changelog
gnome-globalmenu-devel.x86_64: W: no-documentation
gnome-globalmenu-devel.x86_64: W: no-version-in-last-changelog
gnome-globalmenu-gnome-panel.x86_64: W: no-documentation
gnome-globalmenu-gnome-panel.x86_64: E: explicit-lib-dependency libnotify
gnome-globalmenu-gnome-panel.x86_64: E: explicit-lib-dependency libwnck
gnome-globalmenu-gnome-panel.x86_64: W: no-version-in-last-changelog
gnome-globalmenu-gtkmodule.x86_64: W: no-documentation
gnome-globalmenu-gtkmodule.x86_64: W: no-version-in-last-changelog
gnome-globalmenu-xfce-panel.x86_64: W: no-documentation
gnome-globalmenu-xfce-panel.x86_64: E: explicit-lib-dependency libwnck
gnome-globalmenu-xfce-panel.x86_64: W: no-version-in-last-changelog
7 packages and 0 specfiles checked; 6 errors, 14 warnings.

In particular:
- All subpackages with shared libraries should have ldconfig calls:
https://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries

- Drop all explicit library dependencies in favour of rpm automatic dependency finder.

- Description lines should be less 80 symbols.

- Add version to at least last changelog entry.


- Spec template gnome-globalmenu.spec.in will generate bad name for svn snapshots like "0.7.4-1234-1.fc10". It should be like "0.7.4-0.1.1234svn.fc10" instead:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
Comment 12 Feng Yu 2009-03-08 13:47:09 EDT
gnome-globalmenu-common.x86_64: W: non-conffile-in-etc
/etc/gconf/schemas/gnome-globalmenu.schemas

How is the schema file supposed to be installed?


What does this mean?
gnome-globalmenu-gtkmodule.x86_64: W: no-documentation

New file has been uploaded to the same location. How did you invoke rpmlint?

----------
[rainwoodman@localhost gnomenu]$ !rpmlint
rpmlint gnome-globalmenu.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
-----------
Comment 13 Alexey Torkhov 2009-03-08 14:15:14 EDT
(In reply to comment #12)
> gnome-globalmenu-common.x86_64: W: non-conffile-in-etc
> /etc/gconf/schemas/gnome-globalmenu.schemas
> 
> How is the schema file supposed to be installed?

It should be marked as %config(noreplace) - that is what warning says.

> What does this mean?
> gnome-globalmenu-gtkmodule.x86_64: W: no-documentation

Add %doc line with AUTHORS and COPYING to -common subpackage. After that, this warning on other packages will be safe to ignore

You can also get brief explanations for rpmlint errors with "rpmlint -I no-documentation".

> How did you invoke rpmlint?

It is invoked on built rpms like "rpmlint gnome-globalmenu*.rpm".
Comment 14 Christoph Wickert 2009-03-08 14:59:19 EDT
(In reply to comment #13)
> 
> It should be marked as %config(noreplace) - that is what warning says.

No it shouldn't, as gconf files are no config files and not supposed to be edited by users. Just ignore the warning.

Some more comments:
- please add a blank line between new changelog entries for legibility
- devel package needs "Requires: pkgconfig" because it puts files into %{_libdir}/pkgconfig which is owned by pkgconfig
- the files section of the devel package could be stripped down with wildcards:
  %{_includedir}/libgnomenu/*.h instead of listing all files one by one
- %{_includedir}/libgnomenu/ is unowned and will be left behind after uninstalling the devel package. So in addition to the above you need 
  %dir %{_includedir}/libgnomenu/
- %{_libdir}/libgnomenu.so belongs into the devel package and not into common, see http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages
- %{_libdir}/gtk-2.0/modules/libglobalmenu-gnome.so is an exception from this rule, so it's ok
- requirements for subpackages should be fuly versioned, see http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage
  so the gnome-panel package needs: Requires: gnome-globalmenu-common = %{version}-%{release}
- Requires: gtk2 is not necessary as rpm will pick up dependencies on libs automatically and other packages (xfce4-panel/gnome-panel) already requires it.
- Requires: gnome-menus is not necessary ether, because it is already required be gnome-panel
- You are running ldconfig in %post and %postun of the common package, so it also needs
  Requires(post): ldconfig
  Requires(postun): ldconfig
- Requires(pre): GConf2
  Requires(post): GConf2
  Requires(preun): GConf2 
  also belong to the common package and not to the empty base package
- Does it really make sense to package the gtkmodule separately?
- I wonder it the gnome-globalmenu-gnome-panel should be named gnome-applet-globalmenu instead, because this would follow the naming guidelines for panel applets.
- Same for gnome-globalmenu-xfce4-panel: Call it xfce4-globalmenu-plugin instead?
Comment 15 Feng Yu 2009-03-08 23:20:14 EDT
All changes done.

I also merged the -common backage to the base package.

New files were uploaded to 
http://rainwoodman.dreamhosters.com/fedora-review/gnome-globalmenu-0.7.4-4.fc10.src.rpm

http://rainwoodman.dreamhosters.com/fedora-review/gnome-globalmenu.spec  

[rainwoodman@localhost gnomenu]$ rpmlint ~/rpmbuild/RPMS/i386/*0.7.4-4*
gnome-applet-globalmenu.i386: W: no-documentation
gnome-globalmenu.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnome-globalmenu.schemas
gnome-globalmenu-devel.i386: W: no-documentation
xfce4-globalmenu-plugin.i386: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 4 warnings.

Does it build with mock?
Comment 16 Sergio Pascual 2009-03-13 14:17:41 EDT
Yes, it does, in devel and F-10

gnome-applet-globalmenu.x86_64: W: no-documentation
gnome-globalmenu.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/gnome-globalmenu.schemas
gnome-globalmenu-devel.x86_64: W: no-documentation
xfce4-globalmenu-plugin.x86_64: W: no-documentation


It works more or less, but does weird things in my dual head configuration (some other gtk standard stock does weird things too, such as the notification area). It wolud be great to be able to apply the globalmenu only to a given screen.
Comment 17 Christoph Wickert 2009-03-17 20:35:30 EDT
(In reply to comment #15)
>
> Does it build with mock?  

Why not test it yourself ;)


REVIEW FOR 
22be0f2045eb66f11a3b3251c07e72ea  gnome-globalmenu-0.7.4-4.fc10.src.rpm


OK - MUST: rpmlint must be run on every package. The output should be posted in the review.
  $ rpmlint /var/lib/mock/fedora-rawhide-i386/result/*.rpm
  gnome-applet-globalmenu.i386: W: no-documentation
  gnome-globalmenu.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnome-globalmenu.schemas
  gnome-globalmenu-devel.i386: W: no-documentation
  xfce4-globalmenu-plugin.i386: W: no-documentation
  6 packages and 0 specfiles checked; 0 errors, 4 warnings.
All these errors are safe to ignore, because docs are included in the base package and gconf schemas are no config files.

OK - MUST: The package is named according to the Package Naming Guidelines.
OK - MUST: The spec file name matches the base package %{name}, in the format %{name}.spec.
OK - MUST: The package meets the Packaging Guidelines.
OK - MUST: The package is licensed with a Fedora approved license and meets the Licensing Guidelines: GPLv3
FAIL - MUST: The License field in the package spec file does not match the actual license.
OK - MUST: The license file from the source package is included in %doc.
OK - MUST: The spec file is in American English.
OK - MUST: The spec file for the package is legible.
FAIL - MUST: The sources used to build the package matches the upstream source by MD5: The tarball inside the rpm has b9d861827b7ae8f4750df3dd450e0be6 while the one from Google has 9becbaff8deda8acf46b4f89863c8aa3
OK - MUST: The package successfully compiles and builds into binary rpms on i386
N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
OK - MUST: All build dependencies are listed in BuildRequires.
OK - MUST: The spec file handles locales properly with the %find_lang macro.
OK - MUST: The binary RPM package stores shared library files (not just symlinks) in any of the dynamic linker's default paths and therefore calls ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
FAIL - MUST: The package owns all directories that it creates: %{_datadir}/doc/gnome-globalmenu/ is unowned and will be left behind after uninstalling the package
OK - MUST: The package does not contain any duplicate files in the %files listing.
OK - MUST: Permissions on files are set properly. Every %files section includes a %defattr(...) line.
OK - MUST: The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
OK - MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines.
OK - MUST: The package contains code.
N/A - MUST: Large documentation files should go in a -doc subpackage.
OK - MUST: Files included as %doc do not affect the runtime of the application.
OK - MUST: Header files must be in a -devel package.
N/A - MUST: Static libraries must be in a -static package.
OK - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
OK - MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. Note: The gtk modules are an exception here.
OK - MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: The packages does not own files or directories already owned by other packages.
OK - MUST: At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
OK - MUST: All filenames in rpm packages are valid UTF-8.

SHOULD Items:
N/A - SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: The the package builds in mock.
OK - SHOULD: The package should compile and build into binary rpms on all supported architectures.
FAIL - SHOULD: The package does not function as described. The Xfce4-panel plugin does not work at all here and dual head seems a little buggy.
OK - SHOULD: If scriptlets are used, those scriptlets must be sane.
OK - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
OK - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg.
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Issues:
- License is unclear: Spec says GPLv2+ while package includes GPLv3 as COPYING.
Aditionaly the tarball includes two files GPLv2 and GPLv3. If a package has parts that are licensed under different licenses we usually list them all in the spec ("GPLv2 and GPLv3"). If you summarize this as "GPLv2+", you should include the GPLv2+ in the package and you need to be aware of the fact that it also means means "any later version", including GPLv4. 

- Download the sources again to make sure they really match. Also take care of the timestamps, see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

- Docs are incomplete: Include Authors and the correct license(s), but do not include ChangeLog and NEWS as long as they are empty

- Docs are installed in the wrong dir:
%{_datadir}/doc/%{name}/ instead of 
%{_datadir}/doc/%{name}-version/
Just remove %{_datadir}/doc/%{name}/ completely during %install and let let rpm take care of installing the docs:
  ...
  rm -f $RPM_BUILD_ROOT/%{_libdir}/libgnomenu.la
  rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/%{name}-version/
  ...
  %files -f %{name}.lang
  %defattr(-,root,root,-)
  %doc AUTHORS COPYING GPLv2 GPLv3 README

This also fixes the unowned dir mentioned above

- remove the "# comment" leftover

- What exactly do you want to achieve with the "%if 0%{?rhel5}" statements? Do you want to build the xfce4-panel plugin only on Fedora and RHEL4?

- Does the xfce4-panel plugin work for you? For me it does nothing (I'm already on Xfce 4.6, but the panel api has not changed from 4.4)
Comment 18 Feng Yu 2009-03-17 20:57:50 EDT
Thanks for reviewing.

> 
> Issues:
> - License is unclear: Spec says GPLv2+ while package includes GPLv3 as COPYING.
> Aditionaly the tarball includes two files GPLv2 and GPLv3. If a package has
> parts that are licensed under different licenses we usually list them all in
> the spec ("GPLv2 and GPLv3"). If you summarize this as "GPLv2+", you should
> include the GPLv2+ in the package and you need to be aware of the fact that it
> also means means "any later version", including GPLv4. 
It is a GPLv2. The GPLv3 file is not used for licensing. The COPYING file contains wrong content. 
> 
> - Download the sources again to make sure they really match. Also take care of
> the timestamps, see
> https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
> 
> - Docs are incomplete: Include Authors and the correct license(s), but do not
> include ChangeLog and NEWS as long as they are empty
What about adding links to the real ChangeLog and NEWS on the web in these empty files?

> 
> - Docs are installed in the wrong dir:
> %{_datadir}/doc/%{name}/ instead of 
> %{_datadir}/doc/%{name}-version/
> Just remove %{_datadir}/doc/%{name}/ completely during %install and let let rpm
> take care of installing the docs:
>   ...
>   rm -f $RPM_BUILD_ROOT/%{_libdir}/libgnomenu.la
>   rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/%{name}-version/
>   ...
>   %files -f %{name}.lang
>   %defattr(-,root,root,-)
>   %doc AUTHORS COPYING GPLv2 GPLv3 README
> 
> This also fixes the unowned dir mentioned above

> 
> - remove the "# comment" leftover
> 
> - What exactly do you want to achieve with the "%if 0%{?rhel5}" statements? Do
> you want to build the xfce4-panel plugin only on Fedora and RHEL4?
> 
Only on Fedora.
the xfce4-panel plugin can't build on RHEL5, because the dependency are not satisfied.


> - Does the xfce4-panel plugin work for you? For me it does nothing (I'm already
> on Xfce 4.6, but the panel api has not changed from 4.4)  
There are post installation configuration steps. You have to export GTK_MODULES=globalmenu-gnome. Where should this be mentioned?
Comment 19 Christoph Wickert 2009-03-19 16:37:57 EDT
Include the correct license. Ether cat GPLv2 > COPYING during %prep or simply install the GPLv2 file as is.

(In reply to comment #18)

> It is a GPLv2. The GPLv3 file is not used for licensing. The COPYING file
> contains wrong content. 

> What about adding links to the real ChangeLog and NEWS on the web in these
> empty files?

Ok for me, but in a long term proper documentation inside the package would be better. BTW: README contains a wrong URL: http:///gnome2-globalmenu.googlecode.com (note the tripple slashes)

> Only on Fedora.
> the xfce4-panel plugin can't build on RHEL5, because the dependency are not
> satisfied.

If it cant be built on RHEL5 I guess it wont build in RHEL4 ether, right? 

> There are post installation configuration steps. You have to export
> GTK_MODULES=globalmenu-gnome. Where should this be mentioned?  

Ether in the description of xfce4-globalmenu-plugin or in a README.Xfce. I suggest to add a hint to the description that points to README.xfce:

  %description -n	xfce4-globalmenu-plugin
  The XFCE panel applet of Global Menu is a representation of Global Menu 
  with GTK widgets. The applet can be inserted to the default top panel 
  to provide access to the Global Menu of the applications. 

(BTW: There no longer is default top panel in Xfce 4.6, just a single panel at the bottom)

  To enable the xfce4-globalmenu-plugin you need to follow the instructions
  outlined in %{_docdir}/%{name}-%{version}/README.xfce.
Comment 20 Christoph Wickert 2009-04-07 07:40:53 EDT
(In reply to comment #18)

> the xfce4-panel plugin can't build on RHEL5, because the dependency are not
> satisfied.

As it can't be built in RHEL4 ether, you need to use %{?rhel} instead of %{?rhel5}

Can you please make the suggested changes? We are almost done and I'd like to see this package in F11.
Comment 21 Feng Yu 2009-04-07 21:15:16 EDT
Thanks Christ.

I've made a snapshot package (20080407svn2482) with the updated spec file at

http://rainwoodman.dreamhosters.com/fedora-review/

With the changes.
Comment 22 Christoph Wickert 2009-04-07 21:24:50 EDT
Why a snapshot? Does it have any advantage over the stable version? IMO snapshots should be avoided in the stable Fedora releases.

Any if you use a snapshot, the package version is wrong. Please read
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
Comment 23 Christoph Wickert 2009-04-07 21:36:08 EDT
The tag license is still not correct, it's still GPLv3, but you said it's GPL2. Is it GPLv2 only or GPLv2 or later?

Also libpanelapplet-2.0.vapi is LGPL 2.1. This would result in "LGPLv2 and GPLv2" as license tag.
Comment 24 Christoph Wickert 2009-04-07 21:41:20 EDT
(In reply to comment #21)
> Thanks Christ.

I'm not Jesus, just Christoph is ok ;) Please also change that in %changelog, because "suggested by Chirst" sounds a little funny.

Regarding the snapshot: It's ok for me, I trust you as upstream. Do you think 0.7.5 will be released before F11 goes public?
Comment 25 Feng Yu 2009-04-07 23:03:14 EDT
COPYING now contains a copy of GPLv2. 
The spec file now states LGPL2 + GPL2.
new src.rpm is 20080407svn2487

I came up with the new snapshot name from that page.
Post-release is %{X}.YYYYMMDDsvn####
Pre-release is 0.%{X}.YYYYMMDDsvn####

Since I am still using 0.7.4 as package version this snapshot should be a post release? Did I get anything wrong?

When is the release date of F11? The blockers for a 0.7.5 release are already cleared.
Comment 26 Christoph Wickert 2009-04-08 03:03:52 EDT
Well, this is not post-release of 0.7.4 but actually a pre-relaease of 0.7.5. So the correct naming for Fedora would be:
0.7.5-0.1.20080407svn2487
A new release based on the same snapshot would become 
0.7.5-0.2.20080407svn2487
and so on, the 0 as release number must be kept to indicate it's a pre-release. 0.7.5 final would become a normal 0.7.5-1 again.

Fedora 11 will be released on May 26th, see
https://fedoraproject.org/wiki/Schedule
Comment 27 Christoph Wickert 2009-04-17 14:42:47 EDT
Ping?
Comment 29 Feng Yu 2009-04-19 04:09:58 EDT
Updated to svn2511; which is pretty close to 0.7.5.
Comment 30 Feng Yu 2009-04-23 22:15:24 EDT
0.7.5-1 srpm and the corresponding spec files are available at

http://rainwoodman.dreamhosters.com/fedora-review/

Please take a review.
Comment 31 Christoph Wickert 2009-04-25 00:02:54 EDT
OK - License OK
OK - License tag matches
OK - Source0 maches by md5sum bafbad33ad43ba401d228dbaa10f26a0
OK - Package works fine

$ rpmlint /var/lib/mock/fedora-rawhide-i386/result/gnome-*
gnome-applet-globalmenu.i386: W: no-documentation
gnome-globalmenu.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnome-globalmenu.schemas
gnome-globalmenu.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 1)
gnome-globalmenu-devel.i386: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 4 warnings.

Please fix the mixed-use ... warning.

directory %{_datadir}/doc/gnome-globalmenu/ is still unowned. I told you how to install the docs in comment # 17

Do not install generic INSTALL files

Is it possible to move 
%{_sysconfdir}/gconf/schemas/gnome-globalmenu.schemas
into the gnome-applet-globalmenu package? I guess gconf is not really needed when runing in xfce.

Please fix the changelog, one blank between entries.
Comment 32 Christoph Wickert 2009-05-03 07:15:11 EDT
Ping?!
Comment 33 Feng Yu 2009-05-06 18:59:54 EDT
Updated to 0.7.5-2

http://rainwoodman.dreamhosters.com/fedora-review/


It might be more prudent to move the schemas file in the next release
Comment 34 Christoph Wickert 2009-05-29 15:28:03 EDT
*** Bug 503237 has been marked as a duplicate of this bug. ***
Comment 35 Christoph Wickert 2009-05-29 16:39:48 EDT
Sorry it took so long, but I'm still not happy with the spec.

It was ok to drop the gtk-module package,  but IMO the -common package should remain to avoid confusion. Otherwise people install gnome-globalmenu and wonder why the don't have a panel applet.

If you renamed the base package to gnome-globalmenu-common you could make the gnome-applet-globalmenu provide gnome-applet-globalmenu for easy installation with yum.

I have moved the gconf schema to the gnome-applet package and the xfce plugin still works nice. So why wait for a new release to move the file?

Use %global instead of %define, see
https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

Requires(pre|post|preun): GConf2 and the GConf scriptlets are in the base package, but they belong to the gnome-applet package.

Only ldconfig will remain in the base package. You could replace it with "%post -p /sbin/ldconfig" and drop the Requires(post|postun) for ldconfig, rpm will take care of this. Of course it needs to be 
"%post -p common /sbin/ldconfig" if you renamed the base package.

The conditionals are bad. You only want to build the xfce plugin in Fedora. So you better use "%if 0%{?fedora}"

configure can be more simplified to 
%configure --disable-schemas-install \
		--disable-static \
		--disable-tests \
		--with-gnome-panel \
		--docdir=%{pkgdocdir} \
	%if 0%{?fedora}
		--with-xfce4-panel
	%else
		--without-xfce4-panel
	%endif
Comment 36 Christoph Wickert 2009-05-29 16:45:03 EDT
Created attachment 345959 [details]
Patch to incorporate all suggestions I made in comment #35

I have also reworked the descriptions a little so that the correct texts for each subpackage gets displayed in the package-manager.

Take a look at the patch to see what I have in mind. You don't need to apply it to your spec, but take what you like.
Comment 37 Feng Yu 2009-06-27 00:32:04 EDT
Thanks. The patch is applied and the files at

http://rainwoodman.dreamhosters.com/fedora-review/

are updated to 0.7.6-1.
Comment 38 Christoph Wickert 2009-07-22 19:48:34 EDT
OMG, sorry for the enormous delay.

No need to fully apply the patch, it's your package after all. And it's finally

APPROVED


Do you plan to maintain other packages in Fedora or do you just want to maintain your package as the upstream developer? If so, I will sponsor you right away, because I really think that we must enable upstream participating in Fedora more easily. I suggest we co-maintain the package then, so I can help you out.

If you would like do get involved in Fedora deeper and maintain more packages, I would like you to participate in other reviews before I sponsor you. See http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored#Reviewing_Packages
for how this works.
Comment 39 Feng Yu 2009-08-03 23:00:09 EDT
OMG! The notification was buried in the sea of junk mails.

I am happy if we co-maintain the package.  I can't guarantee if I will be able to maintain other packages in Fedora -- I am getting busier next semester. In any cases I am interested in participating the process. Let's begin with this one.

What should I do if there for a 0.7.6.1(or 0.7.6a) update? I am planning to replace the hasty 0.7.6 very soon. It wasn't a good tarball.
Comment 40 Christoph Wickert 2009-08-05 20:53:45 EDT
Ok, fine with co-maintainership. If you want to get involved deeper in Fedora, you can always do.

You can update the package before import or afterwards. The latter has the advantage that the process is transparent for everyone because we all can see you commits. The update doesn't require changes to the spec, does it?

Please apply for a Fedora account in FAS at https://admin.fedoraproject.org
If you already have an account, apply for the packager group there, so I can sponsor you. For more info see
https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account

After that it's time for the cvs admin procedure:
https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Comment 41 Christoph Wickert 2009-09-09 10:58:00 EDT
Ping! Please take action so I can sponsor you.
Comment 42 Bernie Innocenti 2009-09-09 14:25:14 EDT
The latest packate (0.7.6) is missing a "BuildRequires vala".

Also, 0.7.7-beta has just been released by upstream (after 3 months from 0.7.6).  Please, consider updating the package to this version unless it's buggy.
Comment 43 Christoph Wickert 2009-09-09 15:48:40 EDT
Bug reporter == upstream, so he knows best.

Feng Yu, what do you think?
Comment 44 Feng Yu 2009-09-09 15:58:36 EDT
I would like directly update to the 0.7.7 release which is coming very soon.
Comment 45 Christoph Wickert 2009-09-09 16:10:22 EDT
Ok, meanwhile get yourself a Fedora account, tell me it's name and apply for the packager group in the account system:
https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
Comment 46 Feng Yu 2009-09-09 16:15:06 EDT
Did that a minute ago, account name: fengyu.
Comment 47 Bernie Innocenti 2009-09-11 01:20:17 EDT
FYI, installing the 0.7.6 package makes Firefox crash in Rawhide.
Comment 48 Feng Yu 2009-09-13 20:17:43 EDT
Please let me know what to do after getting the cvs access.
Comment 49 Bernie Innocenti 2009-09-13 22:07:43 EDT
(In reply to comment #48)
> Please let me know what to do after getting the cvs access.  

You should already have CVS access.  Now you should request the
creation of the CVS module for your package.  Instructions are
here:

  https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

This is how I'd fill in the request:

New Package CVS Request
=======================
Package Name: gnome-globalmenu
Short Description: Centralized menu bar for GNOME
Owners: fengyu
Branches: 
InitialCC: bernie

(remember to set the CVS request flag)
Comment 50 Christoph Wickert 2009-09-15 20:51:55 EDT
Ping?
Comment 51 jmccann 2009-09-21 11:17:18 EDT
Has there been any progress on this?
Comment 52 Feng Yu 2009-10-04 20:39:53 EDT
New Package CVS Request
=======================
Package Name: gnome-globalmenu
Short Description: Centralized menu bar for GNOME
Owners: fengyu
Branches: 
InitialCC: bernie
Comment 53 Kevin Fenzi 2009-10-06 13:41:53 EDT
This request seems to have already been processed. 
cvs done.
Comment 54 Rahul Sundaram 2009-10-26 10:14:41 EDT
Before building this, you might want to move the latest svn snapshot since it fixes a important issue with this applet. Details at 

https://bugzilla.redhat.com/show_bug.cgi?id=530590
Comment 55 Christoph Wickert 2009-10-26 10:33:31 EDT
Feng is the upstream developer and AFAIK he wants to do another stable release.

Feng, if you need help with importing this into CVS, please let me know.
Comment 56 Feng Yu 2009-10-26 20:03:54 EDT
Yes I need help. I need help. I don't have time to learn through the documents, being stressed out by the heavy wprk load after transfering to a different school.

I guess the cvs permissions are already setup. It would be nice of you if we can meet on IM and work this out together. I am available on gtalk(@gmail)/msn(@hotmail). I guess it actually won't take long if one knows how the process works.

0.7.8 will be delayed due to a severe regression found in the beta. I would go with 0.7.7_1, with some important patches backported from 0.7.8, if needed..
Comment 57 Bernie Innocenti 2009-10-26 21:06:21 EDT
(In reply to comment #56) 
> I guess the cvs permissions are already setup. It would be nice of you if we
> can meet on IM and work this out together. I am available on
> gtalk(@gmail)/msn(@hotmail). I guess it actually won't take long if one knows
> how the process works.

I added you as a buddy in Jabber (GTalk), but you haven't approved me yet. Usually Fedora developers meet on irc.freenode.net in channel #fedora-devel.

You can import your package in 3 easy steps:

  cvs -d bernie@cvs.fedoraproject.org:/cvs/pkgs co gnome-globalmenu
  cd gnome-globalmenu/devel
  ../common/cvs-import.sh path/to/srpm/file

Then tag (I can't remember if the cvs-import script does it already, but it shouldn't hurt to repeat it):

  make tag

Then build it in Koji:

  cvs update
  make build

You're done!
Comment 58 Christoph Wickert 2009-10-26 21:20:29 EDT
(In reply to comment #57)
> I added you as a buddy in Jabber (GTalk), but you haven't approved me yet.

Me too. I'm your sponsor, so don't hesitate to contact me if you have questions or problems.

> Then tag (I can't remember if the cvs-import script does it already, but it
> shouldn't hurt to repeat it):

Yes, this is already done by cvs-import.

>   make tag
> 
> Then build it in Koji:
> 
>   cvs update
>   make build

After the rawhide/devel build succeeds, you build for F-11 and F12:
cp .cvsignore gnome-globalmenu.spec sources ../F-12
cd ../F-12
cvs add /gnome-globalmenu.spec
cvs commit -m "initial build for this branch"
make tag build

Repeat the same for F-11
Comment 59 Feng Yu 2009-10-26 22:21:01 EDT
Package Change Request
======================
Package Name: gnome-globalmenu
New Branches: F-11 F-12
Owners: fengyu
Comment 60 Kevin Fenzi 2009-10-28 20:09:11 EDT
cvs done.
Comment 61 Fedora Update System 2009-10-28 21:02:40 EDT
gnome-globalmenu-0.7.7_1-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/gnome-globalmenu-0.7.7_1-1.fc11
Comment 62 Christoph Wickert 2010-05-20 11:37:16 EDT
Remove alias and unblock NEEDSPONSOR

Note You need to log in before you can comment on or make changes to this bug.