Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple artifact deps on the same crate with different names, for different targets #3176

Merged
merged 6 commits into from
Nov 2, 2021

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Sep 19, 2021

Rendered

Allow Cargo packages to depend on the same crate multiple times with different dependency names, to support artifact dependencies for multiple targets.

RFC 3028 specified "artifact dependencies", allowing crates to depend on a compiled binary provided by another crate, for a specified target.

Some crates need to depend on binaries for multiple targets; for instance, a virtual machine that supports running multiple targets may need firmware for each target platform. Sometimes these binaries may come from different crates, but sometimes these binaries may come from the same crate compiled for different targets.

This RFC enables that use case, by allowing multiple dependencies on the same crate with the same version, as long as they're each renamed to a different name. This allows multiple artifact dependencies on the same crate for different targets.

Note that this RFC still does not allow dependencies on different semver-compatible versions of the same crate, only multiple dependencies on exactly the same version of the same crate.

Work sponsored by Profian.

@joshtriplett joshtriplett added T-cargo Relevant to the Cargo team, which will review and decide on the RFC. I-nominated labels Sep 19, 2021
@joshtriplett
Copy link
Member Author

One additional detail: I have someone lined up who is interested in implementing both this and the baseline artifact dependencies. I'm happy to supply the review bandwidth, and they'll supply the implementation bandwidth.

@joshtriplett
Copy link
Member Author

As discussed in the previous cargo team meeting, starting FCP to confirm consensus. This RFC implements the approach previously suggested for how to handle artifact dependencies for multiple different targets.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 27, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Sep 27, 2021
@alexcrichton
Copy link
Member

From the way this is written it sounds like the intention is that features are unified across all targets, but I think that's something that we've learned is generally a no-no the goal of Cargo is to have activated features-per-target. In that sense I would expect that multiple dependencies on a crate with different targets would each get different sets of features. For example you might want the wasm build to use wasm-bindgen but the arm build shouldn't do that most likely.

@npmccallum
Copy link
Contributor

@joshtriplett We definitely don't want features unified across all targets (per @alexcrichton).

@joshtriplett
Copy link
Member Author

joshtriplett commented Oct 2, 2021

@alexcrichton I'll fix that then; from previous discussions I'd thought that unifying was easier and allowing non-unified features was harder. If the preference is to not unify, that would absolutely be better here.

@joshtriplett
Copy link
Member Author

@alexcrichton I've now updated the RFC to avoid unifying features across targets. Thanks!

@Nemo157
Copy link
Member

Nemo157 commented Oct 14, 2021

One thing that might be worth mentioning with the latest change is whether the features of a library dependency will be unified with the features of an artifact dependency on the build's target:

[dependencies]
example = { package = "example", version = "1.2.3", features = ["lib_feature"] }
example_arm = { package = "example", version = "1.2.3", artifact = "bin", target = "aarch64-unknown-none", features = ["bin_feature"] }

Will cargo build --target aarch64-unknown-none result in a single build of example or is the binary artifact target in some way distinguished from the library target.

@joshtriplett
Copy link
Member Author

@Nemo157 As currently written, it'll unify if they're the same target, but I could change that easily enough. @alexcrichton Which did you intend?

@alexcrichton
Copy link
Member

Looks good to me, thanks! And yeah for @Nemo157's example I'd expect that the features would get unified because they're built for the same target.

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Oct 14, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 14, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Oct 14, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Oct 24, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 24, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@ehuss
Copy link
Contributor

ehuss commented Nov 2, 2021

Huzzah! The @rust-lang/cargo team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/cargo#10030

@ehuss ehuss merged commit 6214e77 into rust-lang:master Nov 2, 2021
@joshtriplett joshtriplett deleted the multidep branch November 2, 2021 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-cargo Relevant to the Cargo team, which will review and decide on the RFC. to-announce
Projects
No open projects
Status: Accepted
Development

Successfully merging this pull request may close these issues.

7 participants