Bug 684938

Due to continuing problems with the startup script, I've dropped it now and added a hint to the description.

The initial problem was: The dockapp doesn't start if no local config file was found. The script runs a test if a config file exists, and if not, then the example config file shipped with the package will be copied to the home folder. Actually I assume that dockapp users should be somewhat familiar with the command line, because that's the only way to see the error message. We have no *.desktop file, that's why the user has to launch wmdrawer from the command line anyway. 

Spec URL: http://dl.dropbox.com/u/19373040/Fedora/SPECS/wmdrawer.spec
SRPM URL:
http://dl.dropbox.com/u/19373040/Fedora/wmdrawer-0.10.5-9.fc14.src.rpm

Files have moved to a new location:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmdrawer.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/wmdrawer-0.10.5-9.fc15.src.rpm

Ok, first things first! :)

Looking at the spec file you're copying INSTALL with the %doc's. I've noticed several packages do this but you don't need the install directions since we're building a binary package, also, the guidelines say to leave it out.

I built the package under mock for F14 x86_64 and I get the following rpmlint output:
"""
$ rpmlint *.rpm
wmdrawer.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, dockage
wmdrawer.src: I: enchant-dictionary-not-found de
wmdrawer.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, dockage
wmdrawer.src: W: spelling-error %description -l en_US usr -> use, us, user
wmdrawer.src: W: file-size-mismatch wmdrawer-0.10.5.tar.gz = 49109, http://people.easter-eggs.org/~valos/wmdrawer/wmdrawer-0.10.5.tar.gz = 49479
wmdrawer.x86_64: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, dockage
wmdrawer.x86_64: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, dockage
wmdrawer.x86_64: W: file-not-utf8 /usr/share/doc/wmdrawer-0.10.5/ChangeLog
wmdrawer.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/wmdrawer-0.10.5/wmdrawer-it.sgml
wmdrawer.x86_64: W: file-not-utf8 /usr/share/doc/wmdrawer-0.10.5/wmdrawer-it.sgml
wmdrawer.x86_64: W: file-not-utf8 /usr/share/doc/wmdrawer-0.10.5/AUTHORS
wmdrawer.x86_64: E: incorrect-fsf-address /usr/share/doc/wmdrawer-0.10.5/COPYING
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/graphics.c
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/config.c
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/utils.c
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/images.c
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/types_defs.h
wmdrawer-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/wmdrawer-0.10.5/wmdrawer.c
3 packages and 0 specfiles checked; 7 errors, 10 warnings.
"""

1. Problem: The file-size-mismatch. Did you alter the source archive? Or maybe upstream tweaked something without creating a new version?

2. All the wrong-file-end-of-line-encoding and file-not-utf8 errors should probably be fixed with "dos2linux -k <file>" after "make install". 

3. The incorrect-fsf-address is nothing you can fix but should be reported upstream.

After looking into things a bit more I noticed a few other issues. 

4. There are no files in the wmdrawer-debuginfo package. The reason is that the Makefile is using "strip" to remove the debugging symbols. There's no flag to pass to stop that behavior so I fixed it with sed as follows:

# Prevent the Makefile from stripping the binaries
# Otherwise there are no debugging symbols
sed -i '/strip/d' Makefile

5. The makefile is not honoring your "OPTS" argument to make. I tried changing it to CFLAGS but that broke compiling because it replaced CFLAGS in the makefile instead of adding to it. I fixed that as follows:

# Patch the Makefile so that CFLAGS are added not replaced.
sed -i 's/CFLAGS =/override CFLAGS +=/g' Makefile

Then updated your make command to:

make -L CFLAGS='%{optflags}' %{?_smp_mflags}

6. Not really a problem but you could (if you wanted to) replace the last two arguments to %doc with:

