#10 Add cargo_vendor_manifest macro and and a _cargo_vendor generator
Merged by decathorpe. Opened by decathorpe.
Unknown source main

Download 10.patch

Add some machinery to help with packaging Rust projects with vendored dependencies.

  • %cargo_vendor_manifest macro: prints dependency tree in machine-readable format (using the same resolution mechanism as the cargo vendor command)
  • __cargo_vendor generator: automatically generates Provides: bundled(crate(...)) = x.y.z for contents of the vendor tarball based on the vendor manifest written by %cargo_vendor_manifest and installed by marking it as %license; requires cargo2rpm >= 0.1.8

The Provides generator works as expected when used for the rust-rpm-sequoia ELN package (with small modifications).

Note: you'll want to add Requires: cargo2rpm >= 0.1.8 when updating rust-packaging.spec.

Yeah, I don't like rust-toolset's %cargo_prep implementation either. I don't like how they just replaced our macro instead of making vendoring configurable, making it not upstream-able. I'd also like to remove the automatic archive unpacking. It's not transparent and should just be handled by the packager using the %autosetup -a1 (-a1 unpacks %{SOURCE1} after changing to the main archive's top directory) or a similar construct.

I would prefer to add a new flag to the current %cargo_prep macro to use sources from ./vendor without unpacking anything. If we want to preserve the automatic archive unpacking, I would still prefer to add the flag to the existing macro rather than having a second %cargo_prep_vendor. I'd also like to get rid of the heredoc embedded in shell embedded in a macro... it's a bit much :). Moving the cargo config into a separate file that we can simply cp is likely a better choice.

I don't immediately have a concrete idea to solve this, but I'll play around with it.

I'd also like to get rid of the heredoc embedded in shell embedded in a macro... it's a bit much :).

I decided moving the cargo configs into separate files and replacing placeholder values with the expanded macros was even worse, so never mind... There is the somewhat obscure %writevars macro that makes replacing placeholders in files with macros easier, but I still lean towards the current approach.

I don't immediately have a concrete idea to solve this, but I'll play around with it.

I came up with https://pagure.io/fork/gotmax23/fedora-rust/rust-packaging/commits/cargo_prep++. I tried to make it more flexible by allowing to specify a custom vendor directory or disable setting a custom registry altogether. I could definitely change it to work like RHEL's current version and accept -V, but I don't think its behavior is very sensible.

Anyways, thanks for working on this, and sorry for the long-winded feedback.

Thanks, I think your version is much better and more flexible. Doing the source unpacking internal to the macro is definitely weird and inflexible (it hard-codes and expects that the referenced tarball contains a toplevel vendor/ subdirectory, for example).

Do you want to submit a PR with that version of the %cargo_prep macro? I'll drop my %cargo_prep changes from this PR.

Note: you'll want to add Requires: cargo2rpm >= 0.1.8 when updating rust-packaging.spec.

Yes, that's the plan :)

rebased onto 7fe691288b88728b5111193e11713a1507bd28a6

rebased onto f4e2b139e9e33fd970189c345564713f4f9b80a4

Hm, now that I think of it ... would it be even better to call %cargo_vendor_manifest in %install instead of %build, and have it write the file to some standard %{buildroot}/%{_defaultlicensedir}/%{name}/cargo-vendor.txt (with an optional argument to override %{name}? That would reduce the number of steps involved (i.e. no need to manually add the generated file to %files with %license).

Doing the source unpacking internal to the macro is definitely weird and inflexible (it hard-codes and expects that the referenced tarball contains a toplevel vendor/ subdirectory, for example).

Exactly.

Do you want to submit a PR with that version of the %cargo_prep macro? I'll drop my %cargo_prep changes from this PR.

Sure, I'll do that later.

Hm, now that I think of it ... would it be even better to call %cargo_vendor_manifest in %install instead of %build, and have it write the file to some standard %{buildroot}/%{_defaultlicensedir}/%{name}/cargo-vendor.txt (with an optional argument to override %{name}? That would reduce the number of steps involved (i.e. no need to manually add the generated file to %files with %license).

You'd still need to include the path in %files (otherwise you'd get unpackaged file errors), so I'm not sure that would make sense.

Do you want to submit a PR with that version of the %cargo_prep macro? I'll drop my %cargo_prep changes from this PR.

Sure, I'll do that later.

Great, thanks!

Hm, now that I think of it ... would it be even better to call %cargo_vendor_manifest in %install instead of %build, and have it write the file to some standard %{buildroot}/%{_defaultlicensedir}/%{name}/cargo-vendor.txt (with an optional argument to override %{name}? That would reduce the number of steps involved (i.e. no need to manually add the generated file to %files with %license).

You'd still need to include the path in %files (otherwise you'd get unpackaged file errors), so I'm not sure that would make sense.

You're right, I forgot about that. In that case the way it is now is better. :) I'll merge this now so you don't have to rebase your PR later.

Pull-Request has been merged by decathorpe

Metadata