Bug 2525860

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

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======
- Permissions on files are set properly.
  Note: See rpmlint output
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_file_permissions

  libbf.aarch65: E: non-standard-executable-perm /usr/lib64/libbf.so.0.2025.06.03 744
  libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfcalc 744
  libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfpi 744

  Mode 744 means u=rwx,g=r,o=r. These files should be executable for all users,
  not only for root. Use mode 755 instead. Since this is the default for the
  install command, you can just write:

    install -D libbf.so.0.2025.06.03 -t %{buildroot}/%{_libdir}
    […]
    install -D bfcalc bfpi -t %{buildroot}/%{_bindir}

- The executable bfpi is not fully compiled with the distribution compiler
  flags; rpmlint flags this because it isn’t a PIE:

    libbf-examples.aarch64: W: position-independent-executable-suggested /usr/bin/bfpi

  Fix this by adding ${CFLAGS} to this line:

    ${CC} bfpi.o -I libbf.h -I%{_includedir} -L. libbf.so.0.2025.06.03 -lm -o bfpi

- You should add a comment indicating why bftest is built only on x86_64. I
  tried it on aarch64 and got the following error:

    + gcc -O2 -flto=auto -ffat-lto-objects -fexceptions -g
      -grecord-gcc-switches -pipe -Wall -Werror=format-security
      -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
      -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
      -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
      -mbranch-protection=standard -fasynchronous-unwind-tables
      -fstack-clash-protection -fno-omit-frame-pointer
      -mno-omit-leaf-frame-pointer -fPIC -c bftest.c softfp.c
    In function ‘get_cycles’,
        inlined from ‘mpfr_exec_op’ at bftest.c:518:17:
    bftest.c:446:5: error: impossible constraint in ‘asm’
      446 |     asm volatile("rdtsc" : "=a" (low), "=d" (high));
          |     ^~~
    In function ‘get_cycles’,
        inlined from ‘mpfr_exec_op’ at bftest.c:617:17:
    bftest.c:446:5: error: impossible constraint in ‘asm’
      446 |     asm volatile("rdtsc" : "=a" (low), "=d" (high));
          |     ^~~
    bftest.c:446:5: error: impossible constraint in ‘asm’
    bftest.c:446:5: error: impossible constraint in ‘asm’
    bftest.c:446:5: error: impossible constraint in ‘asm’
    bftest.c:446:5: error: impossible constraint in ‘asm’

  This line is x86-specific assembly, which is an adequate explanation for why
  it cannot be built on other architectures.

  It’s likely that it would be possible to build and run these tests portably
  if the benchmarking/cycle-counting code were patched out. I think that this
  is worth investigating, but shouldn’t be required for approval.

- This line is both unnecessary (the local unversioned symlink is not used in
  %build, and a separate one is created in %install) and incorrect (the symlink
  is broken because the link target starts with “libbpf” instead of “libbf”).

    ln -s libbpf.so.0.2026.06.03 libbf.so

- I allowed bftest to run for something like two days on a relatively fast
  workstation, and it still hadn’t finished! This is not reasonable for builds
  in Fedora infrastructure.

  Fortunately, you can use the -S option for bftest

    -S         single iteration of tests

  to run a short test, which will still give you some confidence without taking
  an excessive amount of time.

  You can consider whether or not you want to use

    -s seed    set the initial seed

  to make the tests entirely reproducible. It shouldn’t matter if there are no
  bugs…

- The version number is repeated in a lot of places and forms. Consider having
  one source of truth, and deriving the others from it. Maybe something like
  this?

    %global tar_version 2025-06-03
    # https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning
    %global so_version 0.1
    %global dotted_version %(printf '%s\n' '%{tar_version}' | tr - .)
    […]
    Version:        %(printf '%s\n' '%{tar_version}' | tr -d '-')
    […]
    Source:         %{url}/libbf-%{tar_version}.tar.gz
    […]
    %autosetup -n libbf-%{tar_version}
    […]
    ${CC} ${LDFLAGS} -shared libbf.o cutils.o -Wl,-soname,libbf.so.%{so_version} \
        -o libbf.so.%{so_version}.%{dotted_version}
    […]
    ${CC} ${CFLAGS} bfpi.o -I libbf.h -I%{_includedir} -L. \
        libbf.so.%{so_version}.%{dotted_version} -lm -o bfpi
    ${CC} ${LDFLAGS} bfcalc.o readline_tty.o readline.o -I libbf.h -L. \
        libbf.so.%{so_version}.%{dotted_version} -lm -o bfcalc
    […]
    ${CC} ${LDFLAGS} bftest.o softfp.o -I libbf.h -L. \
        libbf.so.%{so_version}.%{dotted_version} -lmpfr -lgmp -lmpdec -lm -o bftest

  There are many ways to handle this, but having to adjust the version number
  separately in  many places and forms throughout the spec file when upstream
  eventually updates seems unnecessarily frustrating.

