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: nobody@fedoraproject.org ReportedBy: davids@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com 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.
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 davids@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
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 tibbs@math.uh.edu 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.
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 davids@redhat.com 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.
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 davids@redhat.com 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?
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) |
--- Comment #4 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-11-16 11:14:34 EDT --- (Removing NEEDSPONSOR)
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |mtasaka@ioa.s.u-tokyo.ac.jp Flag| |fedora-review?
--- Comment #5 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 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...
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 davids@redhat.com 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!
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 mtasaka@ioa.s.u-tokyo.ac.jp 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.
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 davids@redhat.com 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 "fromdistutils.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) becauserpmbuild 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 cannotdetect 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-...
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #9 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-11-26 11:02:31 EDT --- Okay.
----------------------------------------------------------- This package (python-dmidecode) is APPROVED by mtasaka -----------------------------------------------------------
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 davids@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #10 from David Sommerseth davids@redhat.com 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:
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 tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #11 from Jason Tibbitts tibbs@math.uh.edu 2009-12-01 13:46:54 EDT --- CVS done.
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
--- Comment #12 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-12-02 03:22:32 EDT --- Closing.
package-review@lists.fedoraproject.org