Propagate 'public' field on dependencies to the index #1685
Conversation
This implements the crates.io side of rust-lang/rust#44663 This is fully backwards-compatible - 'public' will default to 'false' for old versions of Cargo that don't include it in their manifest
Aaron1011
force-pushed
the
feature/pub-priv-dep
branch
from
0cdf589 to
903e6be
Compare
|
I'd like to see an explicit test publishing a crate with a public dependency. Has the cargo side of this already been implemented? Can I test this today on nightly? Would also like to get someone from @rust-lang/cargo to confirm this is what they want in the index |
When I added the
Not yet, I am hoping that that is where @Aaron1011 is heading next. We do have experimental support for it in the resolver, and this is the only bit of information we need there. There are some open questions about how this will interact with other functionality, that may make things more complicated. One thought is that we can tell |
|
@Eh2406: I think it should be fine to start writing the |
|
Yes, I agree. This won't break anything. What I was thinking is if we end up needing an |
|
I'm wondering, in general, what the expectations are on a timeline for enabling support in the official index. I think it would be best to set up an alternative registry to experiment with until the details are certain. I might be able to carry a patch in my staging area, if that helps. |
This is part of rust-lang/rust#44663 This implements the 'frontend' portion of RFC 1977. Once PRs rust-lang/rust#59335 and rust-lang/crates.io#1685 are merged, it will be possible to test the full public-private dependency feature: marking a dependency a public, seeing exported_private_dependencies warnings from rustc, and seeing pub-dep-reachability errors from Cargo. Everything in this commit should be fully backwards-compatible - users who don't enable the 'public-dependency' cargo feature won't notice any changes. Note that this commit does *not* implement the remaining two features of the RFC: * Choosing smallest versions when 'cargo publish' is run * Turning exported_private_dependencies warnings into hard errors when 'cargo publish' is run The former is a major change to Cargo's behavior, and should be done in a separate PR with some kind of rollout plan. The latter is described by the RFC as being enabled at 'some point in the future'. This can be done via a follow-up PR.
Implement the 'frontend' of public-private dependencies This is part of rust-lang/rust#44663 This implements the 'frontend' portion of [RFC 1977](https://github.com/rust-lang/rfcs/blob/master/text/1977-public-private-dependencies.md). Once PRs rust-lang/rust#59335 and rust-lang/crates.io#1685 are merged, it will be possible to test the full public-private dependency feature: marking a dependency a public, seeing exported_private_dependencies warnings from rustc, and seeing pub-dep-reachability errors from Cargo. Everything in this commit should be fully backwards-compatible - users who don't enable the 'public-dependency' cargo feature won't notice any changes. Note that this commit does *not* implement the remaining two features of the RFC: * Choosing smallest versions when 'cargo publish' is run * Turning exported_private_dependencies warnings into hard errors when 'cargo publish' is run The former is a major change to Cargo's behavior, and should be done in a separate PR with some kind of rollout plan. The latter is described by the RFC as being enabled at 'some point in the future'. This can be done via a follow-up PR.
|
Now that the Cargo frontend has been merged, it's possible to test this with Cargo. |
|
Thanks @Aaron1011. If I understand your comment in rust-lang/rust#44663 (comment) correctly, there is a remaining bullet point to resolve before merging this. If practical, I would also like to wait until this feature is stable on nightly before allowing anyone to publicly publish crates using this feature to the index. Does that seem okay, or are there reasons to merge this sooner than that? |
|
@jtgeibel: By 'stable on nightly', do you mean waiting until the first bullet point (deciding how to handle renamed dependencies) is dealt with? That sounds reasonable to me. |
|
As I brought up in rust-lang/cargo#6129 (comment), I don't think that handling renamed dependencies will require changes to the index format. |
|
@jtgeibel: I don't think this should be considered blocked anymore |
|
Is there anything that I can do to help move this along? |
|
Having another member of the cargo team sign off on this would be helpful. We also still need some form of test for this |
|
We discussed this at the Cargo triage meeting yesterday, and my own personal conclusion is that we're probably not ready to land this at this time. After talking more about the current state of the feature and what it looks like, this is definitely sufficient for exactly what's implemented today but I'm not so certain we want to commit to this just yet. There were a number of ergonomic concerns about the feature today and the rollout plan which makes us hesistant enough that we're pretty unlikely to stabilize the feature as-is, and changes to the feature are likely going to require changes to the index format, in which case would require updates here. Changes to the index aren't catastrophic, but we figured should be done carefully and avoided if possible. For the actual feature and rollout though, I think it's probably best to continue discussion on the Cargo tracking issue, and I think @Eh2406 you were gonna write up some more information there? |
|
I'm a little unclear on the current status of this effort after reading through the tracking issue, but given these comments here:
I'm going to close this PR for now. Please reopen this PR or open a new PR when the path forward is clearer! |
This implements the crates.io side of rust-lang/rust#44663
This is fully backwards-compatible - 'public' will default to 'false'
for old versions of Cargo that don't include it in their manifest