Red Hat Bugzilla – Bug 235763
Review Request: windowlab - Small and Simple Amiga-like Window Manager
Last modified: 2007-11-30 17:12:01 EST
Spec URL: http://dev.nigelj.com/SRPMS/windowlab.spec SRPM URL: http://dev.nigelj.com/SRPMS/windowlab-1.34-1.src.rpm Description: WindowLab is a Window Manager for the X Window System. Features include click-to-focus and a simple menu/taskbar combination. WindowLab is incredibly fast and small. rpmlint on src.rpm and built rpm files are clean. I believe I have got the correct Build-Requires, sadly I don't have a mock enviroment to test with at the moment (I hope to change that soon). It is also compliant with changes made to the Packaging Guidelines announced today. n.b> The chmod a-x trick is to keep rpmlint etc happy by not having executable documentation, but technically the whole source should be extracted without executable permissions, my attempt at this failed, so I'd appreciate if someone could shed some light on to it.
I cannot sponsor you so just a few notes: * windowlab-1.34-pathfixes.patch looks useless - you'd better to redefine $XROOT variable in Makefile from /usr/X11R6 to /usr - it resolves some other issues. * change %files section to something like: %files %defattr(-,root,root,-) %dir %{_sysconfdir}/X11/windowlab/ %config(noreplace) %{_sysconfdir}/X11/windowlab/windowlab.menurc %{_bindir}/windowlab %{_mandir}/man1/windowlab.1x.gz %{_datadir}/xsessions/windowlab.desktop %doc CHANGELOG README TODO LICENCE
Spec URL: http://dev.nigelj.com/SRPMS/windowlab.spec SRPM URL: http://dev.nigelj.com/SRPMS/windowlab-1.34-2.src.rpm -2, updated %files per recommendation, also changed the patch.
I also can't sponsor, but I'll make some notes rather than a formal review: 1) Builds cleanly in mock 2) rpmlint is silent on RPM and SRPM 3) Package doesn't respect RPM_OPT_FLAGS, see: http://fedoraproject.org/wiki/Packaging/Guidelines#head-8b14098227aebff1cf6188939e9d0877295ac448 + make gcc -g -O2 -Wall -W -DSHAPE -DDEF_MENURC="\"/etc/X11/windowlab/windowlab.menurc\"" -I/usr/include -c main.c -o m ain.o gcc -g -O2 -Wall -W -DSHAPE -DDEF_MENURC="\"/etc/X11/windowlab/windowlab.menurc\"" -I/usr/include -c events.c -o events.o
Spec URL: http://dev.nigelj.com/SRPMS/windowlab.spec SRPM URL: http://dev.nigelj.com/SRPMS/windowlab-1.34-3.src.rpm I spoke with Alex on IRC regarding his mock build (while he did it). It was then I discovered an issue with the debuginfo package, both have been fixed (the latter via another patch, depending on upstream's stance the patch may be merged, but I'd like to keep it seperate for now)
Hey Nigel. I will review this package and look at sponsoring you... I know you have 3 other submissions pending, but they are all waiting on guidelines, correct? Do you have any other packages to submit? Look for a review of this package in a bit.
(In reply to comment #5) > Hey Nigel. I will review this package and look at sponsoring you... > I know you have 3 other submissions pending, but they are all waiting on > guidelines, correct? Do you have any other packages to submit? Correct, Bug #235804, Bug #235805 & Bug #235815 are all held up by Ocaml's static linking, I'm still looking for workarounds on the issue, and considering asking FESCo at the next meeting to allow an exception (per guidelines). I'm planning on packaging a game CSP but that is held up by Bug #236517 (current scons version to too old to build some newer packages. I'm also still looking at other suitable packages, but once sponsored I'm going to spend some time to get the ocaml issues sorted with the other 3 packages. > > Look for a review of this package in a bit. Will do, thanks.
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: d566b989f8e59b169f7affa462762c17 windowlab-1.34.tar d566b989f8e59b169f7affa462762c17 windowlab-1.34.tar.1 OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have dist tag OK - Should package latest version Issues: 1. rpmlint says: W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/events.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/taskbar.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/menufile.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/main.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/windowlab.h W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/manage.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/new.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/misc.c W: windowlab-debuginfo spurious-executable-perm /usr/src/debug/windowlab-1.34/client.c This is pretty cosmetic, but you could 'chmod 644' those sources and/or just mention it to the upstream that they shouldn't have executable source files in their tar. I don't see any blockers here, so this package is APPROVED. I will be happy to sponsor you. You can continue the process from: http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b Don't forget to close this review request once this package has been imported and built.
(In reply to comment #7) > 1. rpmlint says: > > W: windowlab-debuginfo spurious-executable-perm > /usr/src/debug/windowlab-1.34/events.c [...] > > This is pretty cosmetic, but you could 'chmod 644' those sources and/or > just mention it to the upstream that they shouldn't have executable source files > in their tar. > Ahh I had that problem before with this package (the chmod trick before, forgot about debuginfo having the same problem, will fix when possible > I don't see any blockers here, so this package is APPROVED. > Thank You > I will be happy to sponsor you. > You can continue the process from: > http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b > Thank you again, I have applied for cvsextras/fedorabugs, also could you add me to the wiki editing group? > Don't forget to close this review request once this package has been imported > and built. > New Package CVS Request ======================= Package Name: windowlab Short Description: Small and Simple Amiga-like Window Manager Owners: dev@nigelj.com Branches: FC-5 FC-6 InitialCC: Thanks
Built okay, closing