Spec URL: https://salimma.fedorapeople.org/specs/rust-coreutils.spec SRPM URL: https://salimma.fedorapeople.org/specs/rust-coreutils-0.0.23-1.fc38.src.rpm Description: coreutils ~ GNU coreutils (updated); implemented as universal (cross- platform) utils, written in Rust. Fedora Account System Username: salimma
Copr build: https://copr.fedorainfracloud.org/coprs/build/6965713 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260598-rust-coreutils/fedora-rawhide-x86_64/06965713-rust-coreutils/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
Skipped renaming binary [fedora-review-service-build]
Created attachment 2011032 [details] The .spec file difference from Copr build 6965713 to 6967305
Copr build: https://copr.fedorainfracloud.org/coprs/build/6967305 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260598-rust-coreutils/fedora-rawhide-x86_64/06967305-rust-coreutils/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.
First-pass review: 1. It looks like the "default" feature set builds only a "minimal" / "core" of tools that are supported everywhere. Looking at the list of things that are enabled by different features, it would be great if we could build with "unix" and "selinux" features enabled. The "unix" feature would enable a few more tools and more features for some tools that are already enabled as part of the "default" set. This would pull in a few more dependencies that are currently optional and disabled. The "selinux" feature would enable SELinux support for tools that are already part of the "default" set like uu_cp, uu_ls (and would not introduce additional dependencies - as far as I can tell by looking). 2. You might not want to build "uudoc" / enable the "uudoc" feature in the package. The feature only gets enabled by rust2rpm because the "uudoc" binary target requires it, but looking at what the "uudoc" binary does, I don't think it makes sense to actually ship that binary with the package (it seems to be a tool to generate some documentation, but that's more for upstream purposes, and not for users in Fedora). 3. It would be great if the man page and shell completions could be included in the package. 4. The list of skipped tests is pretty long, and there's no explanation for what's wrong with them other than "they mostly panic". It would be great if you could do at least *some* amount of investigation into why specific tests are failing - and document whether the failures have something to do with the build environment, or whether they are *actually* indicative of a real failure. === It's fine if you want to enable more tools / functionality *later* and get the package into Fedora as-is. However, it would be great if at least points 2 and 4 above could be addressed. As for point 1, I can help with doing package reviews for the missing uu_* tools that are currently disabled, but that is not a blocker for the review. For point 3, I can submit a PR once the package is imported, or you can add the manpages / shell completions to the package yourself later (building shell completions from the built binary works almost the same way in the "ruff" or "maturin" packages, for example). This would be nice-to-have, but it's not a blocker for the review either.
Will try and address 2, 3, 4 on Monday after FOSDEM, thanks!
Need uu_arch to generate manpages
Spec URL: https://salimma.fedorapeople.org/specs/rust-coreutils.spec SRPM URL: https://salimma.fedorapeople.org/specs/rust-coreutils-0.0.23-1.fc39.src.rpm 2 - uudoc disabled 3 - the `cargo run` command seems to require a lot of the optional uu crates to even run. I have the manpage generation code in the spec but gated behind `%bcond_with man` for now 4 - done. Patched the seq errors. There are some flaky tests that I might need to add to rust2rpm.toml later, but the spec as is would pass builds most of the time.
Created attachment 2019181 [details] The .spec file difference from Copr build 6967305 to 7066176
Copr build: https://copr.fedorainfracloud.org/coprs/build/7066176 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260598-rust-coreutils/fedora-rawhide-x86_64/07066176-rust-coreutils/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 tried running fedora-review here, but the package failed to build due to failing tests: failures: ---- test_mktemp::test_directory_permissions stdout ---- run: /builddir/build/BUILD/coreutils-0.0.23/target/rpm/coreutils mktemp -d XXX thread 'test_mktemp::test_directory_permissions' panicked at tests/common/util.rs:1528:37: called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" } ---- test_mktemp::test_missing_short_tmpdir_flag stdout ---- run: /builddir/build/BUILD/coreutils-0.0.23/target/rpm/coreutils mktemp -p thread 'test_mktemp::test_missing_short_tmpdir_flag' panicked at tests/common/util.rs:1528:37: called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" } ---- test_mktemp::test_missing_xs_tmpdir_template stdout ---- run: /builddir/build/BUILD/coreutils-0.0.23/target/rpm/coreutils mktemp --tmpdir tempX thread 'test_mktemp::test_missing_xs_tmpdir_template' panicked at tests/common/util.rs:1528:37: called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" } ---- test_tail::test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same_size stdout ---- write(default): /tmp/.tmpj9taQk/data run: /builddir/build/BUILD/coreutils-0.0.23/target/rpm/coreutils tail --follow=descriptor --max-unchanged-stats=1 --sleep-interval=0.1 data /tmp/.tmpj9taQk/data write(default): /tmp/.tmpj9taQk/data thread 'test_tail::test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same_size' panicked at tests/by-util/test_tail.rs:4463:10: Expected stderr to be empty, but it's: tail: /tmp/.tmpj9taQk/data: file truncated failures: test_mktemp::test_directory_permissions test_mktemp::test_missing_short_tmpdir_flag test_mktemp::test_missing_xs_tmpdir_template test_tail::test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same_size test result: FAILED. 2514 passed; 4 failed; 28 ignored; 0 measured; 12 filtered out; finished in 21.02s
Yeah, those are weirdly flaky. I'll disable them and retest
Can we get this moving again? It would be great to have it in Fedora before the next mass-update for uu_* and nu-* rolls around the corner, just to avoid churn here.