Spec URL: https://music.fedorapeople.org/rust-libz-ng-sys.spec SRPM URL: https://music.fedorapeople.org/rust-libz-ng-sys-1.1.15-1.fc39.src.rpm Description: Low-level bindings to zlib-ng (libz-ng), a high-performance zlib library. Fedora Account System Username: music This package would allow us to stop hiding the zlib-ng and libz-ng-sys features in the rust-flate2 package. (With https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2024-e3799bc571, this will be possible all the way back to EPEL9.)
Copr build: https://copr.fedorainfracloud.org/coprs/build/7417530 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279208-rust-libz-ng-sys/fedora-rawhide-x86_64/07417530-rust-libz-ng-sys/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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.
Created attachment 2031520 [details] Configuration file for rust2rpm The only manual change to the spec file produced by rust2rpm was removing %license %{crate_instdir}/src/zlib-ng/LICENSE.md since the bundled zlib-ng in src/zlib-ng/ is removed in %prep.
Created attachment 2031521 [details] Configuration file for rust2rpm Removed some inapplicable comment lines.
Updated using the same URLs. [fedora-review-service-build]
Created attachment 2031523 [details] The .spec file difference from Copr build 7417530 to 7418707
Copr build: https://copr.fedorainfracloud.org/coprs/build/7418707 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279208-rust-libz-ng-sys/fedora-rawhide-x86_64/07418707-rust-libz-ng-sys/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ 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.
It occurred to me that #[allow(dead_code)] in the downstream build.rs is not needed (although it is harmless). I’ll plan to remove that line on import.
It's a bit sad that there are literally *zero* tests in this crate. Usually those are helpful in "-sys" crates because if there's at lease *one* test, the resulting binary is linked against the C library. If there's zero tests, I don't think linking is even attempted once during the build. So I can't really verify that this works as expected ... but the patch (i.e. the replaced build script) looks sane to me. I guess we will see if this works when building zip v1 - the tests built and run as part of its build should get linked to z-ng if this crate does its job. === Package was generated with rust2rpm, simplifying the review. ✅ package builds and installs without errors on rawhide 😦 test suite is run and all unit tests pass (there are no tests) ✅ latest version of the crate is packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ license files are included with %license in %files ✅ package complies with Rust Packaging Guidelines Package APPROVED. === Recommended post-import rust-sig tasks: - set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate - add @rust-sig with "commit" access as package co-maintainer (should happen automatically) - set bugzilla assignee overrides to @rust-sig (optional) - track package in koschei for all built branches (should happen automatically once rust-sig is co-maintainer)
(In reply to Fabio Valentini from comment #8) > It's a bit sad that there are literally *zero* tests in this crate. > Usually those are helpful in "-sys" crates because if there's at lease *one* > test, the resulting binary is linked against the C library. > > If there's zero tests, I don't think linking is even attempted once during > the build. So I can't really verify that this works as expected ... but the > patch (i.e. the replaced build script) looks sane to me. > > I guess we will see if this works when building zip v1 - the tests built and > run as part of its build should get linked to z-ng if this crate does its > job. Hmm, this is a good point. I tested this with https://src.fedoraproject.org/rpms/rust-flate2/pull-request/2, but I suppose without enabling extra features it wouldn’t really be usefully tested there. As an experiment, I just tried hacking up the flate2 rust2rpm.toml to explicitly enable the zlib-ng and libz-ng-sys features, resulting in e.g. “%cargo_test -f libz-ng-sys,zlib-ng”. That worked, but I’m not sure how to verify that this really caused the tests to link the zlib-ng C library. As you said, I suppose if there are any problems we’ll just have to go back and fix them. Thank you for the review.
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-libz-ng-sys
https://release-monitoring.org/project/372388/
FEDORA-2024-347e6de6e9 (rust-libz-ng-sys-1.1.15-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-347e6de6e9
Aha, I had started to test this locally with (at the time) rust-zip-1.1.2, but I hadn’t finished the zstd/zstd-safe/zstd-sys PR’s yet. I just tried again and ran into issues with the tests wanting additional data files, like error: couldn't read src/read/../../tests/data/invalid_offset.zip: No such file or directory (os error 2) --> src/read/stream.rs:248:46 | 248 | ZipStreamReader::new(io::Cursor::new(include_bytes!( | ______________________________________________^ 249 | | "../../tests/data/invalid_offset.zip" 250 | | ))) | |_________^ | = note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info) so it looks like a little more work will be required on rust-zip before I can really double-check this.
FEDORA-2024-347e6de6e9 (rust-libz-ng-sys-1.1.15-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.