Bug 462982

Summary: Review Request: buffer - General purpose buffer program
Product: [Fedora] Fedora Reporter: Bruno Cornec <bruno.cornec>
Component: Package ReviewAssignee: manuel wolfshant <manuel.wolfshant>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bruno_cornec, bruno.cornec, fedora-package-review, itamar, notting, ondrejj, tim, tomspur, vanmeeuwen+fedora
Target Milestone: ---Flags: manuel.wolfshant: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-01-04 02:39:00 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Bruno Cornec 2008-09-19 23:55:41 UTC
Spec URL: ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec
SRPM URL: ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-1.fc9.src.rpm
Description: General purpose buffer program

Comment 1 Jan ONDREJ 2008-09-20 05:54:01 UTC
Fix licence. Source code says: "either version 1, or (at your option) any later version.". According to FedoraLicenses, it must be GPL+, not GPLv2+.

Fix "Source:" path. Your URL points to different URL like Source. I think your URL is OK, but you have to change Source to original path, not path to your local site.

Fix BuildRoot, use any of these: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
Your path is similar, but not same as described here.

[ondrejj@builder result]$ rpmlint *.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Builds well on i386, x86_64, f8, f9 and devel.

Other personal suggestions:
  01-debian-patches.all.gz does not look as an Fedora patch. Please, can you rename it and remove debian specific parts?

I don't see more problems in this package. Fix described problems and I can approve this package.

