https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #1 from Michael Schwendt bugs.michael@gmx.net ---

extra care when reviewing its SPEC file

Not really. Header-only C/C++ APIs aren't so special. It's just that there isn't much in the packaging guidelines. Also, there have been package submissions that named the src.rpm "something-devel" because there would be no base package to build. There are more packaging pitfalls with normal lib+header APIs (e.g. sometimes there are headers with a missing lib, or a lib gets deleted after building it, or private headers get installed accidentally and packaged).

Version: 0.0.0 Release: 20130617svn

That's not one of Fedora's package versioning schemes yet: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

Why the triple '0'? Why not a single '0'?

And for snapshots, you'll need the pre-release prefix (for much more flexibility): https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packag...

Release: 0.1.20130617svn

Summary: Embedded LL library

Sum up what it does, not what it's named. The %description is short enough to be suitable as a %summary. Example:

Summary: Header-only C++ library to write EBNF grammars

https://fedoraproject.org/wiki/Examples_of_good_package_summaries (not perfect but a good start)

License: LGPLv3

The source files say "or later" -> LGPLv3+

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_ver...

%build make %{?_smp_mflags}

Since this builds only the test-suite, this "make" invocation could/should be moved into the %check section.

It doesn't use Fedora's compiler flags, btw: https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

# to track the usage of this library Provides: %{name}-static = %{version}-%{release}

Yes, that is not in the guidelines and only helps with tracking (via scripts or repoquery). Basically, any other package can "BuildRequires: ell-devel" and build successfully without any need to make use of to the ell-static Provides. It's the responsibility of the package maintainer(s) to know when to rebuild dependencies against important ELL updates.

Processing files: ell-devel-0.0.0-20130617svn.x86_64

It would be interesting to mention in the spec file that this package does not set "BuildArch: noarch". I assume that is done, so the test-suite will be run for all target archs.

https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #3 from Michael Schwendt bugs.michael@gmx.net ---

IMO you shouldn't add subpackage

Why not?

https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #5 from Michael Schwendt bugs.michael@gmx.net ---

We can

Ah, that's the connection between "can" => "shouldn't".

Yes, it would be possible to not build "ell-devel" but just "ell". There are no naming guidelines about it. The FPC considers it okay when an implicitly development related package doesn't add "-devel" to its name, if there is no need for a package split between run-time and build-time files. E.g. the package of a compiler or similar tool may contain headers (and even libs) without becoming a -devel package.

Whether that is good and convenient also for pure API packages is debatable. The thousands of -devel packages in the collection are considered fully optional and unnecessary at run-time, so one may remove them from an installation for various reasons. So far, it has been possible to use tools like rpmdev-rmdevelrpms or simple RPM queries to do that. The guidelines even comment on that with regard to removing all developer-related packages in an attempt at finding missing build requirements. It's helpful if header-only APIs are stored in -devel packages, too.

And just add virtual provides.

For a non-virtual package name, they are automatic and versioned and even use %_isa. Adding them manually complicates matters. Introducing virtual package names bears a risk, too. Packagers could choose _either_ name to depend on. One packager will use ell-devel out of consistency with other build requirements, another will not even notice the virtual Provides and use "ell". And if you wanted to submit a query on BuildRequires, you would need to consider all available package names. Less convenient.

https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #7 from Michael Schwendt bugs.michael@gmx.net --- Another header-only API has come up, metslib in bug 993456, and there I thought, why not set "BuildArch: noarch" for the -devel subpackage?

What do you think?

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Michael Schwendt bugs.michael@gmx.net changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |bugs.michael@gmx.net Flags| |fedora-review?

--- Comment #9 from Michael Schwendt bugs.michael@gmx.net --- Well, the packaging guidelines don't try to document what RPM can do, but how to package things.

And yes, "BuildArch: noarch" can be set per sub-package (it used to be impossible years ago), with the added responsibility that you don't include any arch-specific packages in such a noarch package accidentally. [A popular case of noarch sub-packages is huge documentation that is split off, btw.]

I don't want to hurry but is my package accepted ?

I'll take another look at set the flags as per: https://fedoraproject.org/wiki/Package_Review_Process

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Michael Schwendt bugs.michael@gmx.net changed:

What |Removed |Added ---------------------------------------------------------------------------- Attachment #785775| |review+ Flags| |

--- Comment #11 from Michael Schwendt bugs.michael@gmx.net --- Created attachment 785775 --> https://bugzilla.redhat.com/attachment.cgi?id=785775&action=edit final spec file modifications

These are the last modifications I would suggest for the spec file:

* the noarch -devel package * an empty %build section, albeit only to silence rpmlint * the dist tag:

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Using_the_.25.7B.3... * preserved time stamps for copied files: https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps * a trailing slash for the included directory tree in %files list (just for increased readability - not mandatory - also see further below)

[...]

With regard to general packaging quality, it would also be an idea to be more explicit about which files to include. For example,

%dir %{_includedir}/ell %{_includedir}/ell/*.h

would make the build fail, if no header files are found, whereas with the current spec,

%{_includedir}/ell/

even an empty directory would result in a successful build. Alternatively, one can add safety/sanity checks to the %check section, too.

I just point this out, because it's a single line in the %install section that copies the headers dir -> a single point of failure.

[...]

The remaining changes to the package may be applied in Fedora package git, so no need to make available another revision here in the review ticket.

APPROVED

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Jonathan De Wachter dewachter.jonathan@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Jon Ciesla limburgher@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Michael Schwendt bugs.michael@gmx.net changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ fedora-cvs?

--- Comment #14 from Michael Schwendt bugs.michael@gmx.net --- Sigh. Bugzilla once again managed to confuse me with its "review" flag it offers when adding an attachment. Obviously, that's not the "fedora-review" flag.

https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #15 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).

https://bugzilla.redhat.com/show_bug.cgi?id=993324

--- Comment #16 from Fedora Update System updates@fedoraproject.org --- ell-0-0.2.20130617svn.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/ell-0-0.2.20130617svn.fc19

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Fedora Update System updates@fedoraproject.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA

--- Comment #18 from Fedora Update System updates@fedoraproject.org --- ell-0-0.2.20130617svn.fc19 has been pushed to the Fedora 19 testing repository.

https://bugzilla.redhat.com/show_bug.cgi?id=993324

Christopher Meng cickumqt@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC|package-review@lists.fedora | |project.org | Flags| |needinfo?(dewachter.jonatha | |n@gmail.com)

--- Comment #20 from Christopher Meng cickumqt@gmail.com --- What are you doing?