Spec URL: https://music.fedorapeople.org/rust-tokio-tar.spec SRPM URL: https://music.fedorapeople.org/rust-tokio-tar-0.3.1-1.fc39.src.rpm Description: A Rust implementation of an async TAR file reader and writer. This library does not currently handle compression, but it is abstract over all I/O readers and writers. Additionally, great lengths are taken to ensure that the entire contents are never required to be entirely resident in memory all at once. Fedora Account System Username: music This is a dependency for a future package of https://pypi.org/project/uv. The spec file is exactly as produced by rust2rpm, with a rust2rpm.toml configuration file to be attached. This package built in koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=117955027 There is an open issue to consider replacing this dependency in uv, https://github.com/astral-sh/uv/issues/3423, but for now it is still needed. It’s generally reasonable to skip tests that need large ancillary test data not shipped in the crate, but in this case the additional data is quite small, and the necessary boilerplate is limited, so I think including the extra data to run the tests can be justified.
Created attachment 2034161 [details] Configuration file for rust2rpm
Oops… this still has some arch-specific issues. Setting the status to MODIFIED until I figure out how to proceed.
Copr build: https://copr.fedorainfracloud.org/coprs/build/7467139 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2281965-rust-tokio-tar/fedora-rawhide-x86_64/07467139-rust-tokio-tar/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.
I took a quick look at the test failures. On i686, it looks like it's failing to allocate 1.6 GB in the doctest that adds the current directory (".") into a tar archive. This likely includes the "target" directory, so any compiled artifacts generated during %cargo_build and %cargo_test. This is likely working *most of the time*, but it's maybe not a good idea in general. see https://github.com/vorot93/tokio-tar/blob/master/src/builder.rs#L400 On s390x, I can't really tell what's going wrong. The error message isn't that helpful because the test just "assert!(entries.next().await.is_none()" instead of using "assert_eq!(entries.next().await, None)", which would print the value that is "not none" in the error message instead of just saying "I wanted a true, but I got a false! FAIL"). Is the failure persistent? It might just have been a fluke. see https://github.com/vorot93/tokio-tar/blob/master/tests/all.rs#L1103
(In reply to Fabio Valentini from comment #4) > I took a quick look at the test failures. > > On i686, it looks like it's failing to allocate 1.6 GB in the doctest that > adds the current directory (".") into a tar archive. > This likely includes the "target" directory, so any compiled artifacts > generated during %cargo_build and %cargo_test. > This is likely working *most of the time*, but it's maybe not a good idea in > general. > > see https://github.com/vorot93/tokio-tar/blob/master/src/builder.rs#L400 This is a logical explanation, although something still isn’t *entirely* adding up—when I tested this in an fedora-rawhide-i386 mock chroot, the target directory was only 303M, so this allocation still seems bizarrely large. Would be nice if upstream had a bug tracker. Still, an arch-dependent or even unconditional test skip is probably the way to go here regardless of what is going on. The only reason to build i686 is to avoid having to propagate ExcludeArch, not to actually *use* it. > > On s390x, I can't really tell what's going wrong. The error message isn't > that helpful because the test just "assert!(entries.next().await.is_none()" > instead of using "assert_eq!(entries.next().await, None)", which would print > the value that is "not none" in the error message instead of just saying "I > wanted a true, but I got a false! FAIL"). Is the failure persistent? It > might just have been a fluke. > > see https://github.com/vorot93/tokio-tar/blob/master/tests/all.rs#L1103 Good question. I just kicked off five scratch builds: https://koji.fedoraproject.org/koji/taskinfo?taskID=118005832 https://koji.fedoraproject.org/koji/taskinfo?taskID=118005833 https://koji.fedoraproject.org/koji/taskinfo?taskID=118005834 https://koji.fedoraproject.org/koji/taskinfo?taskID=118005842 https://koji.fedoraproject.org/koji/taskinfo?taskID=118005845 The test didn’t fail on s390x again, but it did fail twice on aarch64 with similar output: failures: ---- insert_local_file_different_name stdout ---- thread 'insert_local_file_different_name' panicked at tests/all.rs:1103:5: assertion failed: entries.next().await.is_none() note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: insert_local_file_different_name test result: FAILED. 50 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.05s error: test failed, to rerun pass `--test all` Running `/builddir/build/BUILD/tokio-tar-0.3.1/target/rpm/deps/entry-5ef2d6d85a16be6a` …and one of those times, two other tests failed as well: failures: ---- insert_local_file_different_name stdout ---- thread 'insert_local_file_different_name' panicked at tests/all.rs:1103:5: assertion failed: entries.next().await.is_none() ---- large_filename stdout ---- thread 'large_filename' panicked at tests/all.rs:195:5: assertion `left == right` failed left: 0 right: 4 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ---- writing_files stdout ---- thread 'writing_files' panicked at tests/all.rs:160:5: assertion `left == right` failed left: 0 right: 4 failures: insert_local_file_different_name large_filename writing_files test result: FAILED. 48 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.05s As I just wrote in https://github.com/astral-sh/uv/issues/3423#issuecomment-2123668288, It looks like the `insert_local_file_different_name` is flaky, roughly less than 20% of the time, on `s390x`, but it also occurs (more frequently) on `aarch64`. I have also observed flaky failures in other tests on `aarch64` – at least `large_filename` and `writing_files`, and my notes indicate that I saw `path_separators` panic in a previous attempt. I think a reasonable conclusion here is that `tokio-tar` has significant race conditions, at least on some architectures, and should have engaged in much more fearful concurrency.
Now the question is what to do about the problem. I’ll see if the https://crates.io/crates/krata-tokio-tar crate mentioned in the uv issue is able to pass its own tests on all architectures or not. If so, maybe that would be a point in favor of upstream adopting it as at least a short-term band-aid.
(In reply to Ben Beasley from comment #6) > Now the question is what to do about the problem. I’ll see if the > https://crates.io/crates/krata-tokio-tar crate mentioned in the uv issue is > able to pass its own tests on all architectures or not. If so, maybe that > would be a point in favor of upstream adopting it as at least a short-term > band-aid. In ten scratch builds, I observed flaky test failures on x86_64, aarch64, ppc64le, and s390x. So it is also problematic.
I’m able to get this package to fail on every architecture if I do enough scratch builds, too. Furthermore, although it took 25 iterations, I was finally able to reproduce this with plain "cargo test --release" in a git checkout. I’m not sure yet if --release is required.
(In reply to Ben Beasley from comment #8) > I’m able to get this package to fail on every architecture if I do enough > scratch builds, too. Furthermore, although it took 25 iterations, I was > finally able to reproduce this with plain "cargo test --release" in a git > checkout. I’m not sure yet if --release is required. I reproduced it in 82 iterations without --release.
I am not happy with this, but here is a submission that take sufficient measures to ignore the problems with this crate in order to almost always get a successful build. Spec URL: https://music.fedorapeople.org/20240522/rust-tokio-tar.spec SRPM URL: https://music.fedorapeople.org/20240522/rust-tokio-tar-0.3.1-1.fc39.src.rpm Unless someone figures out how to straighten out the concurrency issues properly, or uv upstream figures out an alternative to this crate (and I hope they will), I don’t see what can be done other than paper over the problems and proceed with packaging this as-is.
Created attachment 2034602 [details] Updated configuration for rust2rpm
Created attachment 2034603 [details] The .spec file difference from Copr build 7467139 to 7476256
Copr build: https://copr.fedorainfracloud.org/coprs/build/7476256 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2281965-rust-tokio-tar/fedora-rawhide-x86_64/07476256-rust-tokio-tar/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.
It's a bit unfortunate that the code here looks unreliable (looking at differences between the "tar" crate and "tokio-tar", it seems that wrong usage of Atomics might be to blame). But the package itself looks correct, and every change made on top of the spec file generated by rust2rpm is clearly documented. === 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 (tests appear to be unreliable, but the most-unreliable tests are skipped, so the package at least builds *most* of the time. probably the best one can do without skipping tests entirely, bar fixing the actual underlying issue that causes the unreliability) - 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)
Thank you for the review!
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-tokio-tar
https://release-monitoring.org/project/372545/
FEDORA-2024-83ba0bb084 (rust-tokio-tar-0.3.1-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-83ba0bb084
FEDORA-2024-83ba0bb084 (rust-tokio-tar-0.3.1-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-eb65a23f70 (rust-tokio-tar-0.3.1-1.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-eb65a23f70
FEDORA-2024-1bae0600cb (rust-tokio-tar-0.3.1-1.fc39) has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2024-1bae0600cb
FEDORA-2024-eb65a23f70 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-eb65a23f70 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-eb65a23f70 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-1bae0600cb 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-1bae0600cb \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-1bae0600cb See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-eb65a23f70 (rust-tokio-tar-0.3.1-1.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-1bae0600cb (rust-tokio-tar-0.3.1-1.fc39) has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.