Bug 515230

Here's some quick review commentary.

This builds fine; rpmlint says:

  python-dmidecode.x86_64: E: explicit-lib-dependency libxml2
rpm finds the delepdency on libxml2.so.2 itself; there's no need to specify it manually.

  python-dmidecode.x86_64: E: explicit-lib-dependency libxml2-python
This one seems bogus; ignore it.

  python-dmidecode.x86_64: W: summary-not-capitalized python extension module 
   to access DMI data
Trivial to fix.

  python-dmidecode.x86_64: W: no-documentation
There are several documentation files which should probably be packaged.  It is mandatory that you package at least the license file.

Source0: should be a full URL to the tarball.  If there is no URL where that tarball can be downloaded, where did you get it?  Please see http://fedoraproject.org/wiki/Packaging:SourceURL.

No supported version of Fedora shipped with a python older than 2.5.2; are you sure the %{python_ver} conditional is needed?  (Likewise, no supported Fedora version needs a BuildRoot: tag or the rm line at the beginning of %install.)

Nothing seems to own /usr/share/python-dmidecode.

Thank you for your review!

A new src.rpm and spec file is available:

Spec URL: http://projects.autonomy.net.au/python-dmidecode/export/66ddffc29a5a99f847a8ec8f76f1adbe37c481f6/contrib/python-dmidecode.spec
SRPM URL: http://src.autonomy.net.au/python-dmidecode/python-dmidecode-3.10.7-1.fc11.src.rpm

rpmlint gives now no warnings or errors at all.

Just a comment in regards to the Python check against older versions than 2.5.  This package is also being built on RHEL4 and RHEL5, which ships Python 2.3 and 2.4.  That's the reason for this check, as we want to try to avoid maintaining a separate .spec file for RHEL.  Is this going to be a problem?

(Removing NEEDSPONSOR)

Some notes:

* %define -> %global
  - Now Fedora prefers to use %global instead of %define:
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

* Source
  - Source0 in your srpm differs from what I could download from the URL
    written as %SOURCE0:
---------------------------------------------------------------
97986 2009-09-23 18:24 python-dmidecode-3.10.7.tar.gz
97487 2009-09-23 18:25 python-dmidecode-3.10.7-1.fc11.src/python-dmidecode-3.10.7.tar.gz
---------------------------------------------------------------

* Requires
  - (Here I am speaking of Requires, not BuildRequires)
    dmidecode.py contains:
---------------------------------------------------------------
    28  import libxml2
    29  from dmidecodemod import *
---------------------------------------------------------------
    This means that this package should have "Requires: libxml2-python".

* Parallel make
  - Support parallel make if possible:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make

* Macros
  - Use macros properly. /usr/share should be %{_datadir}.
    https://fedoraproject.org/wiki/Packaging/RPMMacros

* Directory ownership issue
  - The directory /usr/share/python-dmidecode/ must be owned by this
    package:
    https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership

Thanks again for another review!

First of all, I'm sorry for the mix up between the .src.rpm and the .spec file.  I'm not sure how that has happened.  I'm not in direct control over that web space, so I've posted new versions where I have full control over the files.  The community website will be updated again when the review is completed.

Spec URL: http://people.redhat.com/~dsommers/python-dmidecode/python-dmidecode.spec
SRPM URL: http://people.redhat.com/~dsommers/python-dmidecode/python-dmidecode-3.10.7-2.fc11.src.rpm


In regards to the "Requires" section, this contradicts the message in comment #1, where Jason Tibbits says: "rpm finds the delepdency on libxml2.so.2 itself; there's no need to specify it manually."  And now, rpmlint do give an error:

python-dmidecode.x86_64: E: explicit-lib-dependency libxml2-python

I've added it, according to your request, but I am willing to take it away if this is not correct.

* Parallel make
I've looked into this.  As the building is done via python setup.py/Distutil, I don't see any possibility to support parallel building.  Therefore, I find it rather misleading adding %{_smp_mflags} to the make command.  I hope you don't mind me that I have not modified this.

* Macros
Fixed!

* Directory ownership issue
Fixed!

For -2:

* %define -> %global
  - I meant here:
