Spec URL: http://home.badc.rl.ac.uk/iwi/ren.spec SRPM URL: http://home.badc.rl.ac.uk/iwi/ren-1.0-5.src.rpm Description: Rename multiple files This small but very useful utility (not written by me) is in RedHat contrib at version 1.0-4, but doesn't seem to be in Fedora Extras. I have taken it and made some tweaks to clean up the build and packaging and called it 1.0-5, but the functionality is still as it ever was.
Not an official review as I'm not yet sponsored Mock build fro i386 development is sucessfull MUST Items: - MUST: rpmlint shows error W: ren summary-not-capitalized rename multiple files Summary doesn't begin with a capital letter. Change Summary to Summary: Rename multiple files W: ren non-standard-group Utilities/File The group specified in your spec file is not valid. To find a valid group, please refer to the RPM documentation. No such groups exists. W: ren invalid-license public domain The license you specified is invalid. The valid licenses are: -GPL -LGPL -Artistic -BSD -MIT -QPL -MPL -IBM Public License -Apache License -PHP License -Public Domain -Modified CNRI Open Source License -zlib License -CVW License -Ricoh Source Code Public License -Python license -Vovida Software License -Sun Internet Standards Source License -Intel Open Source License -Jabber Open Source License if the license is close to an existing one, you can use '<license> style'. Add License tag and any of above suitable open source license. W: ren no-url-tag Add URL: package hosting URL W: ren hardcoded-prefix-tag /usr The Prefix tag is hardcoded in your spec file. It should be removed, so as to allow package relocation. Use macros instead hardcoding directories. check http://www.fedoraproject.org/wiki/Extras/RPMMacros E: ren no-%clean-section The spec file doesn't contain a %clean section to remove the files installed by the %install section. Add to SPEC file %clean rm -rf $RPM_BUILD_ROOT W: ren patch-not-applied Patch0: ren-1.0.Wall.patch A patch is included in your package but was not applied. Refer to the patches documentation to see what's wrong. Move changelog at end. You need to read http://fedoraproject.org/wiki/Packaging/Guidelines
Please can you download the file again, as I have now corrected as many of the above errors as possible. However, there are two errors that I can't correct because I can't see what's wrong. 1) "no-%clean-section" There is already a %clean section. It says: %clean rm -rf %{buildroot} 2) "patch-not-applied" The patch is applied when I do rpmbuild. The purpose of the patch is to fix a load of things that cause warnings when compiled with "gcc -Wall". When I build it, it tells me: + echo 'Patch #0 (ren-1.0.Wall.patch):' Patch #0 (ren-1.0.Wall.patch): + patch -p1 -s and later: + make CC=gcc CFLAGS=-Wall gcc -o ren -Wall ren.c + exit 0 It could not possibly be completing "gcc -Wall" without errors if the patch was not being applied. Regards, Alan
will check it again now
hey you have not changed version number and not given updates SPEC as well as SRPM links Remember to update release versions whenever you change SPEC file and give upadtes links
ok rpmlint on package return no error Mock built for i386 development is also successfull but still i found that * no dist tag Add to release as Release: 5%{?dist} * Change Buildroot to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * Add version info to each Chnagelog entry. Check other peoples SPEC files here. * You need to provide your License file in tarball.
Sorry, I am going to have to cancel this submission as the License file is not in the tarball, and in accordance with the "pristine sources" principle I will not change the tarball in any way from what is on ftp://sunsite.unc.edu/ (The package originates from a posting to comp.sources.unix, which I assume means it is public domain, but there is no License file.)
hey dont do that now. wait for official review of this package from any of official reviewers.
(In reply to comment #6) > Sorry, I am going to have to cancel this submission as the License file is not > in the tarball, and in accordance with the "pristine sources" principle I will > not change the tarball in any way from what is on ftp://sunsite.unc.edu/ > > (The package originates from a posting to comp.sources.unix, which I assume > means it is public domain, but there is no License file.) Parag is confusing the roles of "upstream" and "packager". Sometimes the two are the same, in which case upstream should ensure that licensing information is included properly in the distribution. In this case, "upstream" and "packager" are different, and the packager should *not* be fiddling with the tarball, Indeed, the package review guidelines do not require the inclusion of license text if upstream does not provide it. There are very few circumstances in which a packager should alter an upstream tarball (e.g. it contains patented content that is not used in the package), and adding a license file is certainly not one of them. Please continue with your submission.
Okay, fine, I won't withdraw it now. However unfortunately I do not understand what is meant regarding dist tags, and have run out of time, so if the review indicates that further changes are required then I will let it drop at that stage. But if someone who knows more about Fedora packaging requirements than I do agrees with my view that this is a very useful little utility to include, and is willing to take on the packaging and iron out any further little problems, then that would be much appreciated, thanks. Bye for now, Alan
(In reply to comment #9) > Okay, fine, I won't withdraw it now. However unfortunately I do not understand > what is meant regarding dist tags, and have run out of time, so if the review > indicates that further changes are required then I will let it drop at that > stage. But if someone who knows more about Fedora packaging requirements than I > do agrees with my view that this is a very useful little utility to include, and > is willing to take on the packaging and iron out any further little problems, > then that would be much appreciated, thanks. The dist tag is described on the wiki: http://fedoraproject.org/wiki/DistTag Use of it is not mandatory, but most people do use it as it's a conveient way of distinguishing packages built for different distribution releases. If you've run out of time then it's probably better to close the review. Adding packages to Fedora Extras is not a "fire and forget" process; packages need to be _maintained_, so that bug reports get answered, security issues get fixed etc. If you don't have the time to finish the submission process then it's unlikely you'll have the time to maintain the package in the longer term, so indeed it would be better off for someone else to maintain it.
Paul, Thanks for clearing licensing issue.
I have adjusted the spec per comment #5. Spec file and src.rpm are available at http://wdl.lug.ro/linux/rpms/ren/ren-1.0-6.src.rpm and http://wdl.lug.ro/linux/rpms/ren/ren.spec Alan, per comment #9, if you have nothing against, I'll try to co-maintain together with you this package. Manuel
What's the reason for using CFLAGS=-Wall instead of CFLAGS="-Wall %{optflags}" ?
I've uploaded to http://wdl.lug.ro/linux/rpms/ren/ren.spec and http://wdl.lug.ro/linux/rpms/ren/ren-1.0-6.src.rpm new versions of the spec and src.rpm files. Changes: cleanup of the %build and %files (permisssions)sections to closer resemble the skeleton generated by fedora-newrpmspec. Obs: the original package does not really include an install section in the Makefile. So we could either keep the 3 lines from the current form (they simply copy the files to their places) or add yet another patch which would modify the Makefile so that make install would copy the files itself.
I am sorry, I meant http://wdl.lug.ro/linux/rpms/ren/ren-1.0-7.src.rpm not 1.0-6
I can review this one. ---------------------------------------- Review for release 7: * RPM name is OK * This is the latest version * Builds fine in mock * rpmlint looks OK * File list looks OK Needs work: debuginfo package is not built correctly. I added CFLAGS="$RPM_OPT_FLAGS" to the make line: make CFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags} This builds a correct debuginfo package. Fix that up and I can approve this submission.
Thank you for the review, Michael. Spec file modified accordingly. The new versions are available at http://wdl.lug.ro/linux/rpms/ren/ren-1.0-8.src.rpm and http://wdl.lug.ro/linux/rpms/ren/ren.spec
Looks good. APPROVED. Please remember to close this review submission once imported into CVS and built.
I am sorry, but as this is my first approved package, I am not sure about the next step... Does your approval mean that I am sponsored?
Nope.. someone needs to sponsor you. You needed to have FE-NEEDSPONSOR blocking your review request. I can't sponsor anyone just yet.
OK, I can sponsor you.( just been made a sponsor) However, this is a fairly small and simple package. Do you plan to submit more packages to FE ?
Actually my first submission was ssmtp. Please be as kind as to check https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=188400 I have adopted ren because the original submitter preferred to abandon rather then make a few [simple] modifications and it was a pity to loose his work. And since we use it anyway at my workplace...
Sorry. Due to my stepping out for a while, I am unable to complete this review.
I'm not a reviewer, as I'm waiting to be sponsored, but I can help out: rpmlint still shows warning on the RPM, I'm not sure if you're absolutely required to fix these as they're minor, but they're there nonetheless: W: ren setup-not-quiet You should use -q to have a quiet extraction of the source tarball, as this generate useless lines of log ( for buildbot, for example ) W: ren macro-in-%changelog defattr Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'. W: ren macro-in-%changelog _mandir Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'.
Thank you for your review. New versions are available at http://wdl.lug.ro/linux/rpms/ren/ren.spec and http://wdl.lug.ro/linux/rpms/ren/ren-1.0-9.src.rpm
Excellent - rpmlint's clean on both SRPM and binary RPM.
It looks to me like Michael was going to sponsor you, but due to his time constraints he was unable to do so. However, this bug is in a weird state where potential sponsors can't see it. I am changing it back to block FE-NEW so another sponsor can see it.
Ok, I'll have a look at it soon.
The source don't match. From spectool -g: 245453453a8bd1c0bf7d03880b97d632 ren-1.0.tar.gz versus yours 2fa23fa3ebf7e6d68147f5f91725dd29 ../SOURCES/ren-1.0.tar.gz Otherwise the man page is better globbed like, in my opinion %{_mandir}/man1/ren.1* I would have choosed ftp://sunsite.unc.edu/pub/Linux/utils/file/ren-1.0.tar.gz.lsm as url. Not a big deal.
Thanks for the review, Patrice. As usual, your eye view is sharper then mine.. at least for the time being :) Indeed, the tar file included by Alan seemed to be a repacked version of the files included in the upstream version. diff -r did not show any differences between the actual content of the two but I have included the original, as it should have been in the first place. AFAIK in all RH/FC distributions, man pages are gzip-ed, hence the .ren.1.gz from the spec. I have decided to follow your advice and play safe, just in case. Please find the updated spec and src.rpm at http://wdl.lug.ro/linux/rpms/ren/ren.spec and http://wdl.lug.ro/linux/rpms/ren/ren-1.0-10.src.rpm respectively.
* rmlint is silent * licence is not explicitely set, however since it comes from a post on usenet with explicit permission to reuse and redistribute this may be considered public domain. * follow guidelines * match upstream 245453453a8bd1c0bf7d03880b97d632 ren-1.0.tar.gz * %files right The timestamp of the file is wrong. To get the right timestamp, you can use wget -N ftp://sunsite.unc.edu/pub/Linux/utils/file/ren-1.0.tar.gz or spectool -g ren.spec This is not a blocker so it is APPROVED. It would be better, however to open a new bug such that you appear as the reporter. So, please next open a new bug for this review, with the latest srpm posted. I'll close that one as a duplicate and set the blockers right. Also go on with the sponsorship process, I'll sponsor you.
Thank you a lot, Patrice. Updated spec and src.rpm which include the correct timestamp are available at http://wdl.lug.ro/linux/rpms/ren/ren.spec and http://wdl.lug.ro/linux/rpms/ren/ren-1.0-11.src.rpm respectively. This is the original review, resubmitted as https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211711
Closing this as a duplicate of #Bug 211711. Alan, if you are still interested in submitting other packages to fedora extras and become a fedora contributor, remember to point your potential sponsor at this bug to show you allready did something usefull. *** This bug has been marked as a duplicate of 211711 ***