Spec URL: http://202.60.89.117/fedora/SPECS/quotatool.spec SRPM URL: http://202.60.89.117/fedora/SRPMS/quotatool-1.4.10-1.fc11.src.rpm Description: Quotatool is a utility to set filesystem quotas from the commandline. It suitable for use in scripts and other non-interactive situations.
This seems to be your first package submission to Fedora. Thus, you need a sponsor reviewing this package (see http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_Sponsored) I'm not a sponsor, so consider the following comments as informal. There are some issues in the spec file that should be fixed: - According to the source files GPLv2 is OK. However, the project homepage doesn't mention a GPL version number but links to GPLv3. Maybe you should ask upstream to fix this ambiguity - add a short comment above %Patch0 about what the patch does - I'd suggest to use the description text from the project website because it's a bit more detailed. - remove -n quotatool-%{version} from %setup, as it's not necessary - Patch0 is not applied because of the wrong parameter -p1 (remove -p1 from %patch0) - remove INSTALL from %doc (it's not of much use in a binary package)
- The website links to the latest GPL page, where the GNU is free to make changes to it at any time. But it was released with GPLv2 which is why I went with that. - Comment added above %Patch0 - I took snippets from the website to keep it brief - Removed some stuff from %setup - Removed -p1 as well - Removed INSTALL from %doc
(In reply to comment #2) > - The website links to the latest GPL page, where the GNU is free to make > changes to it at any time. Yes, that's the point I tried to make. You should inform upstream that source files and website are out of sync concerning the license. This should be fixed. It doesn't affect the package though, as the sources indicate GPLv2.
Oh, so I tell upstream so that they know about it for other packages as well? How do I notify them?
Simply write an email to the author of quotatool and inform him about the issue. You can also add a link to this review request. The email address is given on the website.
Ok, the license on the official website's page now reflects the LICENSE file in the RPM.
OK, thanks for that. Now you have to wait for a sponsor to get your package formally reviewed.
- Drop the empty & commented Requires: and BuildRequires: lines. - Every time you update the spec file increment the Release: tag and make a comment in the changelog. So the last release was -2 and the next one is -3.
Ok, fixed the empty comments and the Release tag.
Can this be built on koji?
(In reply to comment #10) > Can this be built on koji? yes, please try this and let me know if works. https://fedoraproject.org/wiki/PackageMaintainers/Join#Install_the_Client_Tools_.28Koji.29
Ok, it was successfully built on dist-F11 and dist-F12.
A few minor issues of note: * When you make an updated package, it is useful to post updated links to the new SRPM in the review bug. (I found your -3 SRPM, but it might have been overlooked by a different reviewer) * You should try to use %{name} and %{version} in the Source0: line, this helps ensure that as you update the package, you don't accidentally build it against older source. * While technically correct to not explicitly list the patch level when applying Patch0, I generally recommend that packagers do so, especially to make it clear to someone who might inherit this package in the future. Basically, you'd change: %patch0 to %patch0 -p0 * I also generally recommend that packagers generate patches at patch level 1. If you use a suffix when making changes (e.g. Makefile.OLD), then it is simple to use the "gendiff" tool to generate a patch: gendiff quotatool-1.4.10 .BAD * It is also useful to use a relevant suffix when applying the patch in the spec file: %patch0 -p0 -b .destdir This can come in handy when trying to regenerate patches for future updates. * Also, patch naming is useful. You named your patch "Makefile.patch", I would recommend something like: quotatool-1.4.10-DESTDIR.patch * You should go ahead and send patches upstream whenever appropriate. In this case, enabling DESTDIR support is useful for all distributions, and harmless if it is not set, so it is appropriate to send it upstream. You can do this by emailing it to their mailing list, or opening a bug in their bug tracker. You should also put a link to wherever you sent the patch to upstream as a comment in the spec file: # Sent upstream: # http://foo.com/bar/bug12345 * One very minor point: In Fedora 10+, it is no longer necessary to call: rm -rf $RPM_BUILD_ROOT immediately after %install. RPM now does this for you as the first step of %install. * You should get in the habit of running rpmlint against the SRPM and binary RPMs. For example, when I ran: [spot@pterodactyl ~]$ rpmlint /home/spot/rpmbuild/SRPMS/quotatool-1.4.10-3.fc12.src.rpm /home/spot/rpmbuild/RPMS/x86_64/quotatool-1.4.10-3.fc12.x86_64.rpm /home/spot/rpmbuild/RPMS/x86_64/quotatool-debuginfo-1.4.10-3.fc12.x86_64.rpm quotatool.src:48: W: macro-in-%changelog doc quotatool.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12) 3 packages and 0 specfiles checked; 0 errors, 2 warnings. Both of these are minor errors, but easily corrected. (You should use %% to escape out any macros used in changelog entries) Please show me an updated SRPM that incorporates fixes for the above items, and I will finish the review and sponsor you.
quotatool-1.4.10-4 seems clean now with rpmlint. SPEC file: http://xiashing.fedorapeople.org/SPECS/quotatool.spec SRPM: http://xiashing.fedorapeople.org/SRPMS/quotatool-1.4.10-4.fc11.src.rpm
Your patch is backwards, and does not apply. When making the patch, before making any code changes, make a backup of the file you plan to edit: cp Makefile Makefile.old Then, edit the Makefile (leave Makefile.old alone): vi Makefile # make changes here and save as Makefile Then, generate the patch. Somehow, you did it backwards. :)
(In reply to comment #15) > Your patch is backwards, and does not apply. Maybe it's just been generated (manually) the wrong way? $ diff new orig instead of $ diff orig new (the way it's supposed to be done according to the man page).
Yeh, that's what I was asking about on IRC. It seems correct that we are allowed to edit the Makefile, and then Makefile.OLD is the original Makefile. I was under the impression that you create something like Makefile2 which differs from Makefile, and generate a patch from that, to patch Makefile. Anyway, new build: SPEC file: http://xiashing.fedorapeople.org/SPECS/quotatool.spec SRPM: http://xiashing.fedorapeople.org/SRPMS/quotatool-1.4.10-5.fc11.src.rpm
Review ======= Good: - rpmlint checks return nothing - package meets naming guidelines - package meets packaging guidelines - license (GPLv2) OK, text in %doc - spec file legible, in am. english - source matches upstream (9fbad0c03c21463c1529365764ab1e636b4a5c5aed95a7d3ed67b49cd39d7179) - package compiles on devel (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file Licensing Note: The source code does not contain any license attribution aside from one reference: fprintf (stderr, " Distributed under the GNU General Public License\n"); Technically, every source code file should have a comment header in which the license of the code is given, something like: /* Copyright (C) yyyy name of author This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; version 2 of the License. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ You should ask upstream to add this header to all of their source files (with a correct copyright date and author name). Technically, the code without these attributions is under "GPL+", because the GPL explicitly says that: "If the Program does not specify a version number of this License, you may choose any version ever published by the Free Software Foundation." However, we know from the website that the author's intent is for this code to be GPLv2 only, so it is fine to leave it tagged as such. By adding the copyright statement to his source files, he will also be specifying the version of the GPL he intends to use, and closing this ambiguity. ***** This package is approved, and I will sponsor you.
New Package CVS Request ======================= Package Name: quotatool Short Description: summary of package Owners: Xia Shing Zee Branches: F-11 F-12 EL-5 InitialCC: xiashing, spot
Xia, you need to do a few things here. 1. You need to use your username in the "Owners" field, not your real name. :) 2. You don't need to specify F-12, that is "devel", and you always get that branch. 3. You didn't put the summary of the package, you put "summary of package". 4. You didn't change the fedora-cvs flag to ?, so no one will see this request. Try again? :)
I just realized all of that, and got a mid-air collision when I tried to change the fedora-cvs flag ;) This should do it: New Package CVS Request ======================= Package Name: quotatool Short Description: quotatool is a utility to set filesystem quotas from the commandline. It is suitable for use in scripts and other non-interactive situations. Owners: xiashing Branches: F-11 EL-5 InitialCC: xiashing, spot
cvs done
Uhm, and the short description should be the same as the summary in the spec file, i.e. "A utility to set filesystem quotas". Anyway, once you've imported the package the short description should be automatically updated from the spec file summary.
New changes to the .spec file so that it builds successfully on ppc64 with koji: SPEC: http://xiashing.fedorapeople.org/SPECS/quotatool.spec SRPM: http://xiashing.fedorapeople.org/SRPMS/quotatool-1.4.10-6.fc11.src.rpm
Short description updated as well as suggested New Package CVS Request ======================= Package Name: quotatool Short Description: A utility to set filesystem quotas. Owners: xiashing Branches: F-11 EL-5 InitialCC: xiashing, spot
Additional changes that should have been in the last release: SPEC: http://xiashing.fedorapeople.org/SPECS/quotatool.spec SRPM: http://xiashing.fedorapeople.org/SRPMS/quotatool-1.4.10-7.fc11.src.rpm And the Updated CVS Request: Update Package CVS Request ======================= Package Name: quotatool Short Description: A utility to set filesystem quotas. Owners: xiashing Branches: F-11 EL-5 InitialCC: xiashing, spot
After the creation of CVS module you are doing all updates on your own, without new CVS requests in bugzilla, please read https://fedoraproject.org/wiki/PackageMaintainers/UpdatingPackageHowTo
quotatool-1.4.10-7.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/quotatool-1.4.10-7.fc11
Dan, thanks for that, I temporarily forgot that once a CVS module is created, the owner can update it.
quotatool-1.4.10-7.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/quotatool-1.4.10-7.el5
quotatool-1.4.10-7.el5 has been pushed to the Fedora EPEL 5 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update quotatool'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0472
quotatool-1.4.10-7.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
quotatool-1.4.10-7.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.