Bug 708475

rpmlint output:

$ rpmlint pysdm.spec pysdm-0.4.1-1.fc14.src.rpm pysdm-0.4.1-1.fc14.noarch.rpm
pysdm.src: W: spelling-error %description -l en_US mountpoints -> mount points, mount-points, mountaintops
pysdm.src: W: spelling-error %description -l en_US fstab -> stab, f stab, fiesta
pysdm.src: W: spelling-error %description -l en_US udev -> udder, Udine, nude
pysdm.noarch: W: spelling-error %description -l en_US mountpoints -> mount points, mount-points, mountaintops
pysdm.noarch: W: spelling-error %description -l en_US fstab -> stab, f stab, fiesta
pysdm.noarch: W: spelling-error %description -l en_US udev -> udder, Udine, nude
pysdm.noarch: E: zero-length /usr/share/doc/pysdm-0.4.1/README
pysdm.noarch: E: zero-length /usr/share/doc/pysdm-0.4.1/NEWS
pysdm.noarch: E: incorrect-fsf-address /usr/share/doc/pysdm-0.4.1/COPYING
pysdm.noarch: W: no-manual-page-for-binary pysdm
2 packages and 1 specfiles checked; 3 errors, 7 warnings.

The zero length docs are in the source. I guess they could be removed but would need to be added back if they ever contained anything. I think it's easier to leave them in so their contents (whenever added) will automatically find their way into the package.

Unless you plan to use this spec file with EPEL also, you can remove the BuildRoot tag, "rm -rf $RPM_BUILD_ROOT" at the top of %install, the %clean section, and the %defattr line in %files.

+: OK
-: must be fixed
=: should be fixed (at your discretion)
N: not applicable

