Bug 448292 - Review Request: fbpanel - a lightweight X11 desktop panel
Review Request: fbpanel - a lightweight X11 desktop panel
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-25 10:24 EDT by Stefan Posdzich
Modified: 2012-04-21 16:57 EDT (History)
6 users (show)

See Also:
Fixed In Version: fbpanel-6.1-4.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-06-20 15:12:04 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
fbpanel-4.12-libdir.patch (3.62 KB, patch)
2008-06-15 15:14 EDT, Robert Scheck
no flags Details | Diff

  None (edit)
Description Stefan Posdzich 2008-05-25 10:24:14 EDT
Spec URL: http://files.spielen-unter-linux.de/repository/fedora/9/SRPMS/fbpanel.spec
SRPM URL: http://files.spielen-unter-linux.de/repository/fedora/9/SRPMS/fbpanel-4.12-1.fc9.src.rpm

Description: 

fbpanel is a lightweight X11 desktop panel. It works with any ICCCM / NETWM 
compliant window manager (eg sawfish, metacity, openbox , xfwm4, kvm)
It features tasklist, pager, launcbar, clock, menu and sytray.


- This is my first package submission here.
Comment 1 Hemant Goyal 2008-06-06 13:22:37 EDT
Hi Stefan,

This is not a formal review but only a pre-review of your package. I am not
entitled to Sponsor you however, someone else will.

1] Please perform rpmlint <rpmname> on every package that you create.

The following rpmlint errors must be resolved
---------------------------------------------------
bpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/dclock.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/space.so
fbpanel.i386: E: arch-dependent-file-in-usr-share /usr/share/fbpanel/plugins/tray.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/pager.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/launchbar.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/deskno2.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/wincmd.so
fbpanel.i386: E: arch-dependent-file-in-usr-share /usr/share/fbpanel/plugins/test.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/taskbar.so
fbpanel.i386: E: arch-dependent-file-in-usr-share /usr/share/fbpanel/plugins/cpu.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/image.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/tclock.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/icons.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/separator.so
fbpanel.i386: E: arch-dependent-file-in-usr-share /usr/share/fbpanel/plugins/menu.so
fbpanel.i386: E: arch-dependent-file-in-usr-share
/usr/share/fbpanel/plugins/deskno.so
----------------------------------------------------

2]Please note :

Every binary RPM package which stores shared library files (not just symlinks)
in any of the dynamic linker's default paths, must call ldconfig in %post and
%postun. If the package has multiple subpackages with libraries, each subpackage
should also have a %post/%postun section that calls /sbin/ldconfig. An example
of the correct syntax for this is:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

3] 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. This is described in detail here
http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files. If you feel
that your packaged GUI application does not need a .desktop file, you must put a
comment in the spec file with your explanation.

Thanks
Comment 2 Stefan Posdzich 2008-06-06 21:01:58 EDT
Thank you Hemant for your feedback!
Here is my new version.

Spec URL: http://files.spielen-unter-linux.de/repository/fedora/9/SRPMS/
fbpanel.spec
SRPM URL: http://files.spielen-unter-linux.de/repository/fedora/9/SRPMS/
fbpanel-4.12-2.fc9.src.rpm

Changelog: - fixed rpmlint errors, new .desktop file, cleanup
Comment 4 Mamoru TASAKA 2008-06-15 10:03:07 EDT
The srpm on comment 3 does not build.

http://koji.fedoraproject.org/koji/taskinfo?taskID=662736
Comment 5 Robert Scheck 2008-06-15 10:21:32 EDT
Yes, fbpanel-4.12-libs.patch seems a bit broken, tries %{_libdir} rather 
$RPM_BUILD_ROOT%{_libdir} or so.

Stefan, you're replacing "$(PREFIX)/share/fbpanel/plugins" by "/usr/lib/fbpanel/
plugins" but you should try "(PREFIX)/lib/fbpanel/plugins" instead for example.
Comment 6 Robert Scheck 2008-06-15 10:27:30 EDT
Oh, I forgot: This doesn't solve /usr/lib vs. /usr/lib64 anyway.
Comment 7 Robert Scheck 2008-06-15 15:14:28 EDT
Created attachment 309404 [details]
fbpanel-4.12-libdir.patch

This patch should solve all issues regarding libraries in %{_datadir} and the
correct usage of %{_libdir} regarding /usr/lib vs. /usr/lib64. I think, I have
written it sane enough, so that maybe upstream also applies it.
Comment 8 Stefan Posdzich 2008-06-15 15:49:03 EDT
SPEC: http://cheekyboinc.spielen-unter-linux.de/fbpanel.spec
SRPM: http://cheekyboinc.spielen-unter-linux.de/fbpanel-4.12-3.fc9.src.rpm

