Spec URL: http://files.thaumaturge.org/users/dante/rancid/rancid.spec SRPM URL: http://files.thaumaturge.org/users/dante/rancid/rancid-2.3.2a8-0.1.src.rpm Description: RANCID monitors a router's (or more generally a device's) configuration, including software and hardware (cards, serial numbers, etc) and uses CVS (Concurrent Version System) or Subversion to maintain history of changes. This is my first package and I'm looking for a sponsor. I'm not quite sure how this will work with the licensing. They are in the file "COPYING" and URL http://www.shrubbery.net/rancid/LICENSE.txt. For some reason this is not the same license. "COPYING" came with the source and is newer but the developers website state that the URL is the license for Rancid.
Some ramdom comments: * Licensing - Well, the license of LICENSE.txt as shown on the URL is actually NON-FREE, so the license cannot be accepted on Fedora. However as far as I checked the source tarball, this software is licensed under "BSD with advertising". Note that anyway we don't allow the license tag such as "Freely redistributable without restriction". * Versioning - This package seems to be a pre-version for 2.3.2. In such case the current versioning is wrong. Please refer to: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages * builds - Your package does not build: http://koji.fedoraproject.org/koji/taskinfo?taskID=685315 build.log says that at least "ping" binary (in iputils) is missing from BuildRequires. * perl module dependency - When adding perl module dependency as (Build)Requires, please don't write the rpm names directly but write what modules the rpms provide: https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides * Macros - Use macros correctly. For example, /etc must be %{_sysconfdir}. - Also, using the directory %{_prefix}/local is not allowed on Fedora. * Inproper scriptlets - You must call "mkdir" "chown" "ln" "chmod" commands on scriptlets except for very special cases. As same as other packages, for this package you must create the needed directories by the time %install ends, and must add those directories to %files entry and set attribute correctly by using %attr. * Directory ownership issue - Please make it sure that all directories which are created by installing this packages are correctly owned by this package. Currently %{_sysconfdir}/rancid/, %{_datadir}/rancid/ are not owned by any packages. * %changelog version - The EVR (Epoch:Version:Release) number of this package does not coincide with the last entry on %changelog. Please make it sure that when you modify your spec file, you also change the release number of the spec file to avoid confusion.
Thank you for your feedback. I've updated the spec-file with fixes to most of the comments, added a patch for the config-file and a cronjob. I'm still waiting for a email from the developer about the license, until then i'm using the "BSD with advertising" from the tarball. Still puzzled by the versioning.. Version 2.3.2a8 from the developer; would it then be rancid-2.3.2a8-0.2, rancid-2.3.2-0.2.a8 or rancid-2.3.2a8-0.2.a8..? (%{version} something else then developerversion seems to complicate the build, and I have not found similar examples to compare solutions with) New specfile and SRPM: http://files.thaumaturge.org/users/dante/rancid/rancid-0.2.spec http://files.thaumaturge.org/users/dante/rancid/ rancid-2.3.2a8-0.2.a8.fc9.src.rpm
Well: * Versioning - As the tarball included in the tarball seems pre-version, the versioning should be: 2.3.2-0.XXX.a8%{?dist} where XXX should be incremented every time you modify your spec file. * optflags - Fedora specific compilation flags are not passed correctly to C compiler. https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags You can check what compiler flags are used by $ rpm --eval %optflags Also please check the logs printeded out by rpmbuild command. -------------------------------------------------------------- 365 Making all in bin 366 gmake[1]: Entering directory `/builddir/build/BUILD/rancid-2.3.2a8/bin' 367 gcc -DHAVE_CONFIG_H -I. -I../include -g -O0 -MT hpuifilter.o -MD -MP -MF .deps/hpuifilter.Tpo -c -o hpuifilter.o hpuifilter .c -------------------------------------------------------------- * Macros - Please use macros: https://fedoraproject.org/wiki/Packaging/RPMMacros For example, etc must be %{_sysconfdir}, /var must be %{_localstatedir} or so. * Scriptlet related dependency - Please add "Requires(pre)" or so according to https://fedoraproject.org/wiki/Packaging/UsersAndGroups * Timestamps - When using "cp" or "install" commands, please add "-p" option to keep timestamps on installed files. * /sbin/ldconfig call - The rebuilded binary rpm does not contain any libraries under the default ld search path, so calling /sbin/ldconfig on scriptlets is not needed. * Invalid directory - using /var/_local_ is forbidden. Please use %_localstatedir/%name or so. * Directory ownership issue - Please make it sure that all directories created when installing a package are correctly owned by the rpm. For example, the directories %{_sysconfdir}/rancid/ and %{_datadir}/rancid/ are not owned by any packages.
ping?
I'm sorry, i've been on a short vacation. Will ship a new one out by this weekend.
ping again?
Forgive me for not being present at all here! This project have been a part of my work and I had to prioritize.. I hope i will be able to update it during this week.
I've been busy the last couple of weeks but i've squeeze in a update here. Still working on the optflag-part. New spec and srpm: http://files.thaumaturge.org/users/dante/rancid/rancid-0.3.spec http://files.thaumaturge.org/users/dante/rancid/rancid-2.3.2-0.3.a8.fc9.src.rpm
Created attachment 317827 [details] patch to honor cflags correctly For 2.3.2-0.3.a8 * spec file name - First of all, the name of the spec file should be "rancid.spec" * About optflags issue - Applying the patch attached will solve this issue. * SourceURL - Source0 must be written with full URL: https://fedoraproject.org/wiki/Packaging/SourceURL * Perl module dependency - The explicit dependencies perl(LockFile::Simple), perl(Mail::Mailer) can be removed. These dependencies are automatically found and added to binary rpm by rpmbuild itself. * Macros - Please use macros for standard directories. /etc should be %{_sysconfdir}. * Timestamp - Please consider to use -------------------------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" -------------------------------------------------------- to keep timestamps on installed files. This method usually works for Makefiles generated by recent autotools. * Directory ownership issue - Again please check if all directories which are created when this rpm is installed are correctly owned by this package. For example, the directory %{_sysconfdir}/%{name}/, %{_datadir}/%{name}/ are not owned by any packages: https://fedoraproject.org/wiki/PackagingDrafts/UnownedDirectories#Common_Mistakes Then: * Namespace conflict in %_bindir - This package tries to install so many files under %_bindir, and some of the files have rather generic names (like {a,b,c,...}login). Please - Move files to the directory specific to this rpm (e.g. %{_libdir}/%{name}) - Or rename binaries so that there names can easily be distinguished from binaries from other packages (like rancid-alogin or so) to avoid namespace conflict. Currently I have: ---------------------------------------------------------- # LANG=C rpm -ivh rancid-2.3.2-0.3.a8.fc10.1.i386.rpm Preparing... ########################################### [100%] file /usr/bin/par from install of rancid-2.3.2-0.3.a8.fc10.1.i386 conflicts with file from package par2cmdline-0.4-14.fc9.i386 ---------------------------------------------------------- Note that I have not installed the rebuilt binary rpm yet because of this conflict.
Thank you for the _thorough_ feedback and the makefile. I've uploaded a new version: http://files.thaumaturge.org/users/dante/rancid/rancid.cron http://files.thaumaturge.org/users/dante/rancid/rancid-2.3.2-0.4.a8.fc9.i386.rpm Namespace-conflict: I've moved the binaries to %{_libdir}/%{name} and symlinked in some of them to %{_bindir}. Directory ownership: %{_sysconfdir}/%{name}/ should be owned by this command "%attr(750,%{name},%{name}) %config(noreplace) %{_sysconfdir}/%{name}/" ?
(Removing FE-Legal regarding the license on the web as obsolete, review follows later)
For -0.4.a8: * CFLAGS ------------------------------------------------ export CFLAGS=$RPM_OPT_FLAGS ------------------------------------------------ - This line is not needed because %configure does this. (You can check what %configure does by $ rpm --eval %configure ) * Macros in comment ------------------------------------------------ #%{_sysconfdir}-files ------------------------------------------------ - Use %% in comments (and %changelog) instead of %% to prevent maros from being expanded. * Symlinks ------------------------------------------------ W: symlink-should-be-relative /usr/bin/rancid-cvs /usr/lib/rancid/rancid-cvs W: symlink-should-be-relative /usr/bin/rancid-run /usr/lib/rancid/rancid-run W: symlink-should-be-relative /usr/bin/rancid-fe /usr/lib/rancid/rancid-fe W: symlink-should-be-relative /usr/bin/rancid /usr/lib/rancid/rancid ------------------------------------------------ - It is requested that all symlinks should be relative (not absolute) i.e. ------------------------------------------------ for base in \ %{name} %{name}-cvs %{name}-fe %{name}-run do ln -sf ../../%{_libdir}/%{name}/${base} \ $RPM_BUILD_ROOT%{_bindir}/${base} done ------------------------------------------------ is better. Note that with the above lines you don't have to "push" to $RPM_BUILD_ROOT%{_bindir} . * About permission/directory ownership * %_sysconfdir/%name (In reply to comment #10) > Directory ownership: > %{_sysconfdir}/%{name}/ should be owned by this command > "%attr(750,%{name},%{name}) %config(noreplace) %{_sysconfdir}/%{name}/" ? - Perhaps you want: ------------------------------------------------- #%%{_sysconfdir}-files %attr(750,%{name},%{name}) %dir %{_sysconfdir}/%{name} %attr(600,%{name},%{name}) %config(noreplace) %{_sysconfdir}/%{name}/* ------------------------------------------------- * Directories in filesystem rpm ------------------------------------------------- %dir %{_bindir}/ %dir %{_mandir}/*/ ------------------------------------------------- - These directories are owned by filesystem rpm and so should not be owned by this rpm.
By the way: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on: http://fedoraproject.org/PackageReviewStatus/NEW.html (NOTE: please don't choose "Merge Review") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
Thank you for your fast reply. I've updated the spec-file: http://files.thaumaturge.org/users/dante/rancid/rancid.spec http://files.thaumaturge.org/users/dante/rancid/rancid-2.3.2-0.5.a8.fc9.i386.rpm
Well, %attr(750,%{name},%{name}) %config(noreplace) %{_sysconfdir}/%{name}/* Would you explain why files (not directories) under %_sysconfdir/%name should have 0750 permission (i.e. executable permission)? Also, I will wait for your another review request or your pre-review.
The "strange" setting was done in a hurry. I've changed it to something closer to usefull: http://files.thaumaturge.org/users/dante/rancid/rancid.spec http://files.thaumaturge.org/users/dante/rancid/rancid-2.3.2-0.6.a8.fc9.i386.rpm
Now this package itself is good. So as I wrote in comment 13, I will wait for your another review request or pre-review of other person's review request.
Due to "alot of stuff to fix", I will not be able to do any other request/pre-review until next week.
When providing new rpms, please provide SOURCE rpms and not i386.
Sorry for the enormous delay! I've submited a second review request: Bug 472713 - Review Request: Smokeping - keeps track of your network latency https://bugzilla.redhat.com/show_bug.cgi?id=472713
And even more sorry that smokeping-package already was made... I cannot believe that i missed it when i was looking for it!
Well, although your another review request was closed as duplicate, your spec file itself seemed in good shape to some extent. And as this package itself is now good, I will approve this package. --------------------------------------------------------- This package (rancid) is APPROVED by mtasaka --------------------------------------------------------- Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account". After you request for sponsorship a mail will be sent to sponsor members automatically (which is invisible for you) which notifies that you need a sponsor. After that, please also write on this bug for confirmation that you requested for sponsorship and your FAS (Fedora Account System) name. Then I will sponsor you. If you want to import this package into Fedora 9/10, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me.
Hi! Thank you for your feedback on this package and approving it! I'm about to start on the process of rebuilding it for import. I've just requested sponsorship and my FAS-name is "aageolai".
Okay, now I am sponsoring you. Please follow "Join" wiki again.
I will close this bug as NOTABUG if no response from the reporter is received within ONE WEEK.
Closing. Also I removed my sponsorship on the submitter. If someone wants to import this package into Fedora, please submit a new review request and mark this request as a duplicate of the new one. Thank you!
*** This bug has been marked as a duplicate of bug 512068 ***