Fedora Merge Review: hwdata http://cvs.fedora.redhat.com/viewcvs/devel/hwdata/ Initial Owner: karsten
hwdata-0.196-1 has a fixed buildroot
Missing items from SPEC file:- 1) Add disttag (Not necessary but its good to have it in SPEC) 2) You need to replace following line under %files section %config /usr/share/hwdata/* with /usr/share/hwdata/* Or you can move those files to /etc and add following line %config(noreplace) /usr/share/hwdata/* Above is necessary to make rpmlint output silent. However its ok to have following warning reported by rpmlint W: hwdata no-url-tag
I've added the disttag, but I disagree with 2) The packaging guidelines don't mandate that every config file needs to be in /etc The files in /usr/share/hwdata/ are config files and should be labeled as such. They can't be moved to /etc without breaking lots of other packages which need to read from those files. I'd like to avoid using a symlink as a) packages using the /usr/share/hwdata will break when /usr isn't mounted and they check only for files, not for directories b) if I need to revert that later it'll be almost impossible with rpm as rpm can't replace symlinks with directories.
(In reply to comment #3) > I've added the disttag, but I disagree with 2) > The packaging guidelines don't mandate that every config file needs to be in /etc > The files in /usr/share/hwdata/ are config files and should be labeled as such. > > They can't be moved to /etc without breaking lots of other packages which need > to read from those files. I'd like to avoid using a symlink as > a) packages using the /usr/share/hwdata will break when /usr isn't mounted and > they check only for files, not for directories > b) if I need to revert that later it'll be almost impossible with rpm as rpm > can't replace symlinks with directories. Ok. That will be enough explanation for me :) We can then let rpmlint output as not silent. Will do Full review tomorrow.
I strongly disagree that it is acceptable to have config files in /usr. In fact /usr should be mountable read only. And it is in the guidelines, since we are bound to follow the FHS: http://fedoraproject.org/wiki/Packaging/Guidelines#head-e1c5548cbbe551c7a43d375c524ab2ea0188557e Now it may be so hard to put things in /etc that it isn't doable, but in my opinion it is mandatory.
karsten, I think Patrice is right.
In case you have %config and %config(noreplace) in your SPEC then You may like to update the SPEC by removing that. Check http://fedoraproject.org/wiki/Packaging/Minutes20070313 and http://fedoraproject.org/wiki/PackagingDrafts/UsrConfigs
I still don't agree, but the new packaging guidelines are clear about this. fixed in hwdata-0.199-1.fc7
Can you use macros in SPEC instead hard coded directory names? Packaging Guidelines said-> Use macros instead of hard-coded directory names (see Extras/RPMMacros).
any updates?
No response from maintainer
I've built hwdata-0.200-1 with updated pci.ids and use macros now
hmm. Still pending for review since 5 months. let me again take it for review. missing BR gawk. mock build gave me + make install DESTDIR=/var/tmp/hwdata-0.207-1.fc8-root-mockbuild make: awk: Command not found make: awk: Command not found make: awk: Command not found
also got rpmlint messages on SRPM as hwdata.src: W: no-%build-section The spec file does not contain a %build section. Even if some packages don't directly need it, section markers may be overridden in rpm's configuration to provide additional "under the hood" functionality, such as injection of automatic -debuginfo subpackages. Add the section, even if empty. ===> Add empty build section hwdata.src: W: invalid-license GPL/MIT The value of the License tag was not recognized. Known values are: "Adobe", "Affero GPL", "AFL", "ARL", "ASL 1.0", "ASL 1.0+", "ASL 1.1", "ASL 1.1+", "ASL 2.0", "ASL 2.0+", "APSL 2.0", "APSL 2.0+", "Artistic 2.0", "Artistic clarified", "BitTorrent", "Boost", "BSD", "BSD with advertising", "CeCILL", "CDDL", "CPL", "Condor", "Copyright only", "Cryptix", "Crystal Stacker", "EPL", "eCos", "EFL 2.0", "EFL 2.0+", "EU Datagrid", "Giftware", "Glide", "gnuplot", "GPL+", "GPL+ or Artistic", "GPLv2", "GPLv2 with exceptions", "GPLv2+", "GPLv3", "GPLv3+", "IBM", "IJG", "ImageMagick", "iMatix", "Intel ACPI", "Interbase", "ISC", "Jabber", "JasPer", "LGPLv2", "LGPLv2 with exceptions", "LGPLv2+", "LGPLv3", "LGPLv3+", "libtiff", "LPL", "LPPL", "mecab-ipadic", "MIT", "MPLv1.0", "MPLv1.0+", "MPLv1.1", "MPLv1.1+", "NCSA", "NGPL", "NOSL", "Netscape", "Nokia", "OpenLDAP", "OpenPBS", "OSL 1.0", "OSL 1.0+", "OSL 1.1", "OSL 1.1+", "OSL 2.0", "OSL 2.0+", "OSL 3.0", "OSL 3.0+", "OpenSSL", "Phorum", "PHP", "Public Domain", "Python", "QPL", "RPSL", "Ruby", "Sleepycat", "SISSL", "SLIB", "SPL", "TCL", "UCD", "Vim", "VNLSL", "VSL", "W3C", "WTFPL", "wxWindows", "xinetd", "Zend", "ZPLv1.0", "ZPLv1.0+", "ZPLv2.0", "ZPLv2.0+", "ZPLv2.1", "ZPLv2.1+", "zlib", "CDL", "FBSDDL", "GFDL", "IEEE", "OFSFDL", "Open Publication", "CC-BY", "CC-BY-SA", "DSL", "Free Art", "Arphic", "Bitstream Vera", "mplus", "OFL", "Utopia", "XANO", "Redistributable, no modification permitted", "Freely redistributable without restriction". ===> Add GPLv2+ as License
This is last call to you to provide updates here before going on fedora-devel
thanks for the reminder, somehow hwdata never manged to get to the top of my TODO list... I'll start on it right now: - license seems to be 'GPLv2+ and LGPLv2+' - empty %build section added to the git repository
so you built package already? I need cvs update to approve this review.
ping?
Sorry, I needed to leave early into the holiday season due to family reasons. I've pushed hwdata-0.208-1.fc9 to koji a minute ago.
SHOULD: As you are the upstream, With assumption that you will modify Makefile from for foo in $(FILES) ; do \ install -m 644 $$foo $(datadir)/$(NAME) ;\ to for foo in $(FILES) ; do \ install -p -m 644 $$foo $(datadir)/$(NAME) ;\ Above will preserve timestamps. APPROVED.