https://bugzilla.redhat.com/show_bug.cgi?id=2023061
Bug ID: 2023061 Summary: Review Request: libdeflate - Fast implementation of DEFLATE, zlib, and gzip Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: dank@qemfd.net QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://nick-black.com/rpms/libdeflate.spec SRPM URL: https://nick-black.com/rpms/libdeflate-1.8-1.fc36.src.rpm Description: A fast implementation of DEFLATE, zlib, and gzip Fedora Account System Username: nickblack
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
Nick Black dank@qemfd.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value
--- Comment #1 from Nick Black dank@qemfd.net --- looks like I've got a few rpmlint issues to handle:
[dank@localhost ~]$ rpmlint rpmbuild/SRPMS/libdeflate-1.8-1.fc36.src.rpm ====================================================== rpmlint session starts ====================================================== rpmlint: 2.1.0 configuration: /usr/lib/python3.10/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 1
libdeflate.spec:69: W: macro-in-%changelog %autochangelog ======================= 1 packages and 0 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.1 s ======================= [dank@localhost ~]$ rpmlint rpmbuild/RPMS/ build.log libdeflate-1.8-1.fc36.x86_64.rpm root.log hw_info.log libdeflate-devel-1.8-1.fc36.x86_64.rpm state.log installed_pkgs.log libdeflate-static-1.8-1.fc36.x86_64.rpm libdeflate-1.8-1.fc36.src.rpm libdeflate-utils-1.8-1.fc36.x86_64.rpm [dank@localhost ~]$ rpmlint rpmbuild/RPMS/*rpm ====================================================== rpmlint session starts ====================================================== rpmlint: 2.1.0 configuration: /usr/lib/python3.10/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 5
libdeflate.x86_64: W: unstripped-binary-or-object /usr/lib64/libdeflate.so.0 libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gunzip libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gzip libdeflate-static.x86_64: E: static-library-without-debuginfo /usr/lib64/libdeflate.a libdeflate.x86_64: E: shlib-policy-name-error 0 libdeflate-utils.x86_64: W: position-independent-executable-suggested /usr/bin/libdeflate-gunzip libdeflate-utils.x86_64: W: position-independent-executable-suggested /usr/bin/libdeflate-gzip libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gunzip libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gzip libdeflate-devel.x86_64: W: missing-dependency-on libdeflate*/libdeflate-libs/liblibdeflate* = 1.8 libdeflate.spec:69: W: macro-in-%changelog %autochangelog ======================= 5 packages and 0 specfiles checked; 2 errors, 9 warnings, 2 badness; has taken 0.6 s ======================= [dank@localhost ~]$
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyszek@in.waw.pl
--- Comment #2 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl ---
make USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr DESTDIR=$RPM_BUILD_ROOT %{?_smp_mflags}
This is the same as %make_build USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr
%install rm -rf $RPM_BUILD_ROOT
Uh, please drop ths line.
make install DESTDIR=$RPM_BUILD_ROOT USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr %{?_smp_mflags}
This is the same as %make_install USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #3 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- ... and please figure out the rpmlint issues: libdeflate.x86_64: W: unstripped-binary-or-object /usr/lib64/libdeflate.so.0 libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gunzip libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gzip libdeflate-static.x86_64: E: static-library-without-debuginfo /usr/lib64/libdeflate.a
Those all imply that compliation flags are not conveyed properly and postprocessing of files does not happen.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #4 from Nick Black dank@qemfd.net --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
make USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr DESTDIR=$RPM_BUILD_ROOT %{?_smp_mflags}
This is the same as %make_build USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr
done.
%install rm -rf $RPM_BUILD_ROOT
Uh, please drop ths line.
indeed. it came from https://rpm-packaging-guide.github.io/, which i see now is not fedora canon. i will mail the authors.
make install DESTDIR=$RPM_BUILD_ROOT USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr %{?_smp_mflags}
This is the same as %make_install USE_SHARED_LIB=1 LIBDIR=%{_libdir} PREFIX=/usr
done.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #5 from Nick Black dank@qemfd.net --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
... and please figure out the rpmlint issues: libdeflate.x86_64: W: unstripped-binary-or-object /usr/lib64/libdeflate.so.0 libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gunzip libdeflate-utils.x86_64: W: unstripped-binary-or-object /usr/bin/libdeflate-gzip libdeflate-static.x86_64: E: static-library-without-debuginfo /usr/lib64/libdeflate.a
Those all imply that compliation flags are not conveyed properly and postprocessing of files does not happen.
working on these, thanks. i'll post here when i've got the issues resolved.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #6 from Nick Black dank@qemfd.net --- I've eliminated the "unstripped-binary-or-object" warnings, though I had to put in a manual strip operation (I've only built CMake-based RPMs before; maybe this is typical for projects without a configuration process). Working on the "static-library-without-debuginfo" now. I've updated the SRPM/specfile.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #7 from Nick Black dank@qemfd.net --- fixed up the "position-independent-executable-suggested" warnings; i've got this left:
libdeflate-static.x86_64: E: static-library-without-debuginfo /usr/lib64/libdeflate.a libdeflate.x86_64: E: shlib-policy-name-error 0 libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gunzip libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gzip libdeflate-devel.x86_64: W: missing-dependency-on libdeflate*/libdeflate-libs/liblibdeflate* = 1.8 libdeflate.spec:70: W: macro-in-%changelog %autochangelog
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #8 from Nick Black dank@qemfd.net --- I dropped the static library package after noticing that most libraries on Fedora don't appear to ship their .as. This has eliminated the "static-library-without-debuginfo". Working on the 'shlib-policy-name-error' now. Updated specfile and SRPM.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #9 from Nick Black dank@qemfd.net --- Got the "shlib-policy-name-error" resolved by renaming to libdeflate0. This eliminates all remaining errors from rpmlint (there remain several warnings). The new spec/SRPM can be found at:
https://nick-black.com/rpms/libdeflate0-1.8-1.fc36.src.rpm https://nick-black.com/rpms/libdeflate0.spec
PTAL, and thanks for your time!
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #10 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- Created attachment 1842971 --> https://bugzilla.redhat.com/attachment.cgi?id=1842971&action=edit patch to add missing debuginfo and fix stripping
Got the "shlib-policy-name-error" resolved by renaming to libdeflate0.
That's a strange one. I've never seen this message before. Are you using a non-Fedora version of rpmlint? Fedora does not name packages after so-versions, please rename it back.
%define debug_package %{nil}
This is wrong, we need debuginfo.
CFLAGS="-fpic -pie -g"
This is also wrong. Packages must use standard compilation flags as provided by the macros. [https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags]
%{__strip}
General rule: do not use double-underscore-prefixed commands. Those were popular back in the day, but they are useless and the guidelines finally have been changed to discourage them.
Specific rule: strip is called on all binaries through /usr/lib/rpm/redhat/brp-strip-lto, so we need to figure out why that call doesn't work, and fix the compilation process so that it does.
I dropped the static library package after noticing that most libraries on Fedora don't appear to ship their .a
Yeah, that's probably a good solution. I assumed you want the the static library for something, so I didn't suggest dropping it. But if it isn't needed for something else, it can go.
Removing it does not solve the original issue: the debug data is missing. Let's see what is needed to fix this. ... Oh, it turns out that that this is the typical case of "upstream invents own build system, the result is 90% correct". Please see the attached patch.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #11 from Nick Black dank@qemfd.net --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
That's a strange one. I've never seen this message before. Are you using a non-Fedora version of rpmlint?
i hope not?
Fedora does not name packages after so-versions, please rename it back.
done.
%define debug_package %{nil}
This is wrong, we need debuginfo.
CFLAGS="-fpic -pie -g"
This is also wrong. Packages must use standard compilation flags as provided by the macros. [https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags]
%{__strip}
General rule: do not use double-underscore-prefixed commands. Those were popular back in the day, but they are useless and the guidelines finally have been changed to discourage them.
Specific rule: strip is called on all binaries through /usr/lib/rpm/redhat/brp-strip-lto, so we need to figure out why that call doesn't work, and fix the compilation process so that it does. ... Oh, it turns out that that this is the typical case of "upstream invents own build system, the result is 90% correct". Please see the attached patch.
Applied.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #12 from Nick Black dank@qemfd.net --- You had PREFIX=%{_prefix} in the build line and PREFIX=/usr in the install line. I assume you meant the former in both cases. Testing...
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #13 from Nick Black dank@qemfd.net --- there has been a great explosion of warnings and errors with this change, which seems to have undone the -fPIC -fpie -pie stuff on the binaries:
libdeflate-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/.dwz/libdeflate-1.8-1.fc36.x86_64 libdeflate-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/lib64/libdeflate.so.0-1.8-1.fc36.x86_64.debug libdeflate-utils-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/libdeflate-gunzip-1.8-1.fc36.x86_64.debug libdeflate-utils-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/libdeflate-gzip-1.8-1.fc36.x86_64.debug libdeflate-debuginfo.x86_64: E: statically-linked-binary /usr/lib/debug/.dwz/libdeflate-1.8-1.fc36.x86_64 libdeflate-utils-debuginfo.x86_64: E: statically-linked-binary /usr/lib/debug/usr/bin/libdeflate-gunzip-1.8-1.fc36.x86_64.debug libdeflate-utils-debuginfo.x86_64: E: statically-linked-binary /usr/lib/debug/usr/bin/libdeflate-gzip-1.8-1.fc36.x86_64.debug libdeflate.x86_64: E: shlib-policy-name-error 0 libdeflate-debuginfo.x86_64: E: shared-library-without-dependency-information /usr/lib/debug/usr/lib64/libdeflate.so.0-1.8-1.fc36.x86_64.debug libdeflate-utils.x86_64: W: position-independent-executable-suggested /usr/bin/libdeflate-gunzip libdeflate-utils.x86_64: W: position-independent-executable-suggested /usr/bin/libdeflate-gzip libdeflate-utils-debuginfo.x86_64: W: position-independent-executable-suggested /usr/lib/debug/usr/bin/libdeflate-gunzip-1.8-1.fc36.x86_64.debug libdeflate-utils-debuginfo.x86_64: W: position-independent-executable-suggested /usr/lib/debug/usr/bin/libdeflate-gzip-1.8-1.fc36.x86_64.debug libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gunzip libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gzip libdeflate-devel.x86_64: W: missing-dependency-on libdeflate*/libdeflate-libs/liblibdeflate* = 1.8 libdeflate-debuginfo.x86_64: E: missing-PT_GNU_STACK-section /usr/lib/debug/.dwz/libdeflate-1.8-1.fc36.x86_64 libdeflate.spec:57: W: macro-in-%changelog %autochangelog libdeflate-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz libdeflate-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz libdeflate-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/bb/c2ce132fcddce4617bdf4b5bd2c922f6db65a9 ../../../.build-id/bb/c2ce132fcddce4617bdf4b5bd2c922f6db65a9 libdeflate-utils-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459 ../../../.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459 libdeflate-utils-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459.1 ../../../.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459.1 ====================== 7 packages and 0 specfiles checked; 6 errors, 17 warnings, 6 badness; has taken 1.1 s ======================
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #14 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- Oh, OK. "shlib-policy-name-error" is new in rpmlint-2, and I was doing the build on F33, which has rpmlint-1.x. Googling for the phrase returned mostly opensuse results for some reason, that is why I asked. I tried with rpmlint-2, and I see the error now…
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #15 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl ---
You had PREFIX=%{_prefix} in the build line and PREFIX=/usr in the install line. I assume you meant the former in both cases.
Oh, good catch.
-fPIC -fpie -pie stuff on the binaries:
Yeah, it seems that the pie flags should be added back where they were.
Most of the rpmlint warnings are bogus:
libdeflate-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz libdeflate-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz
All debuginfo packages seem to have that.
libdeflate.x86_64: E: shlib-policy-name-error 0
This is printed for various libs, and it seems to be just an error / stupid default configuration in rpmlint.
libdeflate-devel.x86_64: W: missing-dependency-on libdeflate*/libdeflate-libs/liblibdeflate* = 1.8
The -devel package has Requires:libdeflate(x86-64) = 1.8-3.fc36, so that seems invalid too.
libdeflate.spec:57: W: macro-in-%changelog %autochangelog
Bogus obviously.
libdeflate-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/bb/c2ce132fcddce4617bdf4b5bd2c922f6db65a9 ../../../.build-id/bb/c2ce132fcddce4617bdf4b5bd2c922f6db65a9 libdeflate-utils-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459 ../../../.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459 libdeflate-utils-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459.1 ../../../.build-id/45/c5cea781642cd8998677a25ffdda2d5582e459.1
rpmlint doesn't understand the split into -debugsource packages...
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #16 from Nick Black dank@qemfd.net --- alright, i've added back "-fpic -fpie -pie", and that did eliminate the position-independent warnings once more.
almost everything else is debug package-related, aside from that shlib-policy-name-error business. i'm assuming we are continuing to ignore that, and name it libdeflate?
i've uploaded the specfile and SRPM.
i didn't kill the libdeflate0 variant in case we want that after all, but i'm assuming we're moving forward with:
https://nick-black.com/rpms/libdeflate-1.8-1.fc36.src.rpm https://nick-black.com/rpms/libdeflate.spec
thanks again for your time and patience! it looks like we're just about ready to go, yes?
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |POST Assignee|nobody@fedoraproject.org |zbyszek@in.waw.pl Flags| |fedora-review+
--- Comment #17 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- + package name is OK + license is acceptable (MIT) + license is specified correctly + latest version + builds and installs correctly - %check is missing, see suggestion below + BR/R/P look OK + fedora-review finds no issues
rpmlint: libdeflate.src: W: spelling-error Summary(en_US) gzip -> zip, grip, g zip libdeflate.src: W: spelling-error Summary(en_US) zlib -> lib, glib, z lib libdeflate.src: W: spelling-error %description -l en_US gzip -> zip, grip, g zip libdeflate.src: W: spelling-error %description -l en_US zlib -> lib, glib, z lib libdeflate.src:57: W: macro-in-%changelog %autochangelog libdeflate.x86_64: W: spelling-error Summary(en_US) gzip -> zip, grip, g zip libdeflate.x86_64: W: spelling-error Summary(en_US) zlib -> lib, glib, z lib libdeflate.x86_64: W: spelling-error %description -l en_US gzip -> zip, grip, g zip libdeflate.x86_64: W: spelling-error %description -l en_US zlib -> lib, glib, z lib libdeflate-devel.x86_64: W: no-documentation libdeflate-utils.x86_64: W: no-documentation libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gunzip libdeflate-utils.x86_64: W: no-manual-page-for-binary libdeflate-gzip 7 packages and 0 specfiles checked; 0 errors, 13 warnings. All OK. This is from rpmlint-1.11-19.fc33.noarch. Newer rpmlint has some additional (bogus) complaints.
Package is APPROVED.
I think it's be nice to add something like this: %check %{buildroot}%{_bindir}/libdeflate-gzip -c %{buildroot}%{_bindir}/libdeflate-gzip >gzip.gz %{buildroot}%{_bindir}/libdeflate-gzip -t gzip.gz %{buildroot}%{_bindir}/libdeflate-gunzip -c gzip.gz >gzip.gz.unzipped diff %{buildroot}%{_bindir}/libdeflate-gzip gzip.gz.unzipped to catch trivial build issues.
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #18 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libdeflate
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
--- Comment #19 from Nick Black dank@qemfd.net --- Thanks a bunch for the review and help, Zbigniew!
https://bugzilla.redhat.com/show_bug.cgi?id=2023061
Nick Black dank@qemfd.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |RAWHIDE Status|POST |CLOSED Last Closed| |2021-11-23 20:15:15
package-review@lists.fedoraproject.org