first round of comments
- drop Group: - it's ignored
- drop Packager: - filled by the build system
- drop %clean - rpmbuild does it automagically
- drop all %attr() - rpmbuild set them automagically
- License tag must use Fedora identifiers - "LGPLv2 or BSD"
- %{_libdir}/libbpf.so belongs to -devel subpackage
- -devel subpackage should own %{_includedir}/bpf directory, you might use wildcard to include the individual headers, or even list only the "bpf" directory
- ideally the static library shouldn't be packaged, if it's a must, then it belong to a separate -static subpackage (https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries)
- omit %dist in changelog

Why is this superseding libbpf in kernel-tools? Won't it break compatibility with the official kernel?

Adding the maintainer of kernel-tools to the loop.

(In reply to Iñaki Ucar from comment #2)
> Why is this superseding libbpf in kernel-tools? Won't it break compatibility
> with the official kernel?
> 
> Adding the maintainer of kernel-tools to the loop.

the true source of libbpf is kernel tree, that's why it
initially landed in kernel-tools with other kernel tree
libraries

however, upstream team decided to keep and maintain GitHub libbpf
sources (which are regirarly sync from kernel tree) to maintain
compatibility in between distributions and mainly to have stable
relases of libbpf based on github tree tags rather then on random
kernel versions (from libbpf point of view)

jirka

For the record, it was Jiri who introduced the libbpf subpackage in kernel-tools.

(In reply to Jiri Olsa from comment #3)
> however, upstream team decided to keep and maintain GitHub libbpf
> sources (which are regirarly sync from kernel tree) to maintain
> compatibility in between distributions and mainly to have stable
> relases of libbpf based on github tree tags rather then on random
> kernel versions (from libbpf point of view)

But that GitHub mirror follows bpf-next, right? So the question is whether it could happen that an update of the standalone libbpf has a soname bump that makes it incompatible with the kernel versions packaged in Fedora (which cannot happen if libbpf is a subpackage of kernel-tools, because it has the same versioning and provides from the same "snapshot").

(In reply to Iñaki Ucar from comment #5)
> (In reply to Jiri Olsa from comment #3)
> > however, upstream team decided to keep and maintain GitHub libbpf
> > sources (which are regirarly sync from kernel tree) to maintain
> > compatibility in between distributions and mainly to have stable
> > relases of libbpf based on github tree tags rather then on random
> > kernel versions (from libbpf point of view)
> 
> But that GitHub mirror follows bpf-next, right? So the question is whether

right

> it could happen that an update of the standalone libbpf has a soname bump
> that makes it incompatible with the kernel versions packaged in Fedora
> (which cannot happen if libbpf is a subpackage of kernel-tools, because it
> has the same versioning and provides from the same "snapshot").

well at the moment nothing depends on libbpf and libbpf itself
checks on kernel features before using them, so I don't see any
possible incompatibility issue

new libbpf has the 'Epoch:1' set, so it takes over old libbpf smoothly

jirka

(In reply to Dan Horák from comment #1)
> first round of comments
> - drop Group: - it's ignored
> - drop Packager: - filled by the build system
> - drop %clean - rpmbuild does it automagically
> - drop all %attr() - rpmbuild set them automagically
> - License tag must use Fedora identifiers - "LGPLv2 or BSD"
> - %{_libdir}/libbpf.so belongs to -devel subpackage
> - -devel subpackage should own %{_includedir}/bpf directory, you might use
> wildcard to include the individual headers, or even list only the "bpf"
> directory
> - ideally the static library shouldn't be packaged, if it's a must, then it
> belong to a separate -static subpackage
> (https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-
> libraries)
> - omit %dist in changelog

I put new version (spec and srpm) in here:
  http://people.redhat.com/~jolsa/libbpf/v2/

thanks,
jirka

further comments
- missing BuildRequires: gcc
- fails to rebuild on ppc64le, because it installs the libs into /usr/lib/ - https://github.com/libbpf/libbpf/blob/master/src/Makefile#L57 is wrong
- distro-wide CFLAGS and LDFLAGS are not used, adding CFLAGS=%{build_cflags} LDFLAGS=%{build_ldflags} to "make" might help

(In reply to Dan Horák from comment #8)
> further comments
> - missing BuildRequires: gcc
> - fails to rebuild on ppc64le, because it installs the libs into /usr/lib/ -
> https://github.com/libbpf/libbpf/blob/master/src/Makefile#L57 is wrong
> - distro-wide CFLAGS and LDFLAGS are not used, adding CFLAGS=%{build_cflags}
> LDFLAGS=%{build_ldflags} to "make" might help

new version is in here:
  http://people.redhat.com/~jolsa/libbpf/v3/

thanks,
jirka

formal review is here, see the notes explaining OK* and BAD statuses below:

OK      source files match upstream:
            105e4914bc7b9cb29a5b81be51693e7567405478  v0.0.3.tar.gz
OK      package meets naming and versioning guidelines.
OK      specfile is properly named, is cleanly written and uses macros consistently.
OK      dist tag is present.
OK      license field matches the actual license.
OK*     license is open source-compatible (LGPLv2 or BSD). License text not included in package.
OK      latest version is being packaged.
OK      BuildRequires are proper.
OK      compiler flags are appropriate.
OK      package builds in mock (Rawhide/ppc64le).
OK      debuginfo package looks complete.
OK*     rpmlint is silent.
OK      final provides and requires look sane.
N/A     %check is present and all tests pass.
OK      shared libraries are added to the regular linker search paths.
OK      owns the directories it creates.
OK      doesn't own any directories it shouldn't.
OK      no duplicates in %files.
OK      file permissions are appropriate.
OK      no scriptlets present.
OK      code, not content.
OK      documentation is small, so no -docs subpackage is necessary.
OK      %docs are not necessary for the proper functioning of the package.
OK      headers in devel subpackage
OK      pkgconfig files in devel subpackage
OK      no libtool .la droppings.
OK      not a GUI app.

- the license texts should included in the source archive and then distributed with the built rpms
- rpmlint output is harmless
libbpf-devel.ppc64le: W: no-documentation
libbpf-static.ppc64le: W: no-documentation
libbpf.ppc64le: W: name-repeated-in-summary C Libbpf
libbpf.ppc64le: W: spelling-error %description -l en_US bpf -> bf, pf, bps
libbpf.ppc64le: W: incoherent-version-in-changelog 0.0.3-1 ['1:0.0.3-1.fc32', '1:0.0.3-1']
libbpf.ppc64le: W: no-documentation
libbpf.src: W: name-repeated-in-summary C Libbpf
libbpf.src: W: spelling-error %description -l en_US bpf -> bf, pf, bps
6 packages and 0 specfiles checked; 0 errors, 8 warnings.

Two nitpicks
- you can drop the slash from the LIBDIR definition, the "dir" macros start with a slash
- you can drop the redefinition of OBJDIR in the make_flags, actually it makes it worse when doing a local "rpmbuild" (it puts files into /home/dan/rpmbuild/BUILD which isn't cleaned up). It works well for me when the default is used.

The package is APPROVED.

thanks, requesting repo in here:
https://pagure.io/releng/fedora-scm-requests/issue/17074

jirka

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libbpf

This package was approved and imported in repositories, but this review ticket was never closed.
I'm closing it now.