- The downstream SONAME version should not be 0, but 0.n, where n is a small
  integer, e.g. 0.1, 0.2, and so on. This reduces the likelihood of conflicting
  with an initial upstream SONAME version, should upstream start producing
  shared libraries in the future.

  Also, you are required to contact upstream and ask them to start versioning
  it. I see that upstream’s Makefile doesn’t even make a proper static (.a)
  library, let alone a versioned or unversioned shared libary, and it seems
  likely that upstream has a “we provide sources, you provide a build system”
  philosophy, but this is written as a MUST in the guidelines, so an attempt
  needs to be made and documented.

  https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning

- I think it would be worth adding some comments about why you aren’t using the
  upstream Makefile, and pointing out that the manual compiler invocations are
  generally based on the Makefile. (Same for the %check section.) It’s pretty
  easy to see some deficiencies in the upstream Makefile from our perspective:
  doesn’t build a libbf library, doesn’t have a way to pass through distro
  compiler flags, and so on.

  I think the best outcome would be if upstream could be convinced to accept
  improvements to their Makefile so that distro compiler flags could be
  supplied and a versioned shared library could be built. I can understand that
  this might be unlikely due to a lack of alignment with upstream priorities.

  Failing that, it would be nice if you could get by with a smallish patch for
  the Makefile, or perhaps just a concise downstream Makefile written from
  scratch to do just what you need.

  In the end, the build is simple enough that the fully manual build you’re
  doing seems acceptable; it would just benefit from a little documentation.

- The chosen MIT license requires that the license text be distributed with
  copies, and this should be in a separate file marked with %license. You
  should ask upstream to include a proper license file for clarity. The files
  in the source archive nearly all have MIT license texts in comments, but they
  have a wide variety of copyright statements. If you can’t get upstream to do
  this, you can choose to make an educated guess: at least the parts of the
  license headers in the source files other than the copyright statements seem
  to be consistent.

  https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text

- All install command should use the -p or --preserve-timestamps option. See
  https://docs.fedoraproject.org/en-US/packaging-guidelines/#_timestamps.
  
===== Notes (no change required) =====

- Rpmlint mentions that man pages would be nice. These aren’t required, though,
  and they would have to be hand-written since help2man doesn’t work here. It’s
  fine to omit them.

    libbf-examples.aarch64: W: no-manual-page-for-binary bfcalc
    libbf-examples.aarch64: W: no-manual-page-for-binary bfpi

- On aarch64, rpmlint flags that bfpi links libm but doesn’t use any functions
  from it. This kind of “overlinking” is harmless, except for a tiny
  performance penalty. In principle, you could resolve it by dropping “-lm”
  from:

    ${CC} bfpi.o -I libbf.h -I%{_includedir} -L. libbf.so.0.2025.06.03 -lm -o bfpi

  …except that it turns out that libm *is* needed (for at least ceil) on
  x86_64. I think it’s better to leave it in, and overlink on some
  architectures, than to overcomplicate matters by trying to link it only on
  particular architectures.

- You don’t need to number the sole Source; you can replace “Source0:” with
  just “Source:”

- In %check, it’s safe enough to assume that there isn’t already an
  LD_LIBRARY_PATH set. Instead of e.g.

    LD_LIBRARY_PATH=%{buildroot}/%{_libdir}:${LD_LIBRARY_PATH} ./bfpi 1e5 pi_1e5.txt

  you could just write

    LD_LIBRARY_PATH=%{buildroot}/%{_libdir} ./bfpi 1e5 pi_1e5.txt

  or even

    export LD_LIBRARY_PATH=%{buildroot}/%{_libdir}

  at the beginning of %check.

- I guess the convention where you make a symlink libbf.so.0.2025.06.03 that’s
  .so.${SONAME_VERSION}.${UPSTREAM_VERSION} is OK, and not hurting anything,
  but I am not sure there is any point to making a symlink like this. Does it
  follow any standard convention, and will anything use it? Perhaps just the
  .so.${SONAME} symlink is enough?