doc/*.smgl

That's it as far as I know. I'll probably run it through the packaging guidelines tomorrow.

Here's a link to the updated spec if you want to use it:
http://hobbes1069.fedorapeople.org/wmdrawer.spec

Richard

Thanks for your hints and the updated spec file. But please wait with a full review. I found some more problems (runtime font requirements, use of gdk-pixbuf-2 instead of gdk-pixbuf). Stay tuned, I will fix it tomorrow.

It should be a simple fix. I think I remember something in the makefile about gdk-pixbuf. I think it can use 3 different versions and you just uncomment the appropriate one. 

That being the case I think we should give up on the sed's and just make a patch to the makefile to take care of all 3 problems.

OK, I've patched the Makefile to concatenate the sed commands and the use of gdk-pixbuf2. Now it works fine.

Koji scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3232188

(In reply to comment #3)
> 
> 1. Problem: The file-size-mismatch. Did you alter the source archive? Or maybe
> upstream tweaked something without creating a new version?
Nothing done with the sources,really. I don't understand it... I've downloaded the sources again and used them for the new package. That's all I can do for now.
> 
> 2. All the wrong-file-end-of-line-encoding and file-not-utf8 errors should
> probably be fixed with "dos2linux -k <file>" after "make install". 
> 
Not dos2linux, but dos2unix is available from the yum repos. But dos2unix seems to need an input declaration. Is there a more simple way to convert the files to utf-8 with correct line breaks?

> 3. The incorrect-fsf-address is nothing you can fix but should be reported
> upstream.
It is a common problem in older software, such as a lot of WindowMaker dockapps. Upstream is dead in most cases. Well, I could fix it by patching the source files and COPYING, but I think it isn't worth the effort.

(In reply to comment #4)
[...]

> 6. Not really a problem but you could (if you wanted to) replace the last two
> arguments to %doc with:
> 
> doc/*.smgl
> 
What's the reason for that? I don't know about *.smgl files... Normally, the sgml files should be processed with docbook2man to get a manpage. Actually, I can drop the English version, because there is already a real manpage. The Italian version seems to be incomplete, given the file size. In my opinion, the sgml files should be dropped completely.

New files:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmdrawer.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmdrawer-0.10.5-10.fc15.src.rpm

(In reply to comment #7)
> OK, I've patched the Makefile to concatenate the sed commands and the use of
> gdk-pixbuf2. Now it works fine.
> 
> Koji scratch build:
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3232188
> 
> (In reply to comment #3)
> > 
> > 1. Problem: The file-size-mismatch. Did you alter the source archive? Or maybe
> > upstream tweaked something without creating a new version?
> Nothing done with the sources,really. I don't understand it... I've downloaded
> the sources again and used them for the new package. That's all I can do for
> now.

As long as the md5sum's match it should be good, not sure why it was "off"...


> > 2. All the wrong-file-end-of-line-encoding and file-not-utf8 errors should
> > probably be fixed with "dos2linux -k <file>" after "make install". 
> > 
> Not dos2linux, but dos2unix is available from the yum repos. But dos2unix seems
> to need an input declaration. Is there a more simple way to convert the files
> to utf-8 with correct line breaks?

Sorry, you'll have to excuse my typo's, of course I meant dos2unix. I don't think it requires anything but it doesn't always get it "right" either. I've used it on a few occasions with just the "dos2unix -k <file>". If it makes rpmlint happy then we're good, if not, there's always iconv which does require more arguments and as far as I know doesn't do the conversion "in place". 

 
> > 3. The incorrect-fsf-address is nothing you can fix but should be reported
> > upstream.
> It is a common problem in older software, such as a lot of WindowMaker
> dockapps. Upstream is dead in most cases. Well, I could fix it by patching the
> source files and COPYING, but I think it isn't worth the effort.
> 
> (In reply to comment #4)
> [...]
> 
> > 6. Not really a problem but you could (if you wanted to) replace the last two

Ok, just thought I'd check.


> > arguments to %doc with:
> > 
> > doc/*.smgl
> > 
> What's the reason for that? I don't know about *.smgl files... Normally, the
> sgml files should be processed with docbook2man to get a manpage. Actually, I
> can drop the English version, because there is already a real manpage. The
> Italian version seems to be incomplete, given the file size. In my opinion, the
> sgml files should be dropped completely.

You'll have to excuse my typo again, I meant "*.sgml". All I was trying to point out is that you don't need to list both files separately on the %doc line since globs are allowed. Dropping them is also fine.

Richard

Ok, two things.

1. You'r dos2unix path was pointing to the buildroot but they are not there until the %doc macro copies them, I changed it to this:

dos2unix -k AUTHORS     \
            ChangeLog   \
            doc/%{name}-it.sgml

2. New problem, rpmlint shows mixed spaces and tabs. I ran the spec file through "expand" to convert it all to spaces.

Now onto the guidelines!

Richard

Assuming the above are fixed...

+: OK
-: must be fixed
=: should be fixed (at your discretion)
?: Question or clairification needed
N: not applicable

MUST:
[+] rpmlint output: shown in comment: 
[+] follows package naming guidelines
[+] spec file base name matches package name
[+] package meets the packaging guidelines
[+] package uses a Fedora approved license: GPLv2+
[+] license field matches the actual license.
[+] license file is included in %doc: COPYING
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sums match (608bf2f75fdd084f8e63764c31a2a9a5)
[+] package builds on at least one primary arch: Tested F14
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[N] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[N] package contains a desktop file, uses desktop-file-install/validate
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[N] query upstream for license text
[N] description and summary contains available translations
[+] package builds in mock
[+] package builds on all supported arches
[?] package functions as described: Did not test.
[+] sane scriptlets
[N] subpackages require the main package
[N] placement of pkgconfig files
[N] file dependencies versus package dependencies
[+] package contains man pages for binaries/scripts

*** APPROVED ***

Many thanks for your review and especially your help. I will import the following package once the Git repo has been established:

http://mariobl.fedorapeople.org/Review/SRPMS/wmdrawer-0.10.5-11.fc15.src.rpm

The remaining problems are fixed.

New Package SCM Request
=======================
Package Name: wmdrawer
Short Description: Retractable button bar launcher dockapp
Owners: mariobl
Branches: f15 f16

Git done (by process-git-requests).

wmdrawer-0.10.5-10.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/wmdrawer-0.10.5-10.fc15

wmdrawer-0.10.5-11.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/wmdrawer-0.10.5-11.fc16

wmdrawer-0.10.5-11.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/wmdrawer-0.10.5-11.fc15

wmdrawer-0.10.5-11.fc15 has been pushed to the Fedora 15 testing repository.

wmdrawer-0.10.5-11.fc15 has been pushed to the Fedora 15 stable repository.

wmdrawer-0.10.5-11.fc16 has been pushed to the Fedora 16 stable repository.