Bug 731219 (pipebench)

I have created a patch to use in place of the sed. As I have seen some reviewers prefer the patch over sed. Here is the patch if you would like to use it:

https://gitorious.org/ndowens-packager/master/blobs/master/patches/pipebench-0.40-Makefile-CFLAGS.patch

Also in the files section why is it %{_mandir}/man?/ instead of %{_mandir}/man1/

Thank you for your feedback, Nathan. With regards to using the sed invocation instead of the patch you provided, I appreciate the effort but I think it is unncessary. It's just a few search-and-replace changes; and I feel that using a patch instead would be adding another layer of complexity without just cause.

Also, I use the glob for the man pages as a convenience to myself, mostly force of habit, as I'm accustomed to larger packages where using the "man?/" pattern catches the man pages irrespective of the section they're installed into. It's a tiny bit of future-proofing that I think makes things a little nicer aesthetically. If you'd like me to explicitly list each section directory, I'd be happy to adjust that in the spec file. :)

I ran a rpmlint on the x86_64 and it gave me 3 errors originally, first it said that stdin and stdout is misspelled, and in which we know isn't though to get around that error those two words could be capitalized like STDIN and STDOUT and the third error was that the COPYING file was incorrect-fsf-address. I looked that error up and it appears that the COPYING file provided has some issues, so to fix it like I read, I copied the text from the license from GNU's site and pasted it in a new COPYING file.  

I would suggest doing the above, especially on the COPYING file and put in comment that this fixes the issue of incorrect-fsf-address and have notified upstream.

Thank you for the continued feedback, Nathan. I've fixed the STDIN/STDOUT spelling and added a patch to fix the GPLv2 text, which I've sent upstream (and noted in the spec).

Update spec and SRPM are available from my FedoraPeople webspace:
Spec URL: http://pgordon.fedorapeople.org/pipebench.spec
SRPM URL: http://pgordon.fedorapeople.org/pipebench-0.40-2.fc15.src.rpm

What I would do instead of creating a patch for the COPYING file, I would simply include it as another Source, I have pasted the SPEC I have done:
http://pastebin.com/GBj74qZt

(In reply to comment #6)
> What I would do instead of creating a patch for the COPYING file, I would
> simply include it as another Source, I have pasted the SPEC I have done:
> http://pastebin.com/GBj74qZt

Hmm..that's definitely simpler from the patch that I was using. I've updated the package per your suggestion.

Spec: http://pgordon.fedorapeople.org/pipebench.spec
SRPM: http://pgordon.fedorapeople.org/pipebench-0.40-2.fc15.src.rpm

I am looking at the Review guidelines. One is about the License file, http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text, it sounds like upstream needs to provide the correct license for the package. Also would be better to list the source of the license like Source1: http://www.gnu.org/licenses/gpl-2.0.txt in my opinion.

Has upstream replied about the COPYING file yet?

After emailing the devel mailing list, it seems that we can drop Source1 and ignore the incorrect-fsf-address error

So going by the email, remove the Source1 line and remove lines:

## Update the included LICENSE file to match the current FSF GPLv2 text.
## (Fixes the FSF address and updates the "GNU Library GPL" references to "GNU
## Lesser GPL.") Submitted to upstream via email (2011-08-24).
install -D -m 0644 %{SOURCE1} LICENSE
and the %doc line should contain COPYING README

Thanks for the check, Nathan. As Tom Callaway mentioned in his response dated August 5 (which Rahul linked to), "Incorrect FSF address is something that should be reported to upstreams. No further action is required of packagers, although, if a maintainer wishes to correct the address with a patch, they are certainly permitted to do so."

So I believe that the current fix should be OK. I'd prefer to keep the updated GPL text for clarity.

Thanks for your time in reviewing this.

First off, the macros needs to be used consistently, so for example, if you are going to use %{name}, it needs to be used everywhere pipebench is used at, except of course Name:  pipebench :).

Also noticed that during mkdir for man dir: 
mkdir -m 755 -p %{buildroot}%{_mandir}/man1

I would specify that the man dir needs to have executive permission, so I would just do:
mkdir -p %{buildroot}%{_mandir}/man1

Also there is no need of the Group tag any more, though also doesn't say it must be removed, but in the wiki it says there is no use for it any more.

Also when I ran rpmlint on the SPEC file and SRPM, it gave a error with the Source0: and I found out it the Source listed needs to be http://www.habets.pp.se/synscan/files/%{name}-%{version}.tar.gz or w/o macro, whichever way chosen.

And of course changelog and revision needs to be +1 

Sorry, didn't notice these earlier.

Also before I forget:

 pipebench.src:13: W: mixed-use-of-spaces-and-tabs (spaces: line 13, tab: line 1)

Thanks for your patience, Nathan. I've fixed the issues you've mentioned and uploaded 0.40-4 to my FedoraPeople webspace with these changes:

Spec: http://pgordon.fedorapeople.org/pipebench.spec
SRPM: http://pgordon.fedorapeople.org/pipebench-0.40-4.fc15.src.rpm

Regards.

[ndowens@revan rpmbuild]$ rpmlint ~/Downloads/pipebench-0.40-4.fc17.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[ndowens@revan rpmbuild]$ rpmlint SRPMS/pipebench-0.40-4.fc15.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Above is the rpmlint output

I have went through the review process and everything checks out, one macro left unchanged is http://www.habets.pp.se/synscan/programs.php?prog=pipebench
Change pipebench to %{name}

Other than that up the release and this package is then approved after.

Oops - thanks for the catch. I've updated that as you suggested, and posted 0.40-5 to my webspace:

Spec: http://pgordon.fedorapeople.org/pipebench.spec
SRPM: http://pgordon.fedorapeople.org/pipebench-0.40-5.fc15.src.rpm

And thanks for the review/approval - please remember to set the 'fedora-review' flag as appropriate. :)

You need to do the SCM request http://fedoraproject.org/wiki/Package_SCM_admin_requests

Excellent - thanks for the time reviewing this, Nathan!

New Package SCM Request
=======================
Package Name: pipebench
Short Description: Measures the speed of stdin/stdout communication
Owners: pgordon
Branches: f14 f15 f16
InitialCC:

Git done (by process-git-requests).

pipebench-0.40-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/pipebench-0.40-5.fc16

pipebench-0.40-5.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/pipebench-0.40-5.fc15

pipebench-0.40-5.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/pipebench-0.40-5.fc14

pipebench-0.40-5.fc16 has been pushed to the Fedora 16 testing repository.

pipebench-0.40-5.fc15 has been pushed to the Fedora 15 stable repository.

pipebench-0.40-5.fc14 has been pushed to the Fedora 14 stable repository.

pipebench-0.40-5.fc16 has been pushed to the Fedora 16 stable repository.