Changelog:

- Solved build failure and broken libs-patch with patch from Robert Scheck

Thank you Robert and Mamoru for your feedback!
Comment 9 Mamoru TASAKA 2008-06-16 14:27:26 EDT
If no one beats me, I will try to look at this package tomorrow.
Comment 10 Mamoru TASAKA 2008-06-16 15:09:49 EDT
For 4.12-3:

* License
  - Well, actually I cannot understand why the upstream ships MIT
    license text...
------------------------------------------------------------
bg.c			LGPLv2+
bg.h			LGPLv2+
ev.c			LGPLv2+
ev.h			LGPLv2+
gtkbar.c		LGPLv2+
gtkbar.h		LGPLv2+
gtkbgbox.c		LGPLv2+
gtkbgbox.h		LGPLv2+
plugins/cpu.c		GPLv2+
plugins/pager.c		GPLv2+
systray/eggtraymanager.c	LGPLv2+
systray/eggtraymanager.h	LGPLv2+
systray/fixedtip.c		GPLv2+
systray/fixedtip.h		GPLv2+
------------------------------------------------------------
    I have not analysed how these codes are used/linked yet,
    however from a quick glance, the license tag must be
------------------------------------------------------------
Group:		User Interface/Desktops
# %%{_bindir}/fbpanel and almost all plugins are under LGPLv2+
# Some plugins (cpu.so, pager.so, tray.so) are under GPLv2+
License:	LGPLv2+ and GPLv2+
URL:		http://fbpanel.sourceforge.net
------------------------------------------------------------

* SourceURL
  http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge
  - Please follow above.

* SOURCE2
  - Write the full URL of SOURCE2, or write as a comment how
    you received Source2.

* Redundant BuildRequires
  - gtk2-devel Requires atk-devel, pango-devel, cairo-devel so
    all these 3 BuildRequires are redundant.

* Timestamp
  - When using "cp" or "install" commands, add "-p" option to
    keep timestamps on installed files, like
------------------------------------------------------------
install -p -m 0644 %{SOURCE2}	\
	$RPM_BUILD_ROOT%{_datadir}/icons/hicolor/32x32/apps
------------------------------------------------------------

  - Also, as this package installs many image files and so on
    which are actually not modified during build, keeping 
    timestamps on those files are also perferred.
    To do so, please consider to use below:
------------------------------------------------------------
sed -i.stamp -e 's|install -m|install -p -m|' \
	Makefile */Makefile
------------------------------------------------------------
    at %prep stage.

* Debuginfo rpm missing
  - rpmlint complains about debuginfo rpm as below:
------------------------------------------------------------
fbpanel-debuginfo.i386: E: empty-debuginfo-package
------------------------------------------------------------
    debuginfo rpm is not created correctly. Several reasons are
    related to this.
    * Fedora specific compilation flags are not honored correctly.
      Check:
      http://fedoraproject.org/wiki/Packaging/Guidelines#CompilerFlags
    ! To check if compiler flags are correctly honored, please
      make build log more verbosely. The outputs like
------------------------------------------------------------
    74  CC   systray/eggtraymanager.o
    75  DEP  plugins/space.dep
    76  DEP  plugins/pager.dep
    77  DEP  plugins/launchbar.dep
    78  CC   systray/fixedtip.o
    79  CC   plugin.o
    80  CC   gtkbar.o
 ------------------------------------------------------------
      is not useful.
    * Makefile "strip" binaries to be installed. /usr/lib/rpm/find-debuginfo.sh
      strips installed binaries correctly and Makefiles themselves must
      not strip binaries.
    * Installed plugin objects has 0644 permission. As these objects are dlopened
      so these should work, however as find-debuginfo.sh strips binaries only
      with executable permissions, on Fedora these must have 0755 permission.

    The following will fix these 4 issues
    (please check what these changes actually do)
-------------------------------------------------------------
%prep
%setup -q
%patch -p1 -b .libdir
sed -i.strip -e 's|strip|true strip|' \
	Makefile */Makefile
sed -i.stamp -e 's|install -m|install -p -m|' \
	Makefile */Makefile

%build
./configure --prefix=%{_prefix} --libdir=%{_lib}
make \
	%{?_smp_mflags} \
	Q= \
	CFLAGS="${RPM_OPT_FLAGS}"


%install
rm -rf $RPM_BUILD_ROOT
make install PREFIX=$RPM_BUILD_ROOT%{_prefix}
.......
.......
install -p -m 0644 %{SOURCE2}	\
	$RPM_BUILD_ROOT%{_datadir}/icons/hicolor/32x32/apps

