[Bug 515230] New: Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: python-dmidecode - python extension module to access DMI data

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

Summary: Review Request: python-dmidecode - python extension
                    module to access DMI data
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora
    Target Release: ---

This is my first Fedora package, so I presume I'll need a sponsor for this
package

Spec URL:
http://projects.autonomy.net.au/python-dmidecode/export/392a9976b14d558d4b2f...
SRPM URL:
http://src.autonomy.net.au/python-dmidecode/python-dmidecode-3.10.6-6.fc11.s...

Description: python-dmidecode is a python extension module that uses the
code-base of the 'dmidecode' utility, and presents the data
as python data structures or as XML data using libxml2.

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

David Sommerseth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841(FE-NEEDSPONSOR)

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #1 from Jason Tibbitts   2009-09-22 21:19:02 EDT ---
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.

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #2 from David Sommerseth   2009-09-23 05:32:00 EDT ---
Thank you for your review!

A new src.rpm and spec file is available:

Spec URL:
http://projects.autonomy.net.au/python-dmidecode/export/66ddffc29a5a99f847a8...
SRPM URL:
http://src.autonomy.net.au/python-dmidecode/python-dmidecode-3.10.7-1.fc11.s...

rpmlint gives now no warnings or errors at all.

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #3 from David Sommerseth   2009-09-23 05:34:56 EDT ---
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?

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841(FE-NEEDSPONSOR)      |

--- Comment #4 from Mamoru Tasaka   2009-11-16 11:14:34 EDT ---
(Removing NEEDSPONSOR)

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #5 from Mamoru Tasaka   2009-11-24 12:19:07 EDT ---
Some notes:

* %define -> %global
  - Now Fedora prefers to use %global instead of %define:

https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over...

* 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_Owner...

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #6 from David Sommerseth   2009-11-25 10:06:28 EDT ---
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-...

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!

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #7 from Mamoru Tasaka   2009-11-25 12:13:52 EDT ---
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.

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #8 from David Sommerseth   2009-11-26 09:49:13 EDT ---
(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-...

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #9 from Mamoru Tasaka   2009-11-26 11:02:31 EDT ---
Okay.

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

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

David Sommerseth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #10 from David Sommerseth   2009-11-27 11:15:35 EDT ---
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:

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Jason Tibbitts  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #11 from Jason Tibbitts   2009-12-01 13:46:54 EDT ---
CVS done.

[Bug 515230] Review Request: python-dmidecode - python extension module to access DMI data

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

--- Comment #12 from Mamoru Tasaka   2009-12-02 03:22:32 EDT ---
Closing.