https://bugzilla.redhat.com/show_bug.cgi?id=2266544 --- Comment #9 from Fabio Valentini <decathorpe@xxxxxxxxx> --- (In reply to Jens Reimann from comment #8) > Ok, I think I got all of your remarks covered, I think. There's a new > version of the spec file and source: > > Spec: https://dentrassi.de/download/rust-trunk/rust-trunk.spec > SRPM: > https://dentrassi.de/download/rust-trunk/rust-trunk-0.19.0~rc.2-1.fc41.src. > rpm > > The reason for the version change is that trunk 0.19.0 is soon to be > released and I was able to add support for `native-tls` (and `openssl` on > the server side). Which means that by setting a cargo feature one can switch > from rustls to native-tls/openssl. That also removes the `ring` dependency. Great! It looks like this change was only partially applied, though. You need to pass feature flags to *all* %cargo_* macros (except %cargo_prep), otherwise you will get inconsistent results. For example, by adding the flags to %cargo_build / %cargo_install but not to %cargo_license / %cargo_license_summary, both the license summary and the generated breakdown (LICENSE.dependencies) will not match what is actually built and shipped. PS: v0.19.0 was released in the meantime. > I added the Unicode license, but I am just not sure how to process the rest. > Ring and rustls are gone. I couldn't easily verify this from the files you uploaded, since LICENSE.dependencies still lists it when built as-is. After modifying your spec file to apply the feature flags correctly to all macros, you are right, "ring" and "rustls" are gone. > I don't have a list yet. I tried to just build non-vendored, but that > wouldn't show a full report as, to my understanding, it wouldn't show > transient dependencies that are missing. But I did get some pointers from a > call today on how it could be possible to evaluate that in a more complete > fashion. I'll let you know when/if I get that working. True, by default, you would only get a first-level view of dependencies that are missing. > I would appreciate another round of review, if that's ok for you. Thanks for the update! I have another few suggestions: 1. You don't actually *need* to package this from the sources that are published on crates.io if you never expect "trunk" to provide a Rust library interface. See guidelines for this case: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_rust_applications_non_crates_io_crates I see that some tests are also disabled because files are missing from the tarball published on crates.io You could package this as "trunk" (i.e. without "rust-" prefix) and package from the GitHub tarball instead. That might make some things easier (including if trunk ever splits itself into multiple crates or stops publishing on crates.io - i.e. starts being a cargo workspace). The spec file would be very similar to what you have now (but a but simpler). You can try this by downloading the GitHub sources and running "rust2rpm ./trunk-0.19.0/Cargo.toml --vendor" to generate a "local project" spec file. 2. Some of your spec file additions / modifications look quite out of place. - "BuildRequires: openssl-devel" was added between Summary and License tags, instead of ... where there was already another BuildRequires line. I would also recommend to document that "openssl-devel" dependency is necessary because the package needs OpenSSL header files present to generate and build Rust bindings for OpenSSL. - The "ExcludeArch: %{ix86}" was added at the top. This is usually just preceding the BuildRequires. - Defining custom macros usually happens at the top of the spec file. I'm not sure why you added all the macros at the top except for _trunk_features, which is in a really odd place. -- 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%23c9 -- _______________________________________________ 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
