feat(build-std): Introduce opaque dependencies and builtin source - #17398
feat(build-std): Introduce opaque dependencies and builtin source#17398adamgemmell wants to merge 10 commits into
Conversation
rustbot
added
A-crate-dependencies
|
r? @weihanglo rustbot has assigned @weihanglo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
92fc1dd to
859aaf2
Compare
This comment has been minimized.
This comment has been minimized.
This is technically not necessary - all Builtins are currently opaque and vice-versa. However I feel separating the concern of opaque dependencies from that of builtins is important here even if opaque dependencies are not yet supported more generally.
859aaf2 to
b79ca52
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| .url | ||
| .to_file_path() | ||
| .expect("builtin sources cannot be remote"); | ||
| Ok(Box::new(BuiltinSource::new(&path, self, gctx))) |
There was a problem hiding this comment.
Existing concern from @epage regarding which source to use #16675 (comment)
Since then this patch now uses a BuiltinSource which wraps a RecursivePathSource.
| /// The summary has no dependencies and is artificial - it is used purely guide the resolver | ||
| /// by satisfying opaque dependencies and is discarded during Unit generation. The real | ||
| /// packages that are converted into `Unit`s come from a different invocation of the resolver. | ||
| pub fn new_opaque(pkg_id: PackageId, sid: SourceId) -> Self { |
There was a problem hiding this comment.
Existing concern from @epage regarding the best way of constructing an opaque Summary #16675 (comment)
Since then I've split that logic into a function that looks much like the other Summary constructors.
3 participants
What does this PR try to resolve?
This PR, part of a stack split off from #16675, introduces:
Opaque dependencies, as mentioned in the
build-std=alwaysRFC. An opaque dependency is one that cannot be fully resolved when encounted and is satisfied by an opaque Summary with no dependencies (which stops the resolver from recursing further). A future PR will handle the opaque summary by replacing it during unit generation with a package from a separate resolve, such as the standard library one.A new builtin source which discovers packages in the standard library source and generates opaque Summaries for them in order to satisfy dependencies which are specifically both opaque and Builtin. These concepts are intertwined at this stage to simplify implementation for build-std, but can be separated if Cargo decides to implement opaque dependencies more generally.
How to test and review this PR?
The PR introduces unit tests for all new behaviour and I recommend reading the commits in order. This PR cannot be manually tested.
There is a very large amount of context behind build-std. Please feel free to spam me with questions and I can link to relevant bits of this context than reviewers needing to reread this context every time you need to review a build-std PR if you prefer.
This PR is part of a stack: