https://bugzilla.redhat.com/show_bug.cgi?id=2266544 --- Comment #5 from Fabio Valentini <decathorpe@xxxxxxxxx> --- Ok, I have some comments. ========== > # disable i686 due to https://bugzilla.redhat.com/show_bug.cgi?id=2266232 > ExcludeArch: %{ix86} You do not need an explanation for disabling i686. For new packages (and existing leaf packages), it can just be dropped (and explicitly removing it is even encouraged): https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval ========== > # prevent library files from being installed > %global __cargo_is_lib() 0 This was an ugly workaround for cargo-rpm-macros < 26 that only worked "by accident". With version 26, the correct form is now this: "%global cargo_install_lib 0" which of course also requires: BuildRequires: cargo-rpm-macros >= 26 This version of rust-packaging / cargo-rpm-macros is now available across all branches of Fedora and on EPEL9. ========== The License tag can be simplified a bit. The SPDX "AND" and "OR" are commutative and associative (but not distributive), so some amount of duplication can be removed ("A OR B" and "B OR A" are equivalent, for example), and some nested parentheses can be dropped. As far as I can tell, the resulting "simplified" license tag would look like this: License: Apache-2.0 AND BSD-3-Clause AND CC0-1.0 AND ISC AND MIT AND MPL-2.0 AND (0BSD OR MIT OR Apache-2.0) AND (Apache-2.0 OR BSL-1.0) AND (Apache-2.0 OR ISC OR MIT) AND (Apache-2.0 OR MIT) AND (Apache-2.0 OR MIT OR MPL-2.0) AND (Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT) AND (BSD-2-Clause OR Apache-2.0 OR MIT) AND (CC0-1.0 OR MIT-0 OR Apache-2.0) AND (MIT OR Apache-2.0 OR Zlib) AND (Unlicense OR MIT) Which is about 140 characters shorter. ========== # the following tests don't work in the rpm build %cargo_test -- --bin trunk -- --skip config::models_test::err_bad_trunk_toml_watch_ignore --skip config::models_test::err_bad_trunk_toml_watch_path --skip tools::tests::download_and_install_binaries I would appreaciate something more detailed than just "these tests don't work": - Are they missing data files that are not included in tarballs? - Do they require internet access? ========== Some licenses for bundled crates don't match with the licenses they are assigned in Fedora packages according to the rules we got from Red Hat Legal. For example, the regex-automata crate bundles some data from Unicode, which is licensed under the terms of the Unicode-DFS-2016 license - but this is not reflected in crate metadata. Another example is the bundled "ring" crate. It does not specify an SPDX expression for its license at all, so it does not show up in the output of the %cargo_license / %cargo_license_summary macros. According to at two independent analyses (one by me and one by the authors of the "cargo-deny" tool, I think), the license expression for the "ring" crate would be best expressed as "ISC AND MIT AND OpenSSL". So the OpenSSL license is missing from the package's license tag, too. ========== It might or might not be desirable for you to ship a nonstandard TLS implementation here, especially if you also want to ship this package in RHEL. I'm not sure if "ring" / "rustls" are an acceptable crypto stack there. Since rustls / ring are not direct dependencies, but rather only pulled in because the "rustls" backend of several dependencies is chosen (axum, reqwest, tungstenite), it might be possible to switch to the "native-tls" backend (i.e. OpenSSL) with a patch. ========== On that note - do you have a list of dependencies that would be missing if you were to try to build this package without vendored dependencies? Looking at the list of bundled crates, I don't see that many that are not packaged yet for Fedora (though some might be at the wrong version). Packaging this initially with vendored dependencies would be fine to get things going, but it would be great if things could be unbundled on the short-to-medium-term. ========== Other than these issues, the package looks pretty good. It's mostly the spec file as generated by "rust2rpm --vendor", just with the necessary modifications t make it complete and valid. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2266544 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202266544%23c5 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
