Bug 2392738 - Review Request: aff4-cpp-lite - Advanced Forensic Format version 4 library
Summary: Review Request: aff4-cpp-lite - Advanced Forensic Format version 4 library
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/aff4/aff4-cpp-lite
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-09-02 22:13 UTC by bcling
Modified: 2025-11-21 16:40 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description bcling 2025-09-02 22:13:38 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/bcling/aff4-cpp-lite/fedora-42-x86_64/09516534-aff4-cpp-lite/

SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/4578/136694578/aff4-cpp-lite-2.1.1pre-1.fc42.x86_64.rpm

Description: A lightweight C++/C AFF4 reader library, supporting the AFF4 format in xmount.

The Advanced Forensics Format 4 (AFF4) is an extensible open format for the
storage of disk images and related forensic metadata. AFF was originally developed
by Simson Garfinkel and Basis Technology. AFF4 builds upon many of the concepts
developed in AFF and was developed by Michael Cohen, Simson Garfinkel, and
Bradley Schatz.

Fedora Account System Username: bcling

Comment 1 Fedora Review Service 2025-09-02 22:14:09 UTC
Cannot find any valid SRPM URL for this ticket. Common causes are:

- You didn't specify `SRPM URL: ...` in the ticket description
  or any of your comments
- The URL schema isn't HTTP or HTTPS
- The SRPM package linked in your URL doesn't match the package name specified
  in the ticket summary


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 3 Fedora Review Service 2025-09-02 22:52:49 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9516899
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2392738-aff4-cpp-lite/fedora-rawhide-x86_64/09516899-aff4-cpp-lite/fedora-review/review.txt

Found issues:

- aff4-cpp-lite : /usr/include/aff4/AFF4Containers.h aff4-cpp-lite : /usr/include/aff4/AFF4Defaults.h aff4-cpp-lite : /usr/include/aff4/AFF4Lexicon.h aff4-cpp-lite : /usr/include/aff4/IAFF4Container.h aff4-cpp-lite : /usr/include/aff4/IAFF4Image.h aff4-cpp-lite : /usr/include/aff4/IAFF4Map.h aff4-cpp-lite : /usr/include/aff4/IAFF4Resolver.h aff4-cpp-lite : /usr/include/aff4/IAFF4Resource.h aff4-cpp-lite : /usr/include/aff4/IAFF4Stream.h aff4-cpp-lite : /usr/include/aff4/RDFValue.h aff4-cpp-lite : /usr/include/aff4/aff4-c.h aff4-cpp-lite : /usr/include/aff4/aff4.h 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- License file COPYING is not marked as %license
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text
- Package has .a files: aff4-cpp-lite. Illegal package name: aff4-cpp-lite. Does not provide -static: aff4-cpp-lite.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 4 Ben Beasley 2025-09-16 10:19:24 UTC
Trying to fix the URLs:

Spec URL: https://download.copr.fedorainfracloud.org/results/bcling/aff4-cpp-lite/fedora-42-x86_64/09516534-aff4-cpp-lite/aff4-cpp-lite.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/bcling/aff4-cpp-lite/fedora-42-x86_64/09516534-aff4-cpp-lite/aff4-cpp-lite-2.1.1pre-1.fc42.src.rpm

Note that COPR artifacts get cleaned up after a while, often before anyone gets around to reviewing them. Something like https://fedoraproject.org/wiki/Infrastructure/fedorapeople.org would be a more durable place to host submissions for review.

Comment 5 Ben Beasley 2025-09-16 11:01:50 UTC
I don’t have time to go through this in full detail right now, but you really need to spend some time with the packaging guidelines in order to accomplish a successful package review. Here are some of the things that stood out in a quick skim of the spec file.

The headers, unversioned shared library link, and .pc file need to be in a -devel subpackage, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages.

The shared libraries should normally be in a -libs subpackage, separate from the CLI tools, which can be in the base package. This helps with multilib issues. The -devel package should have a fully-versioned dependency on the -libs subpackage, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package, and so should the base package if the CLI tools link the shared libraries.

Installation paths should be listed with the appropriate macros, https://docs.fedoraproject.org/en-US/packaging-guidelines/RPMMacros/#macros_installation.

Libraries need to be installed into %{_libdir}, which is /usr/lib64 on most platforms, not into /usr/lib, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros. Patch the build system if necessary, or configure it with the proper options. Most likely, this will be fixed by the following:

You should invoke configure and make with the macros %configure, %make_build, and %make_install. This ensures that the proper options are passed. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make.

You need to provide useful debuginfo, https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/, not disable it with %define debug_package %{nil}. You don’t have debuginfo because you aren’t respecting the system compiler flags, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags; see the recommendations about macros like %configure above.

This is not a useful Summary: “aff4 libraries from GitHub.” The one from upstream is fine: “A lightweight C++/C AFF4 reader library”

You need to remove the bundled dependencies in win32/ in %prep to prove they are not used. You also need to account for src/utils/PortableEndian.h as a bundled dependency according to https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling, submit the public-domain dedication for validation similar to https://gitlab.com/fedora/legal/fedora-license-data/-/issues/667, and include a LicenseRef-Fedora-Public-Domain term in the License.

You need to remove the Group: and Buildroot: sections from the spec file, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections. You should also remove %clean, and you do not need to explicitly remove the contents of the buildroot.

Don’t include empty %post, %preun, and %postun scriptlet sections. Remove those entirely.

Remove the %defattr(-,root,root) directive, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_permissions.

Make sure you own all directories you create. In addition to the incorrect use of %{_exec_prefix}/include instead of %{_includedir}, %{_exec_prefix}/include/aff4/* does not own %{_includedir}/aff4. Either list the directory (with implicit recursion), ideally with a trailing slash so it can only match a directory, like %{_includedir}/aff4/, or list the directory and its contents separately, %dir %{_includedir}/aff4/ and %{_includedir}/aff4/*.h.

Don’t glob over shared directories like %{_bindir}/* or %{_exec_prefix}/lib/* (the latter of which has other problems already noted). Use explicit lists, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists.

Don’t glob over the SONAME version, either, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files. This makes it too easy to miss an SONAME version bump that would require dependent packages to be rebuilt.

Listing all of your Requires and BuildRequires in one line each is technically allowed, but listing them one per line is much easier to read and edit.

Dependencies that are found via pkg-config (check configure.ac to find out) should be listed as e.g. pkgconfig(zlib) instead of zlib-devel, https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/.

You do not need to number the sole Source. Instead of Source0:, you can just write Source:.

I see that the upstream package has tests, and you even have a BuildRequires on the test dependency cppunit-devel. You should try to actually build and run the tests, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites.

Wherever possible (and here it is possible), your Source field should be a URL, https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/. Something like https://github.com/aff4/aff4-cpp-lite/archive/v2.1.1-pre/aff4-cpp-lite-2.1.1-pre.tar.gz should work.

The Version should be 2.2.1~pre, https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_handling_non_sorting_versions_with_tilde_dot_and_caret.


Note You need to log in before you can comment on or make changes to this bug.