MUST:
[+] rpmlint output: shown in comment 1 (slightly different spelling suggestions, but otherwise identical on my machine)
[+] follows package naming guidelines
[+] spec file base name matches package name
[=] package meets the packaging guidelines: the canonical URL for Source0 is actually http://downloads.sourceforge.net/pysdm/pysdm-0.4.1.tar.gz, as noted in https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net.
[+] package uses a Fedora approved license
[-] license field matches the actual license: I don't see any license statements, except on bundled code, and the top-level COPYING file is the LGPL.  Where did "GPLv2" come from?
[+] license file is included in %doc
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum is bc3b671ac95065c5121e056d820fd0a2 for both
[+] package builds on at least one primary arch (tried x86_64)
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[+] spec file handles locales properly
[N] ldconfig in %post and %postun
[-] no bundled copies of system libraries: This package bundles two files from tepache (https://launchpad.net/tepache), namely tepache.py and SimpleGladeApp.py.
[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
[+] package contains a desktop file, uses desktop-file-install
[+] 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: tried fedora-rawhide-i386
[+] package builds on all supported arches: tried i386 and x86_64
[+] package functions as described: minimal testing only
[+] 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

(In reply to comment #2)
> Unless you plan to use this spec file with EPEL also, you can remove the
> BuildRoot tag, "rm -rf $RPM_BUILD_ROOT" at the top of %install, the %clean
> section, and the %defattr line in %files.

Yeah, usually I would remove it but thought there might be an off chance that someone will want this on EPEL... 


> MUST:
> [=] package meets the packaging guidelines: the canonical URL for Source0 is
> actually http://downloads.sourceforge.net/pysdm/pysdm-0.4.1.tar.gz, as noted in
> https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net.

I should have known there was a wiki for this :) "curl -L -O..." always worked so I figured it was just rpmlint being pedantic.... Fixed.


> [-] license field matches the actual license: I don't see any license
> statements, except on bundled code, and the top-level COPYING file is the LGPL.
>  Where did "GPLv2" come from?

I'm still learning how licences work... I thought that when COPYING said GPL Version two that it meant GPLv2... I knew the L was "Lesser" but didn't know that GNU Library == "Lesser"

So it should be LGPL?


> [-] no bundled copies of system libraries: This package bundles two files from
> tepache (https://launchpad.net/tepache), namely tepache.py and
> SimpleGladeApp.py.

Man, I got to get better at finding bundled libraries :)

Ok, what do we do here? It doesn't appear to be already packaged for Fedora so this package would be the only user. Do we need a separate review request?

Strangely enough only SimpleGladeApp.py seems to make it into the rpm package...


> SHOULD:
> [=] package contains man pages for binaries/scripts

GUI only app...


Thanks,
Richard

(In reply to comment #3)
> I'm still learning how licences work... I thought that when COPYING said GPL
> Version two that it meant GPLv2... I knew the L was "Lesser" but didn't know
> that GNU Library == "Lesser"
> 
> So it should be LGPL?

"LGPLv2+" in particular; see the entry for "GNU Lesser General Public License (no version)" here: https://fedoraproject.org/wiki/Licensing#Good_Licenses.

> Ok, what do we do here? It doesn't appear to be already packaged for Fedora so
> this package would be the only user. Do we need a separate review request?

I'm afraid so.  I'll be happy to review that one for you, too.

> Strangely enough only SimpleGladeApp.py seems to make it into the rpm
> package...

Oh yeah?  Does it look like tepache.py should be in there too?

> GUI only app...

OK, no problem.

(In reply to comment #4)
> (In reply to comment #3)
> > I'm still learning how licences work... I thought that when COPYING said GPL
> > Version two that it meant GPLv2... I knew the L was "Lesser" but didn't know
> > that GNU Library == "Lesser"
> > 
> > So it should be LGPL?
> 
> "LGPLv2+" in particular; see the entry for "GNU Lesser General Public License
> (no version)" here: https://fedoraproject.org/wiki/Licensing#Good_Licenses.

Done.


> > Ok, what do we do here? It doesn't appear to be already packaged for Fedora so
> > this package would be the only user. Do we need a separate review request?
> 
> I'm afraid so.  I'll be happy to review that one for you, too.

I've got one pretty much ready but ran into an issue, see below.


> > Strangely enough only SimpleGladeApp.py seems to make it into the rpm
> > package...
> 
> Oh yeah?  Does it look like tepache.py should be in there too?

Ok, so pysdm only appears to use SimpleGladeApp.py astepache.py is intended as a binary, not a library. I'm guessing the pysdm group just pulled in all of the tepache package instead of just using SimpleGladeApp.py, which BTW, is ALL OVER THE PLACE in on the web and in other distros. Looks like it's been widely copied in many projects.

Also, a problem:
# yum whatprovides /usr/lib/*/site-packages/SimpleGladeApp.py
Loaded plugins: changelog, presto, refresh-packagekit, remove-with-leaves, rpm-warm-cache
gresistor-0.0.1-17.fc14.noarch : Gnome resistor color code calculator
Repo        : fedora
Matched from:
Filename    : /usr/lib/python2.7/site-packages/SimpleGladeApp.py

I'm assuming that once tepache gets approved that the gresistor maintainer will have to fix his package, right?

And this was supposed to be a simple one :)

Thanks,
Richard

(In reply to comment #5)
> Ok, so pysdm only appears to use SimpleGladeApp.py astepache.py is intended as
> a binary, not a library. I'm guessing the pysdm group just pulled in all of the
> tepache package instead of just using SimpleGladeApp.py, which BTW, is ALL OVER
> THE PLACE in on the web and in other distros. Looks like it's been widely
> copied in many projects.

Argh.  That's the lazy way for the developer to cope with the problem, but it causes all kinds of problems later on.

> Also, a problem:
> # yum whatprovides /usr/lib/*/site-packages/SimpleGladeApp.py
> Loaded plugins: changelog, presto, refresh-packagekit, remove-with-leaves,
> rpm-warm-cache
> gresistor-0.0.1-17.fc14.noarch : Gnome resistor color code calculator
> Repo        : fedora
> Matched from:
> Filename    : /usr/lib/python2.7/site-packages/SimpleGladeApp.py
> 
> I'm assuming that once tepache gets approved that the gresistor maintainer will
> have to fix his package, right?

Yes.  I think the right way to go about this is to file the review request for tepache, then file a bug against gresistor, and make the second bug depend on the first one.  That way the gresistor maintainer will be notified when tepache hits the repository.

> And this was supposed to be a simple one :)

I've thought that a couple of times, too. :-)

Review Request for tepache:

https://bugzilla.redhat.com/show_bug.cgi?id=710194

I created a patch to remove SimpleGladeApp.py from the makefile and installed the new build of pysdm and tepache and it found SimpleGladeApp.py in /usr/lib/python*/site-packages/ so other than getting tepache approved I think we're good to go.

Can you provide a link to the final spec file?  I'd like to look through it one more time to satisfy myself that we've addressed all the issues.  Thanks.

I can, but was waiting on a final call on the tepache package name...

(In reply to comment #9)
> Can you provide a link to the final spec file?  I'd like to look through it one
> more time to satisfy myself that we've addressed all the issues.  Thanks.

Here's the revised sources:

SPEC: http://hobbes1069.fedorapeople.org/pysdm/pysdm.spec
SRPM: http://hobbes1069.fedorapeople.org/pysdm/pysdm-0.4.1-2.fc14.src.rpm

On a side note, do you use a template for the MUST's and SHOULD's? The reason I usually only list ones I find a problem with is because it seems a lot of work to list stuff that's good/OK.

Thanks,
Richard

(In reply to comment #11)
> Here's the revised sources:

Okay, looks good.  I'll approve this, although actually building will depend on the availability of tepache, of course.

> On a side note, do you use a template for the MUST's and SHOULD's? The reason I
> usually only list ones I find a problem with is because it seems a lot of work
> to list stuff that's good/OK.

Yes, I've taken to cutting and pasting that list into reviews.  That way, if there is any question about whether a particular item was covered, or if the review list changes, it's possible to go back and say, "Yes, I did cover that point" or "No, I did not cover that point".

New Package SCM Request
=======================
Package Name: pysdm
Short Description: Python based Storage Device Manager
Owners: hobbes1069
Branches: f14 f15
InitialCC:

Git done (by process-git-requests).

pysdm-0.4.1-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/pysdm-0.4.1-2.fc14

pysdm-0.4.1-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/pysdm-0.4.1-2.fc15

pysdm-0.4.1-2.fc15 has been pushed to the Fedora 15 testing repository.

pysdm-0.4.1-2.fc15 has been pushed to the Fedora 15 stable repository.

pysdm-0.4.1-2.fc14 has been pushed to the Fedora 14 stable repository.