Spec URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled.spec SRPM URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled-0.34.7-1.fc41.src.rpm Description: Lightweight high-performance pure-rust transactional embedded database. Fedora Account System Username: dcavalca
$ cat rust2rpm.toml [package] cargo-toml-patch-comments = [ "bump rand_distr to 0.4: https://github.com/spacejam/sled/pull/1500", "bump zerocopy to 0.7: https://github.com/spacejam/sled/pull/1501", "bump zstd to 0.13", ] extra-patches = [ {"number" = 2, "file" = "sled-unbreak-tests.patch", "comments" = ["Allow tests to run in the build"]}, ] [scripts] check.pre = [ # Drop test that does not compile properly "rm -r tests/{tree/,test_tree.rs}", ] [tests] skip = [ "db::Db::import", "db::Db::export", ] comments = [ "Disable brittle tests" ]
Previous review: https://bugzilla.redhat.com/show_bug.cgi?id=2259675
Copr build: https://copr.fedorainfracloud.org/coprs/build/7525436 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2284641-rust-sled/fedora-rawhide-x86_64/07525436-rust-sled/fedora-review/review.txt Please take a look if any issues were found. --- 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.
Some comments: - Why add a patch to "unbreak" tests instead of just passing "-f testing" to %cargo_test? - Is bumping from zstd 0.9 to 0.13 safe? It's a feature that is not compiled by default. There's no way to see if bumping the dep causes issues without enabling the "compression" feature during the build. - "tsan_suppressions.txt" doesn't look like %docs. Please exclude it from the package. - You're rm-ing some tests in %check without explanation. Please either don't drop them or explain why they are dropped. - It might also be a good idea to exclude the "art" folder from the -devel package. It only contains unused images.
(In reply to Fabio Valentini from comment #4) > Some comments: > > - Why add a patch to "unbreak" tests instead of just passing "-f testing" to > %cargo_test? Because I didn't know this was an option :) > - Is bumping from zstd 0.9 to 0.13 safe? It's a feature that is not compiled > by default. > There's no way to see if bumping the dep causes issues without enabling > the "compression" feature during the build. Upstream had previously bumped it to 0.11.2 in git, afaict it should be fine. > - "tsan_suppressions.txt" doesn't look like %docs. Please exclude it from > the package. Will do > - You're rm-ing some tests in %check without explanation. Please either > don't drop them or explain why they are dropped. There's a comment in the rust2rpm.toml, these fail to build in a way that breaks the build early on and prevents excluding them normally. I'll add a comment in the spec as well. > - It might also be a good idea to exclude the "art" folder from the -devel > package. It only contains unused images. Will do
(In reply to Davide Cavalca from comment #5) > (In reply to Fabio Valentini from comment #4) > > Some comments: > > > > - Why add a patch to "unbreak" tests instead of just passing "-f testing" to > > %cargo_test? > > Because I didn't know this was an option :) Um, this doesn't seem to work: + /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 'RUSTFLAGS=-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Cstrip=none -Cforce-frame-pointers=yes -Clink-arg=-specs=/usr/lib/rpm/redhat/redhat-package-notes --cap-lints=warn' /usr/bin/cargo test -j8 -Z avoid-dev-deps --profile rpm --no-fail-fast --features testing -- --skip db::Db::import --skip db::Db::export error: no matching package named `backtrace` found location searched: registry `crates-io` required by package `sled v0.34.7 (/builddir/build/BUILD/rust-sled-0.34.7-build/sled-0.34.7)` As a reminder, you're using offline mode (--offline) which can sometimes cause surprising resolution failures, if this error is too confusing you may wish to retry without the offline flag. error: Bad exit status from /var/tmp/rpm-tmp.Xk0cre (%check)
Spec URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled.spec SRPM URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled-0.34.7-1.fc41.src.rpm Changelog: - add comment for dropped tests - exclude unneeded files
> Um, this doesn't seem to work You need to pass the "-f testing" argument to all %cargo macros, in particular, the %cargo_generate_buildrequires macro. The "testing" feature pulls in some optional dependencies that are not enabled by default. You can automate this with ``` [features] enable = ["testing"] ```
Spec URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled.spec SRPM URL: https://dcavalca.fedorapeople.org/review/rust-sled/rust-sled-0.34.7-1.fc41.src.rpm Changelog: - enable testing feature instead of patching - reenable tests - drop unused zstd dependency and compression feature
$ cat rust2rpm.toml [package] cargo-toml-patch-comments = [ "bump rand_distr to 0.4: https://github.com/spacejam/sled/pull/1500", "bump zerocopy to 0.7: https://github.com/spacejam/sled/pull/1501", "drop unused zstd dependency and compression feature", ] [features] enable = ["testing"] [scripts] check.pre = [ # Drop test that does not compile properly "rm -r tests/{tree/,test_tree.rs}", ]
I changed my mind about zstd, looks like upstream dropped it entirely, and the cached crate doesn't use the feature, so I'll drop it to avoid trouble.
Thank you for the update, looks good to me! === Package was generated with rust2rpm, simplifying the review. ✅ package contains only permissible content ✅ package builds and installs without errors on rawhide ✅ test suite is run and all unit tests pass (some tests disabled because they fail to compile) ✅ latest version of the crate is packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ licenses of statically linked dependencies are correctly taken into account ✅ license file is 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)
Also ran a scratch build that passed on all architectures: https://koji.fedoraproject.org/koji/taskinfo?taskID=120853390
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-sled
FEDORA-2024-6656c0bb3c (rust-sled-0.34.7-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-6656c0bb3c
FEDORA-2024-1cdb332410 (rust-sled-0.34.7-1.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-1cdb332410
FEDORA-2024-6656c0bb3c (rust-sled-0.34.7-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-d19ba09cdf (rust-sled-0.34.7-1.fc39) has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2024-d19ba09cdf
FEDORA-2024-d19ba09cdf has been pushed to the Fedora 39 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-d19ba09cdf \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-d19ba09cdf See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-1cdb332410 has been pushed to the Fedora 40 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-1cdb332410 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-1cdb332410 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-d19ba09cdf (rust-sled-0.34.7-1.fc39) has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-1cdb332410 (rust-sled-0.34.7-1.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.
*** Bug 2259675 has been marked as a duplicate of this bug. ***