https://bugzilla.redhat.com/show_bug.cgi?id=2392738
--- Comment #5 from Ben Beasley <[email protected]> --- 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. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2392738 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202392738%23c5 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
