-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: resolve crates.io source replacement ambiguity #3289
Conversation
I'd expand the language in Change 3 to clarify that credentials for one registry must not be sent to any other registry, regardless of whether either is crates.io. Any exception to that should have to be an explicit configuration by the user (and probably not in a place that would come from source control), or something controlled by the registry for which you have credentials (e.g. maybe crates.io could somehow say official-second-site-of-crates-io.net can receive crates.io credentials) |
…aced API operations an error
|
||
Should the `--registry <NAME>` command line argument be allowed to reference the name of a `source` from the `[source]` table as well? This makes it more flexible, but adds potentially unnecessary complexity. | ||
|
||
Cargo's tests rely on the ability to replace the crates.io source and have the crates.io credentials go to the replaced source. We need a way for these tests to continue working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too worried about this, as I believe all of the relevant tests could be adjusted to include --registry
if necessary. In the past, the tests would pass --index
, though that was removed in rust-lang/cargo#7973.
It is unfortunate that it is difficult to test cargo's behavior without using source replacement. There are some tests that are impossible to write, but I don't think it should be too much of a problem.
Team member @ehuss 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. |
Oh, I did just think of one thing. If you are using a mirror of crates-io, and you run I'm wondering if it would make sense to leave the verification step to use source replacement? I can't think of a strong reason to change that. The mirror might lag the original source, but in that case the user can either wait if they are publishing multiple inter-dependent packages, or they can use Can you say more why that particular statement is there? |
Yes, when using If the verification step followed source replacement, then Cargo would need to fetch the index for the replacement and crates.io (the It's currently not possible to publish to crates.io with source replacement configured, so this is not a loss of functionality. Users in China currently have to disable the USTC replacement in the config in order to publish. I suppose if we were using Edit: In the implementation of this RFC, the verification build does use the source replacement. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for writing this! Just some nitpicks.
Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
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. |
Huzzah! The @rust-lang/cargo team has decided to accept this RFC. To track further discussion, subscribe to the tracking issue here: |
Implement RFC 3289: source replacement ambiguity ### Implements [RFC 3289](rust-lang/rfcs#3289) * When the crates-io source is replaced, the user needs to specify `--registry <NAME>` when running an API operation to disambiguate which registry to use. Otherwise, cargo will issue a new error. * In source replacement, the `replace-with` key can reference the name of an alt registry in the `[registries]` table. * Publishing to source-replaced crates.io is no longer permitted using the crates.io token (`registry.token`). We have had a deprecation warning in place since #7973 (1.45.0). ### Testing * Tests are updated to add the `--registry dummy-registry` parameter to specify the test registry (otherwise they would get the new error message) * A few tests that need to verify crates-io-specific configuration use an internal `allow_silent_crates_io_replacement` function to allow the previous behavior of silently replacing crates.io within the testing framework. Changes are insta-stable. cc #10894 r? `@Eh2406`
Implement RFC 3289: source replacement ambiguity ### Implements [RFC 3289](rust-lang/rfcs#3289) * When the crates-io source is replaced, the user needs to specify `--registry <NAME>` when running an API operation to disambiguate which registry to use. Otherwise, cargo will issue a new error. * In source replacement, the `replace-with` key can reference the name of an alt registry in the `[registries]` table. * Publishing to source-replaced crates.io is no longer permitted using the crates.io token (`registry.token`). We have had a deprecation warning in place since #7973 (1.45.0). ### Testing * Tests that interacting with crates.io use the new `replace_crates_io` function, which internally sets an environment variable to change the URL of crates.io. Changes are insta-stable. cc #10894 r? `@Eh2406`
Implement RFC 3289: source replacement ambiguity ### Implements [RFC 3289](rust-lang/rfcs#3289) * When the crates-io source is replaced, the user needs to specify `--registry <NAME>` when running an API operation to disambiguate which registry to use. Otherwise, cargo will issue a new error. * In source replacement, the `replace-with` key can reference the name of an alt registry in the `[registries]` table. * Publishing to source-replaced crates.io is no longer permitted using the crates.io token (`registry.token`). We have had a deprecation warning in place since #7973 (1.45.0). ### Testing * Tests that interacting with crates.io use the new `replace_crates_io` function, which internally sets an environment variable to change the URL of crates.io. Changes are insta-stable. cc #10894 r? `@Eh2406`
Implement RFC 3289: source replacement ambiguity ### Implements [RFC 3289](rust-lang/rfcs#3289) * When the crates-io source is replaced, the user needs to specify `--registry <NAME>` when running an API operation to disambiguate which registry to use. Otherwise, cargo will issue a new error. * In source replacement, the `replace-with` key can reference the name of an alt registry in the `[registries]` table. * Publishing to source-replaced crates.io is no longer permitted using the crates.io token (`registry.token`). We have had a deprecation warning in place since #7973 (1.45.0). ### Testing * Tests that interacting with crates.io use the new `replace_crates_io` function, which internally sets an environment variable to change the URL of crates.io. Changes are insta-stable. cc #10894 r? `@Eh2406`
When Cargo is performing an API operation (
yank
/login
/publish
/etc.) to a source-replacedcrates-io
, require the user to pass--registry <NAME>
to specify exactly which registry to use. Additionally, ensure that the token forcrates-io
is never sent to a replacement registry.Rendered