Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: quotatool - Disk utility for managing user quotas
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Summary: Review Request: quotatool - Disk utility for managing user quotas Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: XiaShing@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.gieseking@uos.de Blocks| |177841(FE-NEEDSPONSOR)
--- Comment #1 from Martin Gieseking martin.gieseking@uos.de 2009-09-02 13:40:27 EDT --- 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)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #2 from Xia Shing Zee XiaShing@gmail.com 2009-09-02 21:34:51 EDT --- - 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #3 from Martin Gieseking martin.gieseking@uos.de 2009-09-03 03:59:44 EDT --- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #4 from Xia Shing Zee XiaShing@gmail.com 2009-09-03 04:08:01 EDT --- Oh, so I tell upstream so that they know about it for other packages as well? How do I notify them?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #5 from Martin Gieseking martin.gieseking@uos.de 2009-09-03 04:51:47 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #6 from Xia Shing Zee XiaShing@gmail.com 2009-09-07 01:26:49 EDT --- Ok, the license on the official website's page now reflects the LICENSE file in the RPM.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #7 from Martin Gieseking martin.gieseking@uos.de 2009-09-07 02:26:19 EDT --- OK, thanks for that. Now you have to wait for a sponsor to get your package formally reviewed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jussi.lehtola@iki.fi
--- Comment #8 from Jussi Lehtola jussi.lehtola@iki.fi 2009-09-07 02:40:59 EDT --- - 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #9 from Xia Shing Zee XiaShing@gmail.com 2009-09-07 07:12:08 EDT --- Ok, fixed the empty comments and the Release tag.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #10 from Xia Shing Zee XiaShing@gmail.com 2009-09-14 02:06:02 EDT --- Can this be built on koji?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |itamar@ispbrasil.com.br
--- Comment #11 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-09-14 02:14:03 EDT --- (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_To...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #12 from Xia Shing Zee XiaShing@gmail.com 2009-09-14 08:37:44 EDT --- Ok, it was successfully built on dist-F11 and dist-F12.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tcallawa@redhat.com AssignedTo|nobody@fedoraproject.org |tcallawa@redhat.com Flag| |fedora-review?
--- Comment #13 from Tom "spot" Callaway tcallawa@redhat.com 2009-09-15 10:17:27 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #14 from Xia Shing Zee XiaShing@gmail.com 2009-09-15 11:45:16 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #15 from Tom "spot" Callaway tcallawa@redhat.com 2009-09-15 12:06:02 EDT --- 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. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #16 from Jussi Lehtola jussi.lehtola@iki.fi 2009-09-15 12:16:54 EDT --- (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).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #17 from Xia Shing Zee XiaShing@gmail.com 2009-09-15 16:17:03 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) | Flag|fedora-review? |fedora-review+
--- Comment #18 from Tom "spot" Callaway tcallawa@redhat.com 2009-09-16 11:53:42 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #19 from Xia Shing Zee XiaShing@gmail.com 2009-09-16 20:33:32 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #20 from Tom "spot" Callaway tcallawa@redhat.com 2009-09-16 20:41:39 EDT --- 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? :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #21 from Xia Shing Zee XiaShing@gmail.com 2009-09-16 20:53:44 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Xia Shing Zee XiaShing@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Huzaifa S. Sidhpurwala huzaifas@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #22 from Huzaifa S. Sidhpurwala huzaifas@redhat.com 2009-09-17 03:02:30 EDT --- cvs done
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #23 from Jussi Lehtola jussi.lehtola@iki.fi 2009-09-17 03:13:13 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Xia Shing Zee XiaShing@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs+ |fedora-cvs?
--- Comment #24 from Xia Shing Zee XiaShing@gmail.com 2009-09-17 04:50:51 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #25 from Xia Shing Zee XiaShing@gmail.com 2009-09-17 04:52:11 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #26 from Xia Shing Zee XiaShing@gmail.com 2009-09-17 05:17:23 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Dan Horák dan@danny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dan@danny.cz
--- Comment #27 from Dan Horák dan@danny.cz 2009-09-17 05:40:51 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #28 from Fedora Update System updates@fedoraproject.org 2009-09-17 06:17:02 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #29 from Xia Shing Zee XiaShing@gmail.com 2009-09-17 06:18:55 EDT --- Dan, thanks for that, I temporarily forgot that once a CVS module is created, the owner can update it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Xia Shing Zee XiaShing@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #30 from Fedora Update System updates@fedoraproject.org 2009-09-17 06:21:16 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |ON_QA
--- Comment #31 from Fedora Update System updates@fedoraproject.org 2009-09-17 14:01:20 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #32 from Fedora Update System updates@fedoraproject.org 2009-09-18 20:04:22 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |1.4.10-7.fc11 Resolution| |ERRATA
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
--- Comment #33 from Fedora Update System updates@fedoraproject.org 2009-10-08 14:02:46 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520832
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|1.4.10-7.fc11 |1.4.10-7.el5
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla