Spec URL: http://ashay.info/rpm/cpulimit.spec SRPM URL: http://ashay.info/rpm/cpulimit-1.1-1.fc11.src.rpm Description: cpulimit is a simple program that attempts to limit the cpu usage of a process (expressed in percentage, not in cpu time). This is useful to control batch jobs, when you don't want them to eat too much cpu. It does not act on the nice value or other scheduling priority stuff, but on the real cpu usage. Also, it is able to adapt itself to the overall system load, dynamically and quickly. More info here: http://cpulimit.sourceforge.net/ The original source did not use autotools and had a simple Makefile. I converted the source to use autotools. Thank you
Just some quick comments on your spec file: You wrote in your sec file... ----%>------ #The source on sourceforge does not use autotools. Source1 does Source0: http://downloads.sourceforge.net/project/cpulimit/cpulimit/cpulimit/cpulimit-1.1.tar.gz Source1: http://ashay.info/rpm/cpulimit-1.1.tar.gz ----<%------ Have you make a patch for the usage of autotools? If yes, send this patch upstream and apply the patch in the spec file. https://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25prep_section:_.25patch_commands There are more details available about 'Source0' https://fedoraproject.org/wiki/How_to_create_an_RPM_package#Spec_file_pieces_explained look for 'Source0' - 'Source0' seems to be wrong, shouldn't it be 'http://downloads.sourceforge.net/cpulimit/%{name}-%{version}.tar.gz' ? - For valid groups you can use 'less /usr/share/doc/rpm-*/GROUPS' - %attr (-,root,root) can be left away, it sufficient only to use %{_mandir}/*/* - Isn't rpmlint complaining about the line length of the %description?
*** Bug 513663 has been marked as a duplicate of this bug. ***
replace these 2 lines mkdir -p $RPM_BUILD_ROOT%{_mandir}/man1 install -m 644 man/cpulimit.1 $RPM_BUILD_ROOT%{_mandir}/man1/cpulimit.1 with install -Dp -m (only one line do the job)
is this your first package ? what's your fedora account (FAS) username ?
(In reply to comment #1) > - Isn't rpmlint complaining about the line length of the %description? rpmlint output [fab@laptop09 SRPMS]$ rpmlint cpulimit-1.1-1.fc11.src.rpm cpulimit.src: E: description-line-too-long cpulimit is a simple program that attempts to limit the cpu usage of a process (expressed in percentage, not in cpu time). cpulimit.src: E: description-line-too-long This is useful to control batch jobs, when you don't want them to eat too much cpu. cpulimit.src: E: description-line-too-long It does not act on the nice value or other scheduling priority stuff, but on the real cpu usage. cpulimit.src: E: description-line-too-long Also, it is able to adapt itself to the overall system load, dynamically and quickly. cpulimit.src: W: non-standard-group System Tools cpulimit.src: W: invalid-license GPL 1 packages and 0 specfiles checked; 4 errors, 2 warnings.
Thank you for your comments... I have updated the spec file as suggested. The "convert to autotools" portion is now a patch, which I also sent upstream. rpmlint errors/warnings are fixed. Yes, this is my first package. My FAS account name is ashayh. Thank you
(In reply to comment #6) please submit the new spec and src.rpm file, remember to bump version to avoid confusion.
(In reply to comment #7) > (In reply to comment #6) > please submit the new spec and src.rpm file, remember to bump version to avoid > confusion. Here is the new srpm: http://ashay.info/rpm/cpulimit-1.1-2.fc11.src.rpm http://ashay.info/rpm/cpulimit.spec
Your patch add several files. The AUTHORS, COPYING, and all other documentation files should go in %doc. There is a rpmlint warning. This is easy to fix ;-) [root@laptop09 i586]# rpmlint cpulimit* cpulimit.i586: W: incoherent-version-in-changelog 1.1 ['1.1-2.fc11', '1.1-2'] 2 packages and 0 specfiles checked; 0 errors, 1 warnings. A blank line between the changelog entry blocks make the whole changelog easier to ready. One suggestion: Would it make the life of upstream easier if the man page is installed within the Makefile? The time stamps are not preserved (https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps). Almost all files are generated with the patch so this will affect after a new upstream release.
Upstream has a patch (an empty one) http://sourceforge.net/tracker/?group_id=174425&atid=869186
Added man file in make. Should I bump the version each and every time I fix something here? (I bumped it) I have no idea how to fix the time stamps issue...please advise if you do. http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-3.fc11.src.rpm
(In reply to comment #11) > Added man file in make. To the Makefile? Is in this case the line 'install -Dp -m 644 man/cpulimit.1 $RPM_BUILD_ROOT%{_mandir}/man1/cpulimit.1' still needed? > Should I bump the version each and every time I fix something here? (I bumped > it) Everytime you make a change, bump the release. As Itamar already said, this avoid confusion. > I have no idea how to fix the time stamps issue...please advise if you do. You can use 'make install INSTDIR=$RPM_BUILD_ROOT INSTALL="install -p"' instead of 'make install DESTDIR=$RPM_BUILD_ROOT '. You can change this later when everything is upstream but often it don't harm when it's added before it takes effect.
I removed the "install -Dp -m 644 man/cpulimit.1 $RPM_BUILD_ROOT%{_mandir}/man1/cpulimit.1" " line. ' make install INSTDIR=$RPM_BUILD_ROOT INSTALL="install -p" ' does not work. ( configure and make do not recognize any variable called INSTDIR, and rpmbiuld fails because of that. ) So I added "install -p" to ./configure. Will this work as expected to preserve timestamps? http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-4.fc11.src.rpm Thanks
Sorry, my fault... INSTDIR should be DESTDIR. make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" https://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25install_section As I already said as long the doc files and the man page aren't upstream this is only cosmetically. (In reply to comment #13) > So I added "install -p" to ./configure. Will this work as expected to preserve > timestamps? No, I was misleading you. Can you please fix this? Then I will make a full review. BTW, I'm not a sponsor. You can get some hints on that process at the following page https://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored . The long story short: Make some informal reviews of other packages and make some more packages to show that you understand the guidelines and are familiar with them.
Thanks, I will follow the sponsorship process. Fixed files here: http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-5.fc11.src.rpm Any issues now?
Don't bother with autotools. There's nothing wrong with a package that builds without them, especially when the build process is this simple. In fact, you don't even need to use the makefile (since it doesn't support optflags). I recommend that you drop Patch0 and %setup -q %build gcc $RPM_OPT_FLAGS -lrt -o cpulimit cpulimit.c %install rm -rf $RPM_BUILD_ROOT install -p -m 755 cpulimit $RPM_BUILD_ROOT/%{_bindir}/cpulimit ** Besides, your Patch is *creating* the files AUTHORS COPYING README INSTALL ChangeLog If you want to ship these, list them as extra Source lines. Where did you get these, anyway..? The tarball only contains the source code file and the simple Makefile. ** I am willing to sponsor you, if you show me you know the Fedora guidelines; the most important of which are http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets To do this you need to make another submission, and perform two informal reviews of packages of other people. Please review only packages not marked with the FE-NEEDSPONSOR blocker as I will have to do the full formal review after you to check that you have got everything right. After being sponsored, you are able to do reviews of your own.
(In reply to comment #16) > install -p -m 755 cpulimit $RPM_BUILD_ROOT/%{_bindir}/cpulimit .. should be, of course, install -D -p -m 755 (I thought something was missing). Fabian: as you've already started, you can do the review on this one. I'll throw in extra comments if necessary :)
(In reply to comment #16) > Don't bother with autotools. There's nothing wrong with a package that builds > without them, especially when the build process is this simple. I agree, but my recommendation is a small patch to makefile without auto-tools. and sending this small patch to upstream of cpulimit.
(In reply to comment #18) > (In reply to comment #16) > > Don't bother with autotools. There's nothing wrong with a package that builds > > without them, especially when the build process is this simple. > > I agree, but my recommendation is a small patch to makefile without auto-tools. > and sending this small patch to upstream of cpulimit. Yes, that is fine too. Then the makefile should look like BINDIR=/usr/local/bin MANDIR=/usr/local/man INSTALL="install" CFLAGS="-Wall -O2" all:: cpulimit cpulimit: cpulimit.c gcc ${CFLAGS} -o cpulimit cpulimit.c -lrt clean: rm -f cpulimit install: ${INSTALL} -D -m 755 cpulimit ${DESTDIR}${BINDIR} ${INSTALL} -D -m 644 cpulimit.1 ${DESTDIR}${MANDIR}/man1/cpulimit.1 By default this will compile with "-Wall -O2" and install to /usr/local/bin. In the spec file one would use %build make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" to override the CFLAGS variable in Makefile and %install make DESTDIR=$RPM_BUILD_ROOT BINDIR=%{_bindir} MANDIR=%{_mandir} INSTALL="install -p" install to override the other variables. One can also do without DESTDIR support, then one just prepends $RPM_BUILD_ROOT to MANDIR and BINDIR.
(In reply to comment #19) the auto-tools previous patch was very intrusive. patching makefile sounds perfect for me; you could include CC, something like this. > BINDIR=/usr/local/bin > MANDIR=/usr/local/man > INSTALL="install" > CFLAGS="-Wall -O2" CC="gcc" > all:: cpulimit > > cpulimit: cpulimit.c > ${CC} ${CFLAGS} -o cpulimit cpulimit.c -lrt >
(In reply to comment #17) > Fabian: as you've already started, you can do the review on this one. > > I'll throw in extra comments if necessary :) Ok, I will. It good to know are you are watching over my shoulder ;-)
Ok, so as suggested, I got rid of autotools and made a patch for the Makefile. The man page was not included by upstream, I got that from the debian package. If I do not include the man page, rpmlint warns about no documentation. So I decided to include the man page as Source1. (and sent it upstream on sf.net) If that's not a good idea, I will remove the man page and let the warning be. Thanks everyone for their comments. Jussi, thanks for the sponsorship offer. I'll make a few more packages and review some packages soon. I intend to make/maintain packages on a regular basis. http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-6.fc11.src.rpm
(In reply to comment #22) > Ok, so as suggested, I got rid of autotools and made a patch for the Makefile. > > The man page was not included by upstream, I got that from the debian package. > If I do not include the man page, rpmlint warns about no documentation. > So I decided to include the man page as Source1. (and sent it upstream on > sf.net) > > If that's not a good idea, I will remove the man page and let the warning be. OK. Debian considers not having a man file a packaging error, so they have the tendency of generating them if they do not already exist. In Fedora we do not do so; we try to stay as close to upstream as possible and thus if there is no documentation in the tarball we don't ship any, either. I wouldn't ship the man page (generally there's no guarantee that it isn't obsolete), but you should send it upstream to see if they could include it in the next release. Also ask them to add the relevant license file, i.e. http://www.gnu.org/licenses/gpl-2.0.txt Some times you don't have to care about rpmlint warnings. The no-doc warning is the most common of these cases; you just have to check if there is something that should be put there. > Thanks everyone for their comments. > > Jussi, thanks for the sponsorship offer. > I'll make a few more packages and review some packages soon. > I intend to make/maintain packages on a regular basis. Good, since packaging shouldn't be a one-off activity :) Paste the numbers of the BZ entries here so I can check them out.
Ok, I removed the man page. Here are the new files: http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-7.fc11.src.rpm
After the rebuild of the source RPM rpmlint says: [fab@laptop09 i586]$ rpmlint cpulimit* cpulimit.i586: W: no-documentation cpulimit-debuginfo.i586: E: debuginfo-without-sources 2 packages and 0 specfiles checked; 1 errors, 1 warnings. For it seems that the compiler flags aren't honored. Comment #19 said what to do ;-)
(In reply to comment #25) > After the rebuild of the source RPM rpmlint says: > > [fab@laptop09 i586]$ rpmlint cpulimit* > cpulimit.i586: W: no-documentation > cpulimit-debuginfo.i586: E: debuginfo-without-sources > 2 packages and 0 specfiles checked; 1 errors, 1 warnings. > > For it seems that the compiler flags aren't honored. Comment #19 said what to > do ;-) i.e. you need make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" instead of make %{?_smp_mflags}
Ok, I added that: http://ashay.info/rpm/cpulimit.spec http://ashay.info/rpm/cpulimit-1.1-8.fc11.src.rpm
Have you made any other submissions of performed informal reviews?
*** Bug 524006 has been marked as a duplicate of this bug. ***
Is this review request still ongoing?
Ashay, did you make some informal reviews or other packages?
ping?
Hi Fabian Sorry for the delay. I've been on vacation for a long time with no practical access to computers/internet. I've done one informal review so far. And created another package (adtool) I'll be back in a few weeks and continue...
Ashay, is there any progress? There was another review request by Xia submitted #524006, if you have no time it would be an option to close this review and go with the one of Xia. What do you think?
Please close this and go ahead with Xias review. Thanks
Closing as requested.
*** This bug has been marked as a duplicate of bug 772406 ***