In your changelog, you should adjust:
`* Wed Aug 27 2014 David Parsons
<david.parsons> 2.2.1-1`
so that it's one line

The Group tag (Group: Sciences/Biology) are no longer used in Fedora.

The line in %files: %defattr(-,root,root,-) can be dropper, it's no longer used in Fedora.

Similarly, in Fedora the ``rm -rf %{buildroot}`` can be removed at the top of the %install section

That being said, if you intend to package this application for EPEL as well, this last comment does not apply (You'll need rm -rf %{buildroot} at the top of %install)

Thank you Pierre-Yves

I have applied your suggestions except for the last once since I indeed intend to package for EPEL as well.

Here's the new SPEC and SRPM files :
http://parsons.eu/tmp/kissplice.spec
http://parsons.eu/tmp/kissplice-2.2.1-2.fc20.src.rpm

I've incremented the release number though I'm not sure I should have...
Thanks again

Incrementing the release number was good, replacing the entry for 2.2.1-1 in the changelog not :)

The changelog contains a description of the changes that occurred to the spec file across versions and releases.

So at the moment it should look something like:
``
* Fri Aug 29 2014 David Parsons <david.parsons> 2.2.1-2
- Drop the %%defattr(-,root,root,-) line from %%files
- Drop the Group tag
- Fix changelog format

* Wed Aug 27 2014 David Parsons <david.parsons> 2.2.1-1
- Initial version
``

It's important to have 1 blank line between two changelog entries (some tools rely on that).

During the review process you should bump the release and adjust the changelog for every changes that the reviewer ask you to do. This way when you import your spec file in Fedora it will contain the history of the changes made during the review.

Yes, I should have seen that. Well, here come v3 then...

http://parsons.eu/tmp/kissplice.spec
http://parsons.eu/tmp/kissplice-2.2.1-3.fc20.src.rpm

V3 is not enough, v8 might be good ;)

Something unsatisfied:

1. Drop BR gcc-c++.

2. Drop rm -rf %{buildroot}

3. This is superfluous:

