https://bugzilla.redhat.com/show_bug.cgi?id=2326979 --- Comment #6 from Uri Lublin <uril@xxxxxxxxxx> --- (In reply to Fabio Valentini from comment #3) Fabio, thank you for looking too. I added the rust2rpm.toml used in comment 4. > I agree, the changes made to the package look strange. > > The patch does some things that I would classify as "suspicious": > > 1. Bumping the bitflags dependency from ^1.2 to ^1.3 - this looks like a > misunderstanding of what `version = "1.2"` means (it means "1.2 or any > version compatible with it, i.e. <2.0", which includes 1.3.x) Okay, I'll leave it at "1.2". > > 2. Removing the dependencies for the "generate" and "jwt-convert" features, > but not the features themselves is wrong. > If these features should not be available in the packaged crate, just > dropping the dependencies but not the features themselves will have > unintentional consequences (for example, the subpackages for these features > will still be generated in the .spec file). The features "generate" and "jwt-convert" are not needed and so are "hidden". Removing features in Cargo.toml can be risky, since then code with e.g. '#[cfg(feature = "<removed-feature>")] fails to build. If the dependencies of hidden features stay in Cargo.toml, they are still required. That can be a problem especially if they are not in Fedora. I found that having the "hidden" features in Cargo.toml but removing their dependencies solves it - the code builds, the hidden features do not become subpackages and their dependencies are not required. > > 3. Moving "num-bigint" and "yasna" into "named dependencies" (i.e. > "dep:num-bigint" and "dep:yasna") is an unnecessary divergence from the > upstream crate metadata, and could lead to problems with dependent crates > which could expect the implicitly defined features for "num-bigint" and > "yasna" to be present. I think there is a problem with rust2rpm. I am not sure so I did not yet report it. The problem is that, without changes, rust2rpm creates a spec file with subpackages for dependencies (not features). With the added "dep:" for all dependencies rust2rpm works correctly. For example, with this package (with the above rust2rpm.toml file): $ rust2rpm jsonwebkey 0.3.5 • Generated: rust-jsonwebkey.spec $ grep '^.package' rust-jsonwebkey.spec %package devel %package -n %{name}+jsonwebtoken-devel %package -n %{name}+num-bigint-devel %package -n %{name}+p256-devel %package -n %{name}+pkcs-convert-devel %package -n %{name}+rand-devel %package -n %{name}+yasna-devel > > 4. Setting `%global cargo_install_bin 0` is a noop, this crate does not > contain any executables. I don't know where you got this from, but it is > useless here. Okay, I can remove it, if you prefer. With it in the rust2rpm.toml file, I'm sure that even if there are binaries they are not included. > > 5. Do not drop the rust-%{crate}-$feature-devel subpackages from the > generated spec file manually. They are there for a reason, and they MUST > match cargo metadata to avoid build failures or unexpected problems in > packages for dependent crates. > > In particular, dropping the subpackage for the "default" feature is > problematic. The "default" feature is unconditionally defined by cargo even > if it is not explicitly mentioned in crate metadata. I'll "unhide" the "default" feature. > > In general, I would recommend not to make any changes to the spec file > generated by rust2rpm unless you are *sure* that they are correct and > necessary. -- 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=2326979 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202326979%23c6 -- _______________________________________________ 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
