Red Hat Bugzilla – Bug 438588
Review Request: zfstream - C++ iostream like access to compressed files
Last modified: 2009-05-09 00:17:15 EDT
Spec URL: http://sailer.fedorapeople.org/zfstream.spec SRPM URL: http://sailer.fedorapeople.org/zfstream-20041202-1.fc8.src.rpm Description: zfstream provides a library to access compressed files (gzip, bzip2, single file contained in a ZIP archive) using a C++ iostream compatible interface.
I noticed the following troubling rpmlint complaints; it looks like this library isn't linked against libbz2 but is linked against libm even though it doesn't need to be. The latter is easily fixed with the trick in the unused-direct-shlib-dependency section of http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues. zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 inflateEnd zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 gzopen zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 gzclose zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 deflate zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 BZ2_bzwrite zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 inflateInit2_ zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 inflate zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 BZ2_bzopen zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 gzwrite zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 crc32 zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 deflateEnd zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 BZ2_bzclose zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 deflateInit2_ zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 gzread zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 BZ2_bzread zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 get_crc_table zfstream.x86_64: W: undefined-non-weak-symbol /usr/lib64/libzfstream.so.0.0.0 BZ2_bzerror zfstream.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libzfstream.so.0.0.0 /lib64/libm.so.6
What tool versions did you use? I just tried to compile it on a freshly installed F9 system with all released updates applied, and it works. The following packages were used: libtool-1.5.24-6.fc9.i386 automake-1.10.1-2.noarch autoconf-2.61-10.fc9.noarch binutils-2.18.50.0.6-2.i386 gcc-4.3.0-8.i386
FYI, this is all still present in current rawhide. Please note that in order to see these complaints, you must install the package and then run "rpmlint zfstream". I don't think any of this is x86_64-dependent.
Ping?
The actual description in the Spec does not match with the one in the review. In any case, I don't think it is relevant to include the names of upstream developers in the description. Use a separate file in %doc if need be.
http://sailer.fedorapeople.org/zfstream.spec http://sailer.fedorapeople.org/zfstream-20041202-2.fc10.src.rpm New %description: zfstream is a small C++ library which provides an abstraction API for reading and writing compressed and non-compressed files using the same API. It supports libz and libbz2 compression schemes. The library is trivial to use and provides client applications with a unified interface for reading and writing files without having to know whether they are compressed or not. On my current x86_64 rawhide system: rpmlint zfstream-20041202-2.fc10.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. pmlint zfstream.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint zfstream-20041202-2.fc10.x86_64.rpm. zfstream-devel-20041202-2.fc10.x86_64.rpm zfstream-debuginfo-20041202-2.fc10.x86_64.rpm zfstream.x86_64: W: no-documentation zfstream-devel.x86_64: W: no-documentation zfstream-debuginfo.x86_64: E: non-standard-dir-perm /usr/lib/debug 0775 3 packages and 0 specfiles checked; 1 errors, 2 warnings. rpmlint zfstream zfstream.x86_64: W: no-documentation I don't know what to do about the debuginfo error, as the debuginfo package is generated automatically and I don't do anything evil in the spec file, afaik.
I wouldn't worry about the non-standard-dir-perm thing; I believe it simply has to do with your umask. I don't see it here. I have to ask, though, where does zfstream-autotools.tar.gz come from? It seems to contain several additional source files and such. What is the upstream status of all that code? We strongly discourage significantly forking upstream source within a package; if you want to fork, do it properly and set up some kind of project hosting.
zfstream-autotools.tar.gz has been written by me. Yes, it has been sent upstream, but the author won't take it, as he is also the author of toc, the original build system. I tried half a day to get toc working, without success. Even if I got it to work, it would unlikely have worked and picked the correct compiler flags etc. for any architecture I don't own (i.e. everything but x86). So it seemed far less work to just drop in autotools scripts. And autotool is working and maintained for pretty much all architectures fedora supports, so it seems to me that with autotools build scripts, I will have a better chance of getting the packages to compile and run on other architectures as well. See the similar discussion on: https://bugzilla.redhat.com/show_bug.cgi?id=438587 BTW: I do not intend to actually fork the project source code, just replace the build system, as it proved unworkable for me. I will send source code changes upstream, and the author will apply them. He's just not keen on replacing the build system.
So you're saying that all of the .c, .cpp and .h files in zfstream-autotools.tar.gz are part of the build system? That seems difficult to believe. Obviously if toc doesn't work we have to do something. I'm surprised that someone thinks autotools is an improvement over anything, but so be it. Still, adding new source goes beyond a simple buildsystem change.
autotools may be crap, but at least it's known crap 8-) If you have an example of a Makefile, that builds a shared library and works on all supported/soon to be supported architectures (including mingw), then I'd be interested! The archive also contains the source code for supporting zip files (in addition to gz and bz2). This code has been sent upstream, and the author agreed (on Jan 2008) to include it, but apparently hasn't gotten around it. I will ping again...
Here are my notes for this package: - Package builds fine in koji rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=1329697 * rpmlints: zfstream-devel.x86_64: W: no-documentation zfstream.x86_64: W: no-documentation The file ChangeLog can go to %doc. I don't think it is worth packaging the LICENSE file as it doesn't say much about the license. * Patches should be explained and be submitted to upstream. Since the project seems dead, it won't hurt to bypass the submission part. But please explain in the SPEC file what the patch does. * Similarly, please give the reasoning of Source1. * Please remove the duplicate copy of minizip, which we already have in Fedora, from Source1 and adjust the BR's. Note that minizip-devel already requires zlib-devel. * The package must be named according to the Package Naming Guidelines. The upstream tarball has a different name. Why don't we use it? * No need to explicitly BR automake. libtool does and always will pull that for you. ! Please make use of the %{name} macro. * The devel package must require "bzip2-devel", "zlib-devel or minizip-devel (?)", and "pkgconfig" ? About "touch NEWS README AUTHORS": Wasn't there an --add-missing flag to automake? ! No need for blank %doc's. ! Please make the descriptions span 80 columns.
Update Spec URL: http://sailer.fedorapeople.org/zfstream.spec SRPM URL: http://sailer.fedorapeople.org/zfstream-20041202-3.fc11.src.rpm (In reply to comment #12) > The file ChangeLog can go to %doc. I don't think it is worth packaging the > LICENSE file as it doesn't say much about the license. Done and agreed. > * Patches should be explained and be submitted to upstream. Since the project > seems dead, it won't hurt to bypass the submission part. But please explain in > the SPEC file what the patch does. I have sent the patch upstream by PM (the project lacks a bug tracker). While the original author said he integrated the patch into his tree, he apparently hasn't gotten around to release a new version. > * Similarly, please give the reasoning of Source1. See comment #9. Basically because I couldn't get toc working, and furthermore allows autotools to cross-compile the lib (I have also submitted mingw32-zfstream for review). > * Please remove the duplicate copy of minizip, which we already have in Fedora, > from Source1 and adjust the BR's. Note that minizip-devel already requires > zlib-devel. Done. > * The package must be named according to the Package Naming Guidelines. The > upstream tarball has a different name. Why don't we use it? The project is named just "zfstream" on the author's homepage. It's just that the tarball has libs11n_ prefixed. s11n.net is another project by the same author, so I guess that's where the prefix comes from. zfstream however has nothing to do with s11n.net, so I found it more natural to just name the package zfstream. > * No need to explicitly BR automake. libtool does and always will pull that for > you. Done > ! Please make use of the %{name} macro. Done. > * The devel package must require "bzip2-devel", "zlib-devel or minizip-devel > (?)", and "pkgconfig" Done. I think zlib-devel is enough, as zipstream.hpp does not include anything from minizip-devel, minizip is only needed for building > ? About "touch NEWS README AUTHORS": Wasn't there an --add-missing flag to > automake? Actually, -a (which is already present) or --add-missing (the full name for -a) only adds missing files _other_ than NEWS, README and AUTHORS. So build will fail if you remove the touch. > ! No need for blank %doc's. Done. > ! Please make the descriptions span 80 columns. Shortened.
Thanks for adding the source file explanations to the specfile. Everything looks good, except > > ! Please make the descriptions span 80 columns. > > Shortened. I don't see any change. In fact, you shouldn't shorten them. Some lines in the description are even smaller than 70 columns. They need stretched out a little. But this is not a blocker. You can do it before you commit ------------------------------------------- This package (zfstream) is APPROVED by oget -------------------------------------------
(In reply to comment #14) > I don't see any change. In fact, you shouldn't shorten them. Some lines in the > description are even smaller than 70 columns. They need stretched out a little. Ah sorry, I misunderstood what you meant. Will fix before checkin, thanks!
Thank you for the review. New Package CVS Request ======================= Package Name: zfstream Short Description: C++ iostream like access to compressed files Owners: sailer Branches: F-10 F-11 InitialCC:
CVS Done
zfstream-20041202-5.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/zfstream-20041202-5.fc11
zfstream-20041202-5.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/zfstream-20041202-5.fc10
zfstream-20041202-5.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
zfstream-20041202-5.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.