find $RPM_BUILD_ROOT%{_libdir}/%{name} -name \*.so -print0 | \
	xargs -0 chmod 0755

%clean
.......
-------------------------------------------------------------

* /sbin/ldconfig call
  - Calling /sbin/ldconfig on scriptlets is not needed for this
    package as no "libraries" are installed under default ldconfig
    paths.

* Icon cache update
  - As icons are installed under %_datadir/icons/hicolor, please
    follow
    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GTK.2B_icon_cache
Comment 11 Stefan Posdzich 2008-06-17 10:08:43 EDT
Wow, so...
Here is the new package.

SPEC: http://cheekyboinc.spielen-unter-linux.de/fbpanel.spec
SRPM: http://cheekyboinc.spielen-unter-linux.de/fbpanel-4.12-4.fc9.src.rpm

Changelog:

- Add correct url for Source:
- Add gtk-update-icon-cache
- Add timestamps
- Add missing debuginfo rpm
- Changed licence, MIT to LGPLv2+ and GPLv2+
- Remove unneeded ldconfig
- Remove redundant BuildRequires: atk-devel, pango-devel and cairo-devel
Comment 12 Mamoru TASAKA 2008-06-17 13:46:20 EDT
For 4.12-4:

* License analysis
  - Please also write license analysis I wrote on comment 10
    as comments in the spec file.

* Again Source2:
--------------------------------------------------------
# image taken from the fbpanel source /config/images/star.png
Source2:	%{name}.png
--------------------------------------------------------
  - Okay, then you don't have to add this file seperately to
    srpm. The following will be enough.
--------------------------------------------------------
install -p -m 0644 config/images/star.png	\
	$RPM_BUILD_ROOT%{_datadir}/icons/hicolor/128x128/apps/%{name}.png
--------------------------------------------------------
    ! Note: This file has 128x128 size, not 32x32.

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 13 Stefan Posdzich 2008-06-17 20:00:23 EDT
SPEC: http://cheekyboinc.spielen-unter-linux.de/fbpanel.spec
SRPM: http://cheekyboinc.spielen-unter-linux.de/fbpanel-4.12-5.fc9.src.rpm

Changelog:

- Add comment about the license
- Remove redundant Source2:

You can find another package (review request) here:

XQF - A server browser for many popular games
https://bugzilla.redhat.com/show_bug.cgi?id=451280

Thank you Mamoru for your helpfull overview about the review process and all 
the other things. 
Comment 14 Mamoru TASAKA 2008-06-18 05:29:37 EDT
Okay.

+ This package itself is good now
+ Your anothre review request seems good to some extent at a quick
  glance.

----------------------------------------------------------------
            This package (fbpanel) is APPOVED by me
----------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.
Comment 15 Stefan Posdzich 2008-06-18 07:58:32 EDT
I finished the procedure in May.
I am member of cla_done and have requested the membership of cvsextras.
My FAS name is "cheekyboinc"

------

Okay, I will look at the koji and Bodhi system! 
Comment 16 Mamoru TASAKA 2008-06-18 08:19:59 EDT
Now I am sponsoring you. please follow "Join" wiki again.
Comment 17 Stefan Posdzich 2008-06-18 09:59:03 EDT
New Package CVS Request
=======================
Package Name: fbpanel
Short Description: a lightweight X11 desktop panel
Owners: cheekyboinc
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 18 Kevin Fenzi 2008-06-18 20:30:27 EDT
cvs done.
Comment 19 Fedora Update System 2008-06-19 12:49:21 EDT
fbpanel-4.12-5.fc9 has been submitted as an update for Fedora 9
Comment 20 Fedora Update System 2008-06-19 12:53:13 EDT
fbpanel-4.12-5.fc8 has been submitted as an update for Fedora 8
Comment 21 Mamoru TASAKA 2008-06-19 13:42:28 EDT
Closing.
Comment 22 Fedora Update System 2008-06-20 15:12:01 EDT
fbpanel-4.12-5.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 23 Fedora Update System 2008-06-20 15:14:08 EDT
fbpanel-4.12-5.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Damien Durand 2012-04-05 07:26:17 EDT
Package Change Request
======================
Package Name: fbpanel
New Branches: el6
Owners: splinux
Comment 25 Jon Ciesla 2012-04-05 08:08:40 EDT
Git done (by process-git-requests).
Comment 26 Fedora Update System 2012-04-05 08:30:26 EDT
fbpanel-6.1-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/fbpanel-6.1-4.el6
Comment 27 Fedora Update System 2012-04-21 16:57:25 EDT
fbpanel-6.1-4.el6 has been pushed to the Fedora EPEL 6 stable repository.

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