%{_exec_prefix}/lib/kissplice/*

This package is not noarch packages(arch independant), we put libs on X86_64 at %{_libdir} aka /usr/lib64.

If your package installs programs to /usr/lib, I think it's wrong, at least should be /usr/bin.

I haven't taken deep look on those files(only from koji), can you tell us what are these?

(In reply to Christopher Meng from comment #5)
> 2. Drop rm -rf %{buildroot}

Maybe you could pay attention to the discussion occurring on a ticket before commenting?

(In reply to Pierre-YvesChibon from comment #6)
> (In reply to Christopher Meng from comment #5)
> > 2. Drop rm -rf %{buildroot}
> 
> Maybe you could pay attention to the discussion occurring on a ticket before
> commenting?

Why do you think that rm -rf %buildroot is needed on EPEL? EPEL6 doesn't need that, the latest EPEL5 allegedly doesn't need it as well.

(In reply to Christopher Meng from comment #7)
> (In reply to Pierre-YvesChibon from comment #6)
> > (In reply to Christopher Meng from comment #5)
> > > 2. Drop rm -rf %{buildroot}
> > 
> > Maybe you could pay attention to the discussion occurring on a ticket before
> > commenting?
> 
> Why do you think that rm -rf %buildroot is needed on EPEL? EPEL6 doesn't
> need that, the latest EPEL5 allegedly doesn't need it as well.

So you may want to check the guidelines: http://fedoraproject.org/wiki/EPEL:Packaging#Prepping_BuildRoot_For_.25install

@David you probably want to check that link as well, and if you are planning on packaging this application for epel5, you probably want to look at the next section ;-)

Hi Christopher and thank you for adding your comments.

I had seen that point 1. (BR gcc-c++) wasn't needed but I thought it was harmless so I had left it in. I will remove it.

For point 2. I'm wondering whether it is useful to target EPEL5 as well as EPEL6. However, if EPEL5 is still being used, I suppose it would be best to support it and hence keep the "rm -rf %{buildroot}" statement in the %install section.

As for point 3. I had read this: http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir (files in lib are indeed "executable programs that are designed primarily to be run by other programs rather than by users") but hadn't actually grasped that %{_libdir} would be /usr/lib64 on X86_64.
I understand I have to change the install dir of these files. However I do not know how to do this provided that upstream do not offer the possibility to choose the lib dir. These files are consistently placed in $prefix/lib/kissplice/ Can you help me with this ?

Thanks,
David

(In reply to David Parsons from comment #10)

> For point 2. I'm wondering whether it is useful to target EPEL5 as well as
> EPEL6.
It's up to your personal perference.

> However, if EPEL5 is still being used, I suppose it would be best to
> support it and hence keep the "rm -rf %{buildroot}" statement in the
> %install section.
IMO, it's much easier, cleaner and less error-prone to remove all legacy rpm stuff from Fedora rpms and re-add it if necessary on the corresponding branches in git.

Hi David,

Ralf's comments are right, IMO less legacy RPM stuffs in SPEC will make it cleaner. Also one more important, if you don't use EPEL, don't maintain packages on that branch.

I can help you by proposing a patch later, you should contact upstream first to get their feedback.

Finally, you need a sponsor first, *sigh... Hope you can get sponsored soon.

OK I will remove "rm -rf %{buildroot}" from %install and add it back into the EPEL5 package when needed.

As for the %{_prefix}/lib vs %{_libdir} (or %{_libexecdir}), I'm a former member of the development team of kissplice so I could try and adapt the build process.
These files would however need to remain in %{_prefix}/lib in debian style distributions and I'm not sure how to achieve this in a clean way with CMake... Also, I'm pretty sure there is a simple way to tell rpm to put in %{_libexecdir} what it will find in %{_prefix}/lib, is there not ?

Thanks,
David

(In reply to Ralf Corsepius from comment #11)
> (In reply to David Parsons from comment #10)
> > However, if EPEL5 is still being used, I suppose it would be best to
> > support it and hence keep the "rm -rf %{buildroot}" statement in the
> > %install section.
> IMO, it's much easier, cleaner and less error-prone to remove all legacy rpm
> stuff from Fedora rpms and re-add it if necessary on the corresponding
> branches in git.

I only partly agree with this, I prefer to review the most complete spec file even if things are later cleaned in git. So if David wants to support EL5, basically I prefer to review the EL5 spec since it will be Fedora compatible anyway...

> Finally, you need a sponsor first, *sigh... Hope you can get sponsored soon.

Unless I'm mistaking, I already see two sponsors on that review :)

(In reply to David Parsons from comment #10)
> As for point 3. I had read this:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir (files in lib
> are indeed "executable programs that are designed primarily to be run by
> other programs rather than by users") but hadn't actually grasped that
> %{_libdir} would be /usr/lib64 on X86_64.

If the files in %{_libexecdir} are meant to be executed but not by the user, then my thoughts are that this folder is appropriate and I don't see the need to move them to %{_libdir}.
If the files are not meant to be executed, then we should look into putting them in %{_libdir}

(In reply to Pierre-YvesChibon from comment #14)

> I only partly agree with this, I prefer to review the most complete spec
> file even if things are later cleaned in git.
I've seen way too many rotten and overly complex specs, to support this thought.

(In reply to Pierre-YvesChibon from comment #16)
> (In reply to David Parsons from comment #10)
> > As for point 3. I had read this:
> > http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir (files in lib
> > are indeed "executable programs that are designed primarily to be run by
> > other programs rather than by users") but hadn't actually grasped that
> > %{_libdir} would be /usr/lib64 on X86_64.
> 
> If the files in %{_libexecdir} are meant to be executed but not by the user,
> then my thoughts are that this folder is appropriate and I don't see the
> need to move them to %{_libdir}.
> If the files are not meant to be executed, then we should look into putting
> them in %{_libdir}

The files are indeed meant to be executed by a program (not directly by the user). However, they are not in %{_libexecdir} but in %{_prefix}/lib. This conforms to FHS and debian packaging guidelines but apparently not quite to fedora's...
Would it be as simple as adding "mv %{_prefix}/lib %{_libexecdir}" somewhere?

(In reply to David Parsons from comment #18)
> (In reply to Pierre-YvesChibon from comment #16)
> > (In reply to David Parsons from comment #10)
> > > As for point 3. I had read this:
> > > http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir (files in lib
> > > are indeed "executable programs that are designed primarily to be run by
> > > other programs rather than by users") but hadn't actually grasped that
> > > %{_libdir} would be /usr/lib64 on X86_64.
> > 
> > If the files in %{_libexecdir} are meant to be executed but not by the user,
> > then my thoughts are that this folder is appropriate and I don't see the
> > need to move them to %{_libdir}.
> > If the files are not meant to be executed, then we should look into putting
> > them in %{_libdir}
> 
> The files are indeed meant to be executed by a program (not directly by the
> user). However, they are not in %{_libexecdir} but in %{_prefix}/lib. This
> conforms to FHS and debian packaging guidelines but apparently not quite to
> fedora's...

Each project has its own way to handle files. And I don't agree with Debian all the time.

> Would it be as simple as adding "mv %{_prefix}/lib %{_libexecdir}" somewhere?

You can try and checkout to see if it works as its default behavior. Putting something in /usr/lib directly is the fault from upstream as they don't eithor respect the libdir parameter which could be defined during CMake or take into account of /lib64 on some OS, e.g Fedora, RHEL.

Hi,

I've submitted a patch to upstream that fixes the build issue. I was even lucky enough to come across the team leader yesterday, they will talk about it at their next meeting on friday. I'm confident it will be included in the next release.

In the meantime I've tried the solution I mentionned above (which is actually "mv %{buildroot}/%{_prefix}/lib %{buildroot}/%{_libexecdir}") and it looks like it's working just fine.

Here's the new files 
http://parsons.eu/tmp/kissplice.spec
http://parsons.eu/tmp/kissplice-2.2.1-4.fc20.src.rpm

Thanks,
David

David - are you still interested in this after all these years?

Dear Orion,

Following your question, I've asked upstream what they thought. Since I've had no answer and it's been 6 months+, I suppose we can consider they are not interested.

Thank you for your consideration,
David