Spec Name or Url: http://znark.com/fedora/extras/jigdo.spec SRPM Name or Url: http://znark.com/fedora/extras/jigdo-0.7.2-1.src.rpm Description: Jigsaw Download, or short jigdo, is a tool designed to ease the distribution of very large files over the internet, for example CD or DVD images. Its aim is to make downloading the images as easy for users as a click on a direct download link in a browser, while avoiding all the problems that server administrators have with hosting such large files. It accomplishes this by using the separate pieces of any big file (such as the files contained within a CD/DVD image) to create a special "template" file which makes reassembly of the big file very easy for users who only have the pieces. This is my first package and I need a sponsor.
Not a review, but you've got some duplicate BuildRequires. According to fedora-qa script: Duplicate BuildRequires: zlib-devel (by openssl-devel), openssl-devel (by curl-devel), atk-devel (by gtk2-devel), glib2-devel (by gtk2-devel), pango-devel (by gtk2-devel), XFree86-devel (by gtk2-devel), freetype-devel (by pango-devel)
Here's a review: MUST items: OK Package name. OK Spec file name. OK License good (GPL). OK License in spec matches. OK License is included (GPL COPYING) OK Spec english/readable OK Sources match upstream (md5: 031756ff6c7084a139dc9550a27f6906) OK No packages in BR that are in the exceptions list. OK Correctly uses find_lang OK No dynamic libs, so no ldconfig needed. OK Builds on fc4. OK No rpmlint output (on fc4 build). Good! OK Package owns all it's created directories. OK No duplicate files in files section. OK Permissions look good. OK Clean section looks good. OK desktop file looks good and good install. OK Runs fine on my fc4 machine. ISSUES: 1. Doesn't build in current devel/mock. It has two unavailable BuildRequires: No Package Found for w3c-libwww-devel >= 0:5.3.2 No Package Found for XFree86-devel The w3c-libwww package no longer seems available in core in devel/fc5. XFree86-devel has been replaced with modular xorg packages. You may need to create and import a w3c-libwww package before this one if that support is needed by this package. Can you see if that support is optional, and/or some new package now provides that support for fc5? Also, if you could review/comment on some other in review packages to show your understanding of the package guidelines, that would assist in deciding to sponsor you. ;) You can find a list in bugzilla at: https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-REVIEW&hide_resolved=1
w3c-libwww is bug 178310
I modified the spec to remove the duplicate BuildRequires. Removing XFree86-devel means the same spec builds on devel after the w3c-libwwww-devel in 178310 is installed. I added a patch to build on gcc 4.1. Spec: http://znark.com/fedora/extras/jigdo.spec SRPM: http://znark.com/fedora/extras/jigdo-0.7.2-2.src.rpm
The specfile shows that it owns everything in /usr/share/applications (%{_datadir}/applications/*), as well as all of the standard binaries (%{_bindir}/*), which is probably very bad. ;-) Also, your %files section contains: %dir %{_datadir}/%{name} %{_datadir}/%{name}/* Wouldn't simply owning the directory be simpler? %{_datadir}/%{name} My two cents.
>The specfile shows that it owns everything in /usr/share/applications >(%{_datadir}/applications/*), as well as all of the standard binaries >(%{_bindir}/*), which is probably very bad. ;-) Those globs are expanded at build time under the $RPM_BUILD_ROOT, so they only match files that are installed in the %install step of the rpm building. :) In this case they expand to: /usr/share/applications/fedora-jigdo.desktop /usr/bin/jigdo /usr/bin/jigdo-file /usr/bin/jigdo-lite Which is fine. Some folks like using * in files to match a bunch of files, while others prefer to list each file out. It's just a matter of taste as long as there are no duplicate files listed or files/dirs that are owned by another package already.
I thought just listing the directory would work. And it did work for building on FC4. But when I built the package on rawhide, it complained about the files in usr/shared/jigdo not being included. So I put in the glob.
(In reply to comment 6) Ack. I had not realized that. Thanks, Kevin!
Hey Ian. Can you build against the latest w3c-libwww from https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=178310 and confirm that the package still works as expected?
w3c-libwww is now in extras. Can you confirm that this package functions as expected compiled against that package? In the gcc41 patch you move the outOfMemory function around. Is that really needed? I don't see any changes made to the function, just moving it.
Sorry about taking so long to respond. I just discovered that jigdo doesn't use w3c-libwww. I hadn't noticed until that the binary package didn't depend on w3c-libwww package. It looks like upstream replaced using libwww with libcurl in version 0.7.2. I just rebuilt it without the BuildRequires on w3c-libwww-devel and the result works. I posted the new spec and srpm. http://znark.com/fedora/extras/jigdo.spec http://znark.com/fedora/extras/jigdo-0.7.2-1.src.rpm The change is actually moving the cmdOptions method and the enum out of the local namespace block while keeping outOfMemory inside the namespace. GCC 4.1 seems to have stricter rules about namespaces.
Sorry for the delay in getting back to you on this review. This version looks good. The removal of the w3c-libwww-devel BR looks good. The namespace move makes sense. GCC 4.1 seems stricter in general on g++. Everything looks good. This package is APPROVED. I would be happy to sponsor you. Please continue the process from the "Get a Fedora Account" section at: http://fedoraproject.org/wiki/Extras/Contributors If you have any questions at all, please don't hesitate to drop me an email.
Built for extras devel.