Bug 701079 (wmSun) - Review Request: wmSun - Rise/Set time of Sun in a dockapp
Summary: Review Request: wmSun - Rise/Set time of Sun in a dockapp
Keywords:
Status: CLOSED ERRATA
Alias: wmSun
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ankur Sinha (FranciscoD)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-30 22:46 UTC by Mario Blättermann
Modified: 2011-07-12 21:59 UTC (History)
4 users (show)

Fixed In Version: wmSun-1.03-4.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-07-12 21:59:07 UTC
Type: ---
sanjay.ankur: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Mario Blättermann 2011-04-30 22:46:36 UTC
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmSun.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/wmSun-1.03-1.fc14.src.rpm
Description:
Rise/Set time of Sun in a WindowMaker dockapp.

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3041745

Comment 1 Fabian Affolter 2011-05-30 07:23:48 UTC
Just a quick comment: $RPM_BUILD_ROOT and %{buildroot} are mixed.

Comment 2 Mario Blättermann 2011-05-31 19:41:26 UTC
Thanks for the hint.

New files:
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmSun.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmSun-1.03-2.fc15.src.rpm

Comment 3 Ankur Sinha (FranciscoD) 2011-06-29 16:42:40 UTC
I'll take this one :)

Comment 4 Ankur Sinha (FranciscoD) 2011-06-30 17:29:07 UTC
Review:

+ OK
- NA
? Issue

+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:
[ankur@ankur rpmbuild]$ md5sum wmSun-1.03.tar.gz SOURCES/wmSun-1.03.tar.gz 
34a2ea132f1a04e18be2d93abce40722  wmSun-1.03.tar.gz
34a2ea132f1a04e18be2d93abce40722  SOURCES/wmSun-1.03.tar.gz


- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

? Package is a GUI app and has a .desktop file
^^^

