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.

Sign up for GitHub

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

Jump to bottom

Fix publishing to crates.io with -Z sparse-registry #10831

Merged
merged 1 commit into from
Jul 7, 2022
Merged

Fix publishing to crates.io with -Z sparse-registry #10831

merged 1 commit into from
Jul 7, 2022

Conversation

arlosi
Copy link
Contributor

@arlosi arlosi commented Jul 6, 2022

Attempting to publish a crate to crates.io with -Z sparse-registry failed with the following error:

error: failed to publish to registry at https://crates.io

Caused by:
  the remote server responded with an error: Dependency `serde` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.

The check in registry.rs dep_registry_id != registry_id caused the publish operation include the crates.io index url in the HTTP request because the id was replaced. The crates.io API seems to require that the registry field is not present.

This change fixes the issue by making the registry function return the non-replaced crates.io source_id only for this case. Other replacement indices of crates.io continue to include the registry URL when publishing.

Tested manually by publishing arlosi-cargo-test to crates.io with -Z sparse-registry

Fixes #10828
r? @Eh2406

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 6, 2022
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 7, 2022

Is there any way to add a test for this?
Otherwise it looks good.

@arlosi
Copy link
Contributor Author

arlosi commented Jul 7, 2022

Is there any way to add a test for this?

I don't know of a way to test it as is. The tests for publishing to crates.io actually do include the problematic registry field, since they're publishing to a replaced crates.io. In this PR I did the minimal change of omitting it only for index.crates.io.

If we want to be able to test it, then we need to make a breaking change such that publishing to replaced crates.io doesn't include the registry field in the request. It's unclear to me whether that field should (from a design perspective) be included. Maybe something to discuss in rust-lang/rfcs#3289 .

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 7, 2022

@bors r+

The general problem is part of rust-lang/rfcs#3289 but this can be a one off.

@bors
Copy link
Contributor

bors commented Jul 7, 2022

📌 Commit 132afd3 has been approved by Eh2406

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2022
@bors
Copy link
Contributor

bors commented Jul 7, 2022

⌛ Testing commit 132afd3 with merge 6867277...

@bors
Copy link
Contributor

bors commented Jul 7, 2022

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing 6867277 to master...

@bors bors merged commit 6867277 into rust-lang:master Jul 7, 2022
weihanglo added a commit to weihanglo/rust that referenced this pull request Jul 10, 2022
@weihanglo
7 commits in c0bbd42ce5e83fe2a93e817c3f9b955492d3130a..b1dd22e668af52…
d22c88a 
…79e13a071ad4b17435bd6bfa4c

2022-07-03 13:41:11 +0000 to 2022-07-09 14:48:50 +0000

- Mention `[patch]` config in "Overriding Dependencies" (rust-lang/cargo#10836)
- Update terminal-width flag. (rust-lang/cargo#10833)
- Refactor check_yanked to avoid some duplication (rust-lang/cargo#10835)
- Fix publishing to crates.io with -Z sparse-registry (rust-lang/cargo#10831)
- Make `is_yanked` return `Poll<>` (rust-lang/cargo#10830)
- Fix corrupted git checkout recovery. (rust-lang/cargo#10829)
- add a cache for discovered workspace roots (rust-lang/cargo#10776)
@weihanglo weihanglo mentioned this pull request Jul 10, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2022
@bors
Auto merge of rust-lang#99107 - weihanglo:update-cargo, r=ehuss
c396bb3 
Update cargo

7 commits in c0bbd42ce5e83fe2a93e817c3f9b955492d3130a..b1dd22e668af5279e13a071ad4b17435bd6bfa4c
2022-07-03 13:41:11 +0000 to 2022-07-09 14:48:50 +0000

- Mention `[patch]` config in "Overriding Dependencies" (rust-lang/cargo#10836)
- Update terminal-width flag. (rust-lang/cargo#10833)
- Refactor check_yanked to avoid some duplication (rust-lang/cargo#10835)
- Fix publishing to crates.io with -Z sparse-registry (rust-lang/cargo#10831)
- Make `is_yanked` return `Poll<>` (rust-lang/cargo#10830)
- Fix corrupted git checkout recovery. (rust-lang/cargo#10829)
- add a cache for discovered workspace roots (rust-lang/cargo#10776)
@weihanglo weihanglo added the relnotes Release-note worthy label Jul 19, 2022
@ehuss ehuss added this to the 1.64.0 milestone Jul 20, 2022
@ehuss ehuss mentioned this pull request Aug 10, 2022
Closed
11 tasks
@joshtriplett
Copy link
Member

Dropping relnotes because this relates to -Z sparse-registry which isn't stable yet.

@joshtriplett joshtriplett removed the relnotes Release-note worthy label Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Eh2406 Eh2406

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.64.0
7 participants
@arlosi @Eh2406 @bors @joshtriplett @ehuss @rust-highfive @weihanglo