Comment 2 Mamoru TASAKA 2008-09-20 06:14:39 UTC
(In reply to comment #1)
> Other personal suggestions:
>   01-debian-patches.all.gz does not look as an Fedora patch. Please, can you
> rename it and remove debian specific parts?

I think that it is rather preferred to write what the used patches comes from
if available:
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
Some packages I maintain contains comments like
# patch from debian

Moreover there are some cases in which the upstream of the project
is debian.

Comment 3 Bruno Cornec 2008-09-20 15:44:54 UTC
(In reply to comment #1)
> Fix licence. Source code says: "either version 1, or (at your option) any later
> version.". According to FedoraLicenses, it must be GPL+, not GPLv2+.

Ok. I based my deduction on the COPYING file provided which says v2.
Will change to GPL+.

> Fix "Source:" path. Your URL points to different URL like Source. I think your
> URL is OK, but you have to change Source to original path, not path to your
> local site.

BTW do I need to have a URL for the Source: Tag ? Can I just use the file name only ?
> 
> Fix BuildRoot, use any of these:
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> Your path is similar, but not same as described here.

You mean changing %(id -u -n) (which was a previous Fedora recommandation btw) or using mktemp ?

> Other personal suggestions:
>   01-debian-patches.all.gz does not look as an Fedora patch. Please, can you
> rename it and remove debian specific parts?

It's jut a patch tha is in the Debian package, and that helps removing compiler warnings. Should it be named 01-gcc-waranings instead ?

> I don't see more problems in this package. Fix described problems and I can
> approve this package.

Ok, will do as soon as I understand better what to fix wrt above questions. Thanks for your help.

Comment 4 Jan ONDREJ 2008-09-20 16:01:49 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Fix "Source:" path. Your URL points to different URL like Source. I think your
> > URL is OK, but you have to change Source to original path, not path to your
> > local site.
> 
> BTW do I need to have a URL for the Source: Tag ? Can I just use the file name
> only ?

Yes, you need to have an URL and why do not use upstream URL?
What is wrong with him?

> > Fix BuildRoot, use any of these:
> > http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> > Your path is similar, but not same as described here.
> 
> You mean changing %(id -u -n) (which was a previous Fedora recommandation btw)
> or using mktemp ?

Select any from packaging guidelines. May be your version was good for older packages, current guidelines requires to select one of these on BuildRoot_tag page.

> > Other personal suggestions:
> >   01-debian-patches.all.gz does not look as an Fedora patch. Please, can you
> > rename it and remove debian specific parts?
> 
> It's jut a patch tha is in the Debian package, and that helps removing compiler
> warnings. Should it be named 01-gcc-waranings instead ?

Please ignore this my comment and see comment from Mamoru Tasaka. He is a better reviewer like me. :-)
You should ignore my comment about this patch completely.

So you have to fix only:
  - license
  - source url path
  - buildroot

Comment 5 Bruno Cornec 2008-09-20 22:11:22 UTC
(In reply to comment #4)
> Yes, you need to have an URL and why do not use upstream URL?
> What is wrong with him?

Nothing. Just asking. It's now fixed.

> Select any from packaging guidelines. May be your version was good for older
> packages, current guidelines requires to select one of these on BuildRoot_tag
> page.

Ok; took the mktemp one

> So you have to fix only:
>   - license
>   - source url path
>   - buildroot

New versions at the same place:
Spec URL: ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec
SRPM URL: ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-1.fc9.src.rpm

Comment 6 Jan ONDREJ 2008-09-21 07:04:48 UTC
One comment:
  please, add a new Changelog entry every package update. Increase release number and write, what has been done.

Hmm, package looks OK, but I am looking, that you are not in packager group.
I can approve this package, but you can't build it.

You have to request to be member of packager group, but you need more packages or reviews to be approved to this group. (I am not a sponsor and can't approve you).

Comment 7 Bruno Cornec 2008-09-22 23:45:19 UTC
Well, as for the moment, this was a study, I didn't consider that keeping any history was worthwhile. I really think that the first published version will be the -1 tag. After that, for every update, of course I'll publish a -2 and log.

Concerning the package group, I don't know how to proceed now. I have 5 packages that I want to add to Fedora. buffer and afio are the 2 most ready. After that, I'll have to come back to mindi/mondo which have been proposed a long time ago.

Can someone help going further ?

THanks in advance,
Bruno.

Comment 8 Jan ONDREJ 2008-09-23 05:30:15 UTC
Mondo/mindi is a good software. I am interested to help with packaging, but I think I can't from this point.

There is a FE-NEEDSPONSOR blocker on this package too, so sponsors will see your package.

Removing me from this package (assigned and review flag) so this package will be free for sponsors, which can sponsor you and then you can build your packages (all you packages are unapproved yet). I am still in CC list.

If you will be sponsored, and this package still needs approval, just write here a message, I can approve this package.

If you want to be sponsored faster, make some reviews for other packages:
  http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

Comment 9 manuel wolfshant 2008-09-23 14:47:36 UTC
I'll take care of this. For the moment there are a few issues:
1. major one: during compilation the mandatory gcc flags as imposed by Fedora (http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags) are not used. Instead compilation is done with  -Wall -O6 -fomit-frame-pointer 

2. minor issues:
- I see no point in applying all the debian patches. We have no need for the content of the debian folder. You do not package it, which is OK, but I would have adjusted the patches to better fit Fedora. Not a blocker, feel free to leave it as it is.
- there is a missing "s" in the Summary(fr) line (des bande -> des bandes)

Comment 10 Bruno Cornec 2008-10-02 15:24:44 UTC
(In reply to comment #9)
> I'll take care of this. For the moment there are a few issues:
> 1. major one: during compilation the mandatory gcc flags as imposed by Fedora
> (http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags) are not
> used. Instead compilation is done with  -Wall -O6 -fomit-frame-pointer

Fixed.

> 
> 2. minor issues:
> - I see no point in applying all the debian patches. We have no need for the
> content of the debian folder. You do not package it, which is OK, but I would
> have adjusted the patches to better fit Fedora. Not a blocker, feel free to
> leave it as it is.

I chose to keep the debian specific stuff, in order to be able to easily back port further Debian patches (which are useful also for the Fedora package) in the future if needed. Removing the creation of the debian content would make a different patch.
If it's not a big issue I'd rather keep it like that.

> - there is a missing "s" in the Summary(fr) line (des bande -> des bandes)

Fixed as well.

Please look at ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec and ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-1.fc9.src.rpm

Comment 11 manuel wolfshant 2008-10-02 18:29:11 UTC
In order to make tracking easier for everyone, please bump the release tag each time you modify the spec file (and _add_ a corresponding changelog entry, not _replace_ the previous one as you have done). For instance


 %changelog
* Thu Oct 02 2008 Bruno Cornec <bruno> 1.19-2.fc9
- fix compilation flags

* Sat Sep 20 2008 Bruno Cornec <bruno> 1.19-1.fc9
- Updated to 1.19


as of 02.oct.2008, ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec still has the missing "s" in  Summary(fr).

Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: devel/x86_64
 [x] Rpmlint output:
source RPM: empty
binary RPM:empty
 [x] Package is not relocatable.
 [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -XXXXXX))
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal
section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: GPL+
 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing th
e text of the license(s) for the package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package match the upstream source, as provided in the spec URL.
     SHA1SUM of package: a2bb4ed163cb166bf54a1ba341c8d1fcba48f271 buffer-1.19.tar.gz
 [x] Package is not known to require ExcludeArch
 [-] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packag
ing Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [-] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [x] Description and summary sections in the package spec file contains translations for supported Non-English languages, i
f available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64
 [?] Package should compile and build into binary rpms on all supported architectures.
     Tested on:
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.


=== Issues ===
1.Summary(Fr) should be fixed before uploading to CVS

================
*** APPROVED ***
================


I will take a look at your other packages and if satisfied I will sponsor you.

Comment 12 Bruno Cornec 2008-10-03 12:14:19 UTC
(In reply to comment #11)
> In order to make tracking easier for everyone, please bump the release tag each
> time you modify the spec file (and _add_ a corresponding changelog entry, not
> _replace_ the previous one as you have done). For instance
> 

Ok, the next one is -3 now so.

>  %changelog
> * Thu Oct 02 2008 Bruno Cornec <bruno> 1.19-2.fc9
> - fix compilation flags
> 
> * Sat Sep 20 2008 Bruno Cornec <bruno> 1.19-1.fc9
> - Updated to 1.19

Ok. Will try to do that properly for -3 tag.
In fact I have a fake changelog I generate for test packages, and a real one for  official packages. Now we are approaching that step, it makes sense.

> as of 02.oct.2008, ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec still
> has the missing "s" in  Summary(fr).

My mistake, I wasn't looking at the right line.


For my own knowledge what is the tool you're using for conformity checking. I just know rpmlint. What else exists ?
 
> 
> I will take a look at your other packages and if satisfied I will sponsor you.

Thanks.

So new versions available at:
 ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec
and ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-3.fc9.src.rpm

Comment 13 Bruno Cornec 2008-11-17 18:57:55 UTC
Any feedback on my latest package version ?

Comment 14 manuel wolfshant 2008-11-17 21:02:56 UTC
Sorry for the delay, I was hoping to see some progress on the other packages you have submitted.
Please let me know your FAS account and I will sponsor you.

PS1: please ditch the ending ".fc9" from the version in changelog, otherwise building on other versions than F9 will lead to the following warning from rpmlint:
buffer.x86_64: W: incoherent-version-in-changelog 1.19-3.fc9 ['1.19-3.fc10', '1.19-3']

Comment 15 Bruno Cornec 2008-12-12 09:09:03 UTC
The problem is that afio is blocked for a cumbersome license issue :-(

I remade a version ditching the suffix as you proposed available at  ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec
and ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-3.fc9.src.rpm

My FAS account is bcornec
Thanks for your help.

Comment 16 manuel wolfshant 2008-12-12 10:34:11 UTC
Please be as kind as to ask again for approval to join the packager group, I cannot locate your account in the current queue.

as of afio: I know, I am watching all your bugs. but I think that the fact that a license is not acceptable for fedora has nothing to do with packaging skills.


obs: you might wish to try to push afio in rpmfusion.

Comment 17 Bruno Cornec 2008-12-12 14:48:27 UTC
Ok. Forgot again to bump the tag.

So here it is:
ftp://ftp.mondorescue.org/test/fedora/9/buffer.spec
and ftp://ftp.mondorescue.org/test/fedora/9/buffer-1.19-4.fc9.src.rpm

I asked to join the packager group.

I'll also try to remove the dependency on afio as in fact star can be used also, and star is already a fedora package (even if the license is also not that standard).

And I'll add a mindi-busybox submission, or work upstream to remove the dependency.

Comment 18 manuel wolfshant 2008-12-12 15:01:51 UTC
You are doomed, sorry, I meant sponsored, now.
You can proceed with requesting CVS branching.

Feel free to ask me for guidance if you need, I'll be glad to help.

Comment 19 Bruno Cornec 2008-12-12 17:53:19 UTC
IIUC, I should see on that bug report a fedora-cvs with a combo near it, now that I'm a member of the fedorabugs group isn't it ?

however, I do not seem to be able to get that flag. Is there anything more I need to do ?

Thanks for your help Manuel.

Comment 20 Jason Tibbitts 2008-12-12 18:19:18 UTC
The sync between the account system happens hourly, so at worst case you may need to wait two hours.  If it doesn't happen soon, though, we'll have someone investigate.

Comment 21 Bruno Cornec 2008-12-14 19:16:52 UTC
When trying to edit flags, I now see the fedora-cvs one. However, the combobox s grey, and I can't put the question mark in it.

Comment 22 Dennis Gilmore 2008-12-15 20:55:30 UTC
without the review flag set to + we are unable to process the cvs request.  Reveiwer please fix and set the cvs flag back to ?

Comment 23 manuel wolfshant 2008-12-16 10:40:39 UTC
I am sorry, I thought I have set it to "+" back in october.

New Package CVS Request
=======================
Package Name:buffer
Short Description: General purpose buffer program
Owners: bcornec
Branches: F9 F10
InitialCC: wolfy

Comment 24 Kevin Fenzi 2008-12-17 22:12:45 UTC
cvs done.

Comment 25 Thomas Spura 2009-10-07 23:08:06 UTC
ping: bcornec, wolfy

any update on this? If you don't plan to add these packages, cvs can be deleted again…

How to commit it to cvs is described at:
http://fedoraproject.org/wiki/PackageMaintainers/UpdatingPackageHowTo

Comment 26 Bruno Cornec 2009-10-07 23:51:41 UTC
Yes, I still plan to ork on this. I'm waiting for an approval by HP OSRB to be able to work on this. Shouldn't take too long now.

Comment 27 Thomas Spura 2010-01-04 02:39:00 UTC
This is build in F-11, F-12 and rawhide.

Closing.