It is a GUI, isn't it? I haven't seen a desktop file in the source. If you can, please create one and add it as an extra source. Do also send this upstream. (Only if it is a GUI of course.)

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.
[ankur@ankur SRPMS]$ rpmlint ../SPECS/wmSun.spec wmSun-1.03-2.fc15.src.rpm /var/lib/mock/fedora-rawhide-i386/result/*.rpm
../SPECS/wmSun.spec:51: W: macro-in-%changelog %{buildroot}
wmSun.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.src:51: W: macro-in-%changelog %{buildroot}
wmSun.i686: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.i686: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.i686: E: incorrect-fsf-address /usr/share/doc/wmSun-1.03/COPYING
wmSun.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.src:51: W: macro-in-%changelog %{buildroot}
wmSun-debuginfo.i686: E: empty-debuginfo-package
4 packages and 1 specfiles checked; 2 errors, 9 warnings.

^^^ 

-> I see you're using the -s (strip) option while installing. Please do not do that.
http://fedoraproject.org/wiki/Packaging:Debuginfo#Useless_or_incomplete_debuginfo_packages_due_to_packaging_issues

-> The macro in changelog is ignorable. 

+ final provides and requires are sane:
[ankur@ankur result]$ review-req-check 
== wmSun-1.03-2.fc16.i686.rpm ==
Provides:
wmSun = 1.03-2.fc16
wmSun(x86-32) = 1.03-2.fc16

Requires:
libX11.so.6  
libXext.so.6  
libXpm.so.4  
libc.so.6  
libc.so.6(GLIBC_2.0)  
libc.so.6(GLIBC_2.1)  
libm.so.6  
libm.so.6(GLIBC_2.0)  
rtld(GNU_HASH)  

== wmSun-1.03-2.fc16.src.rpm ==
Provides:

Requires:
libXext-devel  
libXpm-devel  

== wmSun-debuginfo-1.03-2.fc16.i686.rpm ==
Provides:
wmSun-debuginfo = 1.03-2.fc16
wmSun-debuginfo(x86-32) = 1.03-2.fc16

Requires:


SHOULD Items:

+ Should build in mock.
+ Should build on all supported archs
- Should function as described.
+ Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1. buildroot tag isn't required any more
2. the clean section isn't required any more
3. Please report the incorrect FSF address upstream.
4. Please remove the -s flag from install. It should then generate debuginfo. 

Small, easily correctable issues :)

Thanks,
Ankur.

Comment 5 Mario Blättermann 2011-06-30 20:10:20 UTC
(In reply to comment #4)

> ? Package is a GUI app and has a .desktop file
> ^^^
> 
> It is a GUI, isn't it? I haven't seen a desktop file in the source. If you can,
> please create one and add it as an extra source. Do also send this upstream.
> (Only if it is a GUI of course.)

The package has a GUI, but a *.desktop file is not needed for Windowmaker dockapps, because users don't start them from a menu. Usually, dockapps will be launched from a startup script. No one of the current dockapp packages in Fedora has a desktop file. Look here what "Spot" said:

https://bugzilla.redhat.com/show_bug.cgi?id=478744#c4

> 3. Please report the incorrect FSF address upstream.

Impossible, unfortunately. The development is down for a long time. In such cases, I don't have to do anything, because there is no chance to get it ever fixed upstream:

https://bugzilla.redhat.com/show_bug.cgi?id=690919#c5


Updated package:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmSun.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmSun-1.03-3.fc15.src.rpm

Comment 6 Ankur Sinha (FranciscoD) 2011-07-01 02:33:23 UTC
(In reply to comment #5)
> (In reply to comment #4)
> 
> > ? Package is a GUI app and has a .desktop file
> > ^^^
> > 
> > It is a GUI, isn't it? I haven't seen a desktop file in the source. If you can,
> > please create one and add it as an extra source. Do also send this upstream.
> > (Only if it is a GUI of course.)
> 
> The package has a GUI, but a *.desktop file is not needed for Windowmaker
> dockapps, because users don't start them from a menu. Usually, dockapps will be
> launched from a startup script. No one of the current dockapp packages in
> Fedora has a desktop file. Look here what "Spot" said:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=478744#c4
> 
> > 3. Please report the incorrect FSF address upstream.
> 
> Impossible, unfortunately. The development is down for a long time. In such
> cases, I don't have to do anything, because there is no chance to get it ever
> fixed upstream:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=690919#c5
> 
> 
> Updated package:
> 
> Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmSun.spec
> SRPM URL:
> http://mariobl.fedorapeople.org/Review/SRPMS/wmSun-1.03-3.fc15.src.rpm

Hello!

[ankur@ankur SRPMS]$ rpmlint ../SPECS/wmSun.spec wmSun-1.03-3.fc15.src.rpm /var/lib/mock/fedora-rawhide-i386/result/*.rpm
../SPECS/wmSun.spec:51: W: macro-in-%changelog %{buildroot}
wmSun.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.src:51: W: macro-in-%changelog %{buildroot}
wmSun.i686: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.i686: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.i686: E: incorrect-fsf-address /usr/share/doc/wmSun-1.03/COPYING
wmSun.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.src:51: W: macro-in-%changelog %{buildroot}
wmSun-debuginfo.i686: E: debuginfo-without-sources
4 packages and 1 specfiles checked; 2 errors, 9 warnings.


- You can maybe change the spelling (dockapp -> dock app).

- You can also get rid of the rm -rf $RPM_BUILD_ROOT in the install section. It is not needed any more. 

- Debuginfo is still empty. This is because the fedora optflags are not being honoured in the build:

DEBUG: + make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wl,-z,relro -m32 -march=i6
86 -mtune=atom -fasynchronous-unwind-tables' -j4
DEBUG: gcc  -DLinux -c wmSun.c -o wmSun.o -I/usr/X11R6/include/X11
DEBUG: gcc  -DLinux -c SunRise.c -o SunRise.o -I/usr/X11R6/include/X11
DEBUG: gcc  -DLinux -c ../wmgeneral/wmgeneral.c -o ../wmgeneral/wmgeneral.o -I/usr/X11R6/include/X11
DEBUG: gcc   -lm -o wmSun wmSun.o SunRise.o ../wmgeneral/wmgeneral.o -I/usr/X11R6/include/X11 -L/usr/X11R6/lib -lXpm -lX11 -lXext


Even though CFLAGS has been defined in the makefile, they aren't used in the compile command. Change the make command as so:

make COPTS='%{optflags}' %{?_smp_mflags}

This fixes it. 

Thanks,
Ankur

Comment 7 Mario Blättermann 2011-07-01 17:52:51 UTC
(In reply to comment #6)
> - You can maybe change the spelling (dockapp -> dock app).
> 
It is the usual spelling for Windowmaker related applets:
http://en.wikipedia.org/wiki/Dockapp

> - You can also get rid of the rm -rf $RPM_BUILD_ROOT in the install section. It
> is not needed any more. 
>
Done.
> - Debuginfo is still empty. This is because the fedora optflags are not being
> honoured in the build:
...
> make COPTS='%{optflags}' %{?_smp_mflags}
> 
Done.

Updated package:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmSun.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmSun-1.03-4.fc15.src.rpm

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3174781

Comment 8 Ankur Sinha (FranciscoD) 2011-07-02 03:49:27 UTC
Package looks good. Please do inform upstream about the incorrect FSF address when you can. 

Final rpmlint output:

[ankur@ankur SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-i386/result/*.rpm
wmSun.i686: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.i686: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.i686: E: incorrect-fsf-address /usr/share/doc/wmSun-1.03/COPYING
wmSun.src: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
wmSun.src: W: spelling-error %description -l en_US dockapp -> dock app, dock-app, paddock
wmSun.src:54: W: macro-in-%changelog %{buildroot}
wmSun-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/wmSun-1.03/wmSun/wmSun.c
3 packages and 0 specfiles checked; 2 errors, 5 warnings.


Ignorable. 

This package is XXX APPROVED XXX :D

Comment 9 Mario Blättermann 2011-07-02 16:28:19 UTC
Many thanks for your review!

(In reply to comment #8)
> Package looks good. Please do inform upstream about the incorrect FSF address
> when you can. 

As I already wrote, upstream is dead. But there is a software pool for Windowmaker dockapps at http://www.dockapps.org. I will leave a comment there.

Comment 10 Mario Blättermann 2011-07-02 16:31:51 UTC
New Package SCM Request
=======================
Package Name: wmSun
Short Description: Rise/Set time of Sun in a dockapp
Owners: mariobl
Branches: f15

Comment 11 Gwyn Ciesla 2011-07-02 16:53:59 UTC
Git done (by process-git-requests).

Ankur, in the future, please assign reviews you are working on to yourself. 
Thank you!

Comment 12 Ankur Sinha (FranciscoD) 2011-07-02 17:40:52 UTC
(In reply to comment #11)
> Git done (by process-git-requests).
> 
> Ankur, in the future, please assign reviews you are working on to yourself. 
> Thank you!

OH!! I somehow forgot. Won't happen again.

Comment 13 Fedora Update System 2011-07-02 19:41:53 UTC
wmSun-1.03-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/wmSun-1.03-4.fc15

Comment 14 Fedora Update System 2011-07-03 19:22:43 UTC
wmSun-1.03-4.fc15 has been pushed to the Fedora 15 testing repository.

Comment 15 Fedora Update System 2011-07-12 21:59:01 UTC
wmSun-1.03-4.fc15 has been pushed to the Fedora 15 stable repository.


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