- Since two BuildRequires are only used for bftest, you could conditionalize
  them the same way as bftest, i.e.,

    %ifarch x86_64
    # For bftest:
    BuildRequires:  mpfr-devel
    BuildRequires:  mpdecimal-devel
    %endif

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "MIT License". 8 files have unknown
     license. Detailed output of licensecheck in
     /home/ben/fedora/review/2525860-libbf/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.

     There is no separate license file.

[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries or specifies bundled libraries
     with Provides: bundled(<libname>) if unbundling is not possible.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines

     (except as mentioned)

[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: The License field must be a valid SPDX expression.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 7649 bytes in 2 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.

     Tests pass. Command-line tools seem to work.

[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.

     However, bftest takes too long without the -S option.

[!]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: libbf-20250603-1.fc46.aarch64.rpm
          libbf-devel-20250603-1.fc46.aarch64.rpm
          libbf-examples-20250603-1.fc46.aarch64.rpm
          libbf-20250603-1.fc46.src.rpm
============================ rpmlint session starts ============================
rpmlint: 2.8.0
configuration:
    /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpj3zch8ys')]
checks: 32, packages: 4

libbf-examples.aarch64: W: position-independent-executable-suggested /usr/bin/bfpi
libbf.aarch64: E: non-standard-executable-perm /usr/lib64/libbf.so.0.2025.06.03 744
libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfcalc 744
libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfpi 744
libbf-examples.aarch64: W: no-manual-page-for-binary bfcalc
libbf-examples.aarch64: W: no-manual-page-for-binary bfpi
 4 packages and 0 specfiles checked; 3 errors, 3 warnings, 24 filtered, 3 badness; has taken 0.6 s 




Rpmlint (debuginfo)
-------------------
Checking: libbf-debuginfo-20250603-1.fc46.aarch64.rpm
          libbf-examples-debuginfo-20250603-1.fc46.aarch64.rpm
============================ rpmlint session starts ============================
rpmlint: 2.8.0
configuration:
    /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpo6nlctg_')]
checks: 32, packages: 2

 2 packages and 0 specfiles checked; 0 errors, 0 warnings, 17 filtered, 0 badness; has taken 0.3 s 





Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.10.0
configuration:
    /usr/lib/python3.15/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 33, packages: 5

libbf-examples.aarch64: W: unused-direct-shlib-dependency /usr/bin/bfpi /lib64/libm.so.6
libbf-examples.aarch64: W: position-independent-executable-suggested /usr/bin/bfpi
libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfcalc 744
libbf-examples.aarch64: E: non-standard-executable-perm /usr/bin/bfpi 744
libbf.aarch64: E: non-standard-executable-perm /usr/lib64/libbf.so.0.2025.06.03 744
libbf-examples.aarch64: W: no-manual-page-for-binary bfcalc
libbf-examples.aarch64: W: no-manual-page-for-binary bfpi
 5 packages and 0 specfiles checked; 3 errors, 4 warnings, 39 filtered, 3 badness; has taken 0.7 s 



Source checksums
----------------
https://bellard.org/libbf/libbf-2025-06-03.tar.gz :
  CHECKSUM(SHA256) this package     : 4b23394d67a4a3c3266a38b66831094523fffb5d01974a5c05a0327b36b0a340
  CHECKSUM(SHA256) upstream package : 4b23394d67a4a3c3266a38b66831094523fffb5d01974a5c05a0327b36b0a340


Requires
--------
libbf (rpmlib, GLIBC filtered):
    ld-linux-aarch64.so.1()(64bit)
    libc.so.6()(64bit)
    rtld(GNU_HASH)

libbf-devel (rpmlib, GLIBC filtered):
    libbf(aarch-64)
    libbf.so.0()(64bit)

libbf-examples (rpmlib, GLIBC filtered):
    ld-linux-aarch64.so.1()(64bit)
    libbf(aarch-64)
    libbf.so.0()(64bit)
    libc.so.6()(64bit)
    libm.so.6()(64bit)
    rtld(GNU_HASH)



Provides
--------
libbf:
    libbf
    libbf(aarch-64)
    libbf.so.0()(64bit)

libbf-devel:
    libbf-devel
    libbf-devel(aarch-64)

libbf-examples:
    libbf-examples
    libbf-examples(aarch-64)



Generated by fedora-review 0.11.0 (05c5b26) last change: 2025-11-29
Command line :/usr/bin/fedora-review -b 2525860
Buildroot used: fedora-rawhide-aarch64
Active plugins: C/C++, Shell-api, Generic
Disabled plugins: PHP, Java, Python, Perl, R, Haskell, SugarActivity, Ocaml, fonts
Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH