Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 179910
Summary: | Review Request: commoncpp2 - commoncpp C++ framework | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Andreas Thienemann <andreas> |
Component: | Package Review | Assignee: | Ville Skyttä <scop> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | jgrulich |
Target Milestone: | --- | Flags: | gwync:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-02-06 22:29:41 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Andreas Thienemann
2006-02-03 20:06:29 UTC
Note: The package name was changed after this bug was filed to commoncpp2. It seems the gnu people refer to the software as commoncpp, even though the tarball is named commoncpp2. As the library is named commoncpp2-*.so as well, this is probably a more sensible name. Hi Andreas, this isn't a full review (yet!), just a handful of observations: - please shorten "A portable and highly optimized class framework for writing C++ applications" to something shorter such as "A portable and optimized framework for C++ applications" - "nativily" --> "natively" - please delete all *.la files (per the packaging guidelines) - the "/usr/include/cc++2" dir is unowned More observations: - 1.3.23 is out - Most likely missing build dependencies: libxml2-devel, zlib-devel, doxygen - Missing dependency on pkgconfig in -devel - Missing install-info dependencies in -devel, "|| :" safeguards missing from install-info invocations (breaks --excludedocs installs) - Installation may try to modify /etc/ld.so.conf Feel free to borrow fixes for the above and whatever you find worth taking from my local package at http://cachalot.mine.nu/5/SRPMS/commoncpp2-1.3.23-v1.src.rpm Hi Ville, I think you have a much better idea whats happening here so, if you want, please go ahead and replace me as the reviewer. Otherwise, I'll do what I can to help the submitter get the good bits from your package incorporated into this one. 1.3.23 is out? hmmm. gonna update that then. The webpage referred to 1.3.22. About the build-dependencies, mock does build the package as is. I checked. The sagefuard is a good idea. The modification of ld.so.conf is taken care of, the Makefile.in is being patched accordingly. Thanks so far for the suggestions. I'm gonna update the package. Only thing I'm unsure about are the .la files. It seems they were being used by ccrtp. If so, removing them is probably not a good idea. (Reassigning to myself per comment 4) Sure, mock probably builds it without the build dependencies, but obviously leaves out zlib and libxml2 support. But if they're installed, the corresponding features are built in. So please either add the build dependencies or explicitly disable these features so that the build is reproducible. doxygen is a slightly different thing; with it installed the API docs are built into doc/html/ and I think it would be good to include them in the devel package (%doc doc/html). If you're planning to include the *.la, please show a real case where they are required (not "seems", "probably"...) Okay, Changes are incorporated. Take a look please at http://home.bawue.de/~ixs/commoncpp/commoncpp2.spec and http://home.bawue.de/~ixs/commoncpp/commoncpp2-1.3.23-2.src.rpm Blockers: - need fully versioned %{name} = %{version}-%{release} dependency in -devel. - need Requires: libxml2-devel and zlib-devel in -devel (see ccgnu2-config --stdlibs and --extlibs, and libccext2.pc, and some installed *.h) Suggestions: - configure with --disable-dependency-tracking for possibly faster builds and cleaner build output - Don't hardcode .gz in info filenames, install-info works without it and * can be used in %files - install-info is not a "Requires", it's "Requires(post)" and "Requires(preun)" - move %doc in -devel's %files after %defattr (actually, I'm a bit surprised that it being before %defattr doesn't cause problems with files being owned by the build owner here) Changes incorporated. http://helena.bawue.de/~ixs/commoncpp/ I'm a bit surprised though, to notice that the external dependencies were not taken up by rpm automagically. Looks good. thx for the review. as soon as this one is build, I'll put up the ccrtp for review. I'd be glad, if you could take alook as well. Normalize summary field for easy parsing Package Change Request ====================== Package Name: commoncpp2 New Branches: epel7 Owners: ixs Git done (by process-git-requests). |