https://bugzilla.redhat.com/show_bug.cgi?id=2214364 --- Comment #3 from Fabio Valentini <decathorpe@xxxxxxxxx> --- (In reply to blinxen from comment #2) > Taking this review > > General comments: > > - Package was generated with rust2rpm and changes were made > - License field was specified by the packager because upstream did not > specify it in the crate metadata. The main branch has this fixed but it now > includes both ISC and BSD-3-Clause. My understanding is that BSD-3-Clause is > only required for tests. The code itself is licensed under ISC. Meh, I guess I'll file an issue with upstream to clarify the license. Including BSD-3-Clause in the crate metadata if it only applies to test data that's not linked into binaries that seems off to me. > - The tests directory along with the third-party directory are excluded from > the binary RPM. Not specifying the BSD-3-Clause license should be OK here. > > Questions: > > - The crate depends on `ring` which is architecture dependent. Why is the > package "noarch"? Does the "supported_arches" macro have some kind of magic > powers? No, the idea is that the package is available on all architectures to prevent broken dependencies, but it will not compile on unsupported architectures. This is why %cargo_build and %cargo_test are wrapped in "%if supported_arches" conditions. > - Both build and check sections are dependent on the arch but the install > section is not. Why? Does "cargo install" not also build the crate if it was > not built before? The "%cargo_install" macro does not run "cargo install" for library crates, but only "cargo package" to copy library sources. So no, it does not compile. Otherwise the scratch build would also have failed. > - You drop all integration tests because of missing files in the published > crate. Some tests have their files published with the crate, so technically > they could be run. Did you miss this or did I see wrong? I didn't miss it. But the way the tests are written, they attempt to include files at compile time (i.e. `include_bytes!()` macro). This breaks compiling the tests/integration.rs file even if I tried to skip tests with missing data with "%cargo_test -- -- --skip foo" ... I don't think it's worth my time to manually go through this file for every update to patch out tests that don't compile. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2214364 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202214364%23c3 _______________________________________________ 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
