Red Hat Bugzilla – Bug 249352
Review Request: popt - C library for parsing command line parameters
Last modified: 2007-11-30 17:12:11 EST
Spec URL: http://labs.linuxnetz.de/bugzilla/popt.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/popt-1.12-1.src.rpm Description: Popt is a C library for parsing command line parameters. Popt was heavily influenced by the getopt() and getopt_long() functions, but it improves on them by allowing more powerful argument expansion. Popt can parse arbitrary argv[] style arrays and automatically set variables based on command line arguments. Popt allows command line arguments to be aliased via configuration files and includes utility functions for parsing arbitrary strings into argv[] arrays using shell-like rules.
I'm assuming it's intended to split this out of rpm?
Yes, same like mutt <-> urlview or gmp <-> mpfr or similar.
Bill: Yes, the intent is to split it out, having a separate library with it's own versioning coming out of rpm package is a huge pita and I want to see this done in time for F8. But this needs to be carefully coordinated to avoid making a total mess as rpm is quite intimately married to popt, hold it for a moment, need to discuss this with Paul first.
Panu: Well, splitting popt out of rpm is not that huge problem as it currently looks like to you. Finally popt ever had it's own versioning (just read the popt changelog inside of the popt directory).
Didn't say it was a huge problem, just that it needs to be coordinated.
*** Bug 203538 has been marked as a duplicate of this bug. ***
Also popt-devel will have to be added to a lot of BR, unless my rough analysis in Bug 203538 is incorrect.
Correct, initscripts and mkinitrd (nash) would be two important ones except rpm. IIRC static linking is required for initscripts, maybe I'm wrong, just what my brain thinks to know ;)
There are a LOT of packages depending on popt actually: [pmatilai@localhost ~]$ repoquery --quiet --whatrequires --alldeps --queryformat "%{sourcerpm}" popt|sort -u|wc -l 297 A fair amount of duplicate versions in that number but it's > 200 packages anyway. They haven't needed the explicit BuildRequires on popt because main popt package gets pulled in by rpm always.
The transition path could be eased by having rpm provide explicitly popt-devel, tell on the list that all packages that use popt should BuildRequires popt-devel, wait a bit that most important packages have been rebuilt then split out popt-devel (either in rpm on as part of popt).
This spec installs libpopt to /usr/lib(64) instead of /lib(64), see #249814
Created attachment 160808 [details] Suggested spec tweaks I'd suggest the attached patch to the spec: 1) move the library to /lib[64] as per bug #249814 2) generate and include api documentation Whether a separate subpackage is needed for 2) is another question, it could of course be stuffed into -devel just as well.
(In reply to comment #12) > Created an attachment (id=160808) [edit] > Suggested spec tweaks > > I'd suggest the attached patch to the spec: > 1) move the library to /lib[64] as per bug #249814 I am opposed to this change, because this would put devel-libs into /lib. Instead I'd suggest that you apply the same approach as glib2 does: Put the devel libs into /usr/lib, put the run-time-libs into /lib and add a couple of symlinks inbetween.
Right, stuffing devel libs into /lib was indeed a stupid oversight on my behalf :)
Ralf, thanks for this hint. Panu, thanks for your inital patch, I'll merge today your patch with Ralfs last comment into a -2 and push to the same base URL as my initial review request. When done, URLs will get added.
Yup. Once the /lib move has been done I think it's ok to proceed with the review. I've done a bit of testing with rpm 4.4.2.1 and didn't see any problems with the newer popt. Some of points worth noting: - rpm needs to loose the strict dependency on popt = <version>-<release> (will do) - fedora-maintainers + fedora-devel-announce needs to be notified because the amount of implicit build dependencies on popt-devel - once this package hits rawhide, rpm needs to be modified to build against external popt and kill of the internal version entirely (will do) - I want co-maintainership for the new popt package, just so I know what's going on with it due to the close marriage with rpm (not that I expect much to happen, popt isn't that actively developed)
The License Tag does not math current Guidelines: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines - it needs to have a value that is mentioned in http://fedoraproject.org/wiki/Licensing It seems to be a MIT License "Modern Style with sublicense": http://fedoraproject.org/wiki/Licensing/MIT So the License-tag should be "MIT" instead of "X Consortium". But I do not know, whether or not it is a problem that, test2.c is (L)GPL licensed according to its header, but I guess it is not distributed in the binary rpm. Also some files are missing a license header (test3.c, popint.c and system.h)
Panu, does rpm.org's rpm depend on the libpopt.la file? Can you please verify, whether rebuilding without works or shall we simply keep this file? IIRC rpm is the only package which at least was depending on this file.
(In reply to comment #18) > Panu, does rpm.org's rpm depend on the libpopt.la file? No, rpm.org's hg mainline doesn't > Can you please verify, > whether rebuilding without works or shall we simply keep this file? At least, rpm.org'd hg mainline doesn't.
No need to package the .la file, even 4.4.2.1 appears to builds ok without it.
Oh and btw this part is now done in latest rawhide rpm to remove silly testing roadblocks: > - rpm needs to loose the strict dependency on popt = <version>-<release>
The /lib move has been done, other suggested changes have been merged in. Can we walk through the official review now? Updated package below: Spec URL: http://labs.linuxnetz.de/bugzilla/popt.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/popt-1.12-2.src.rpm
It seems that the bugs in "depends on" were intended for "blocks", because one is for RHEL and may be fixed in this version and the other is more a feature request and may also be already fixed in the reviewed version. Please change this back in case I am wrong.
graphviz (or something else that provides dot) is missing in the BuildRequires, without it errors like this are shown: sh: dot: command not found Problems running dot: exit code=127, command='dot', Arguments='"poptint_8h__dep__incl.dot" -Tpng -o "poptint_8h__dep__incl.png"' Review: - rpmlint: ok (this is with BR on graphviz) E: popt-devel zero-length /usr/share/doc/popt-devel-1.12/html/popt_8h__incl.map This is created by doxygen, imho there is no need to remove it manually. W: popt-static no-documentation This is no problem, too. - naming: ok - spec file readability: ok - source: ok f45290e9ac4b1cf209d0042eb6755543 /tmp/popt-1.12.tar.gz f45290e9ac4b1cf209d0042eb6755543 popt-1.12.tar.gz - mockbuild i386 F7: ok - BuildRequires: NEEDSWORK (add graphviz) - ldconfig scriptlets: ok - Directory Ownership: ok - %clean section: ok - macro usage: ok - doc: large api doc is in devel, ok - devel subpackage with headers: ok - static subpackage: ok - .so file in -devel: ok - subpackage requires: ok - .la removed: ok - correct %install - license included: ok - Buildroot: ok Add graphviz to BR before you import the package and it is APPROVED
New Package CVS Request ======================= Package Name: popt Short Description: C library for parsing command line parameters Owners: redhat-bugzilla@linuxnetz.de,pmatilai@redhat.com For the guy doing the CVS: /me is maintainer, Panu is co-maintainer. Only development branch (upcoming F8), please.
Could you please provide this in the new template format? http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure Specifically, we need the FAS account names.
Okay Warren, here we go again: New Package CVS Request ======================= Package Name: popt Short Description: C library for parsing command line parameters Owners: robert,pmatilai Branches: InitialCC: Cvsextras Commits: no
cvs done.
popt 1.12-3 was sucessfully built for Rawhide in Koji.