-------------------------------------------------------------
     1  %{!?python_sitearch: %define python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
                             ^^here^^
     2  %{!?python_ver: %define python_ver %(%{__python} -c "import sys ; print sys.version[:3]")}
                        ^^here^^
-------------------------------------------------------------

* Tarball
(In reply to comment #6)
> First of all, I'm sorry for the mix up between the .src.rpm and the .spec file.
>  I'm not sure how that has happened.  I'm not in direct control over that web
> space, so I've posted new versions where I have full control over the files. 
> The community website will be updated again when the review is completed.

  - Well, is my recognition correct that you are explaining here why
    the source tarball changed? (by the way the tarball changed again).

* Duplicate files
  - Now build.log complains:
-------------------------------------------------------------
   157  warning: File listed twice: /usr/share/python-dmidecode/pymap.xml
-------------------------------------------------------------
    Note that %files entry:
-------------------------------------------------------------
%files
foo/
-------------------------------------------------------------
    (where foo/ is a directory) contains the directory foo/ itself and
    all files/directories/etc under foo/:
    https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Unversioned

!! Requires
(In reply to comment #6)
> In regards to the "Requires" section, this contradicts the message in comment
> #1, where Jason Tibbits says: "rpm finds the delepdency on libxml2.so.2 itself;
> there's no need to specify it manually."  And now, rpmlint do give an error:
> 
> python-dmidecode.x86_64: E: explicit-lib-dependency libxml2-python
> 
> I've added it, according to your request, but I am willing to take it away if
> this is not correct.

  - Well, Jason actually said the correct thing (and it has no contradiction with
    my comment).

    Actually
    - "Requires: libxml2" is not needed (and should be removed) because rpmbuild
      actomatically detects library related dependency (in this case libxml2.so.2)
      and adds such dependency into binary rpms automatically, which pulls
      libxml2 in when trying to install this package by yum.

    - However "Requires: libxml2-python" is needed because rpmbuild cannot detect
      such python module based dependency.
      Now rpmlint throws out the error message you pointed out, because the
      package name "libxml2-python" contains "lib" string.
      However "this one (rpmlint) is bogus; ignore it." (as Jason said).


> * Parallel make
> I've looked into this.  As the building is done via python setup.py/Distutil, I
> don't see any possibility to support parallel building.  Therefore, I find it
> rather misleading adding %{_smp_mflags} to the make command.  I hope you don't
> mind me that I have not modified this.
  + Okay.

(In reply to comment #7)
> For -2:
> 
> * %define -> %global
>   - I meant here:
> -------------------------------------------------------------
>      1  %{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
>                              ^^here^^
>      2  %{!?python_ver: %define python_ver %(%{__python} -c "import sys ; print
> sys.version[:3]")}
>                         ^^here^^
> -------------------------------------------------------------

Ouch!  I oversaw that one.  Sorry about that.


> * Tarball
> (In reply to comment #6)
> > First of all, I'm sorry for the mix up between the .src.rpm and the .spec file.
> >  I'm not sure how that has happened.  I'm not in direct control over that web
> > space, so I've posted new versions where I have full control over the files. 
> > The community website will be updated again when the review is completed.
> 
>   - Well, is my recognition correct that you are explaining here why
>     the source tarball changed? (by the way the tarball changed again).

The rpms are created by calling 'make rpm' in the source tree.  This creates a directory and copy over the needed files and tar is down to a new tarball.  This tarball is then used for the rpmbuild which is called via make.  When we did this round the last time with the upstream community site, the version numbers did not match complete between the tarball and the spec file, and for some reason I believe an older src.rpm turned up in the community website than what I anticipated.  Anyhow, I'm using my space at people.redhat.com now, to avoid any issues.

> 
> * Duplicate files
>   - Now build.log complains:
> -------------------------------------------------------------
>    157  warning: File listed twice: /usr/share/python-dmidecode/pymap.xml
> -------------------------------------------------------------
>     Note that %files entry:
> -------------------------------------------------------------
> %files
> foo/
> -------------------------------------------------------------
>     (where foo/ is a directory) contains the directory foo/ itself and
>     all files/directories/etc under foo/:
>     https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Unversioned

Fixed.  Unfortunately, I missed that one when reading through the build log yesterday.

> !! Requires
> (In reply to comment #6)
> > In regards to the "Requires" section, this contradicts the message in comment
> > #1, where Jason Tibbits says: "rpm finds the delepdency on libxml2.so.2 itself;
> > there's no need to specify it manually."  And now, rpmlint do give an error:
> > 
> > python-dmidecode.x86_64: E: explicit-lib-dependency libxml2-python
> > 
> > I've added it, according to your request, but I am willing to take it away if
> > this is not correct.
> 
>   - Well, Jason actually said the correct thing (and it has no contradiction
> with
>     my comment).
> 
>     Actually
>     - "Requires: libxml2" is not needed (and should be removed) because
> rpmbuild
>       actomatically detects library related dependency (in this case
> libxml2.so.2)
>       and adds such dependency into binary rpms automatically, which pulls
>       libxml2 in when trying to install this package by yum.
> 
>     - However "Requires: libxml2-python" is needed because rpmbuild cannot
> detect
>       such python module based dependency.
>       Now rpmlint throws out the error message you pointed out, because the
>       package name "libxml2-python" contains "lib" string.
>       However "this one (rpmlint) is bogus; ignore it." (as Jason said).

Ahh got it!  Okay, then this thing is fixed.  I've not done any changes here.


Let's hope this version makes you happier :)


Spec URL: http://people.redhat.com/~dsommers/python-dmidecode/python-dmidecode.spec
SRPM URL: http://people.redhat.com/~dsommers/python-dmidecode/python-dmidecode-3.10.7-3.fc11.src.rpm

Okay.

-----------------------------------------------------------
  This package (python-dmidecode) is APPROVED by mtasaka
-----------------------------------------------------------

New Package CVS Request
=======================
Package Name: python-dmidecode
Short Description: Python module to access DMI data
Owners: dsommers
Branches: F-11 F-12 EL-4 EL-5
InitialCC:

CVS done.

Closing.