Skip to content

Commit 7e484fc

Browse files
borsbors
committed
Auto merge of #11062 - epage:wait, r=weihanglo
fix(publish): Block until it is in index Originally, crates.io would block on publish requests until the publish was complete, giving `cargo publish` this behavior by extension. When crates.io switched to asynchronous publishing, this intermittently broke people's workflows when publishing multiple crates. I say interittent because it usually works until it doesn't and it is unclear why to the end user because it will be published by the time they check. In the end, callers tend to either put in timeouts (and pray), poll the server's API, or use `crates-index` crate to poll the index. This isn't sufficient because - For any new interested party, this is a pit of failure they'll fall into - crates-index has re-implemented index support incorrectly in the past, currently doesn't handle auth, doesn't support `git-cli`, etc. - None of these previous options work if we were to implement workspace-publish support (#1169) - The new sparse registry might increase the publish times, making the delay easier to hit manually - The new sparse registry goes through CDNs so checking the server's API might not be sufficient - Once the sparse registry is available, crates-index users will find out when the package is ready in git but it might not be ready through the sparse registry because of CDNs So now `cargo` will block until it sees the package in the index. - This is checking via the index instead of server APIs in case there are propagation delays. This has the side effect of being noisy because of all of the "Updating index" messages. - This blocks by default but there is an unstable `publish.timeout` config field that will disable blocking when set to 0. See #11222 for stablization Blocking is opt-out as that is the less error prone case for casual users while those doing larger integrations are also likely to do the testing needed to make more complicated scenarios work where blocking is disabled. Right now we block after the publish. An alternative would be to block until all dependencies are in the index which makes the blocking only happen when needed - Blocking on dependencies can be imprecise to detect when to block vs propagate an error up - This is the less error prone case for users. For example I recently publish a crate in one tab and immediately switched to another tab to use it and this only worked because `cargo-release` blocked until it was ready to use In reviewing this change, be sure to look at the individual commits - The first makes it possible to write the tests for this - The second adds a test that shows the current behavior - The third updates the test to the expected behavior, showing all of this works In addition to the publish tests: - We want to maximize the nightly-to-stable time to collect feedback - We will put this in TWiR's testing section to raise visibility Fixes #9507
2 parents 1985caf + f2fc5ca commit 7e484fc

8 files changed

+246
-88
lines changed

‎src/cargo/ops/registry.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
192192
opts.dry_run,
193193
)?;
194194
if !opts.dry_run {
195-
const DEFAULT_TIMEOUT: u64 = 0;
195+
const DEFAULT_TIMEOUT: u64 = 60;
196196
let timeout = if opts.config.cli_unstable().publish_timeout {
197197
let timeout: Option<u64> = opts.config.get("publish.timeout")?;
198198
timeout.unwrap_or(DEFAULT_TIMEOUT)

‎tests/testsuite/artifact_dep.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1872,7 +1872,9 @@ fn env_vars_and_build_products_for_various_build_targets() {
18721872

18731873
#[cargo_test]
18741874
fn publish_artifact_dep() {
1875+
// HACK below allows us to use a local registry
18751876
let registry = registry::init();
1877+
18761878
Package::new("bar", "1.0.0").publish();
18771879
Package::new("baz", "1.0.0").publish();
18781880

@@ -1901,6 +1903,15 @@ fn publish_artifact_dep() {
19011903
.file("src/lib.rs", "")
19021904
.build();
19031905

1906+
// HACK: Inject `foo` directly into the index so `publish` won't block for it to be in
1907+
// the index.
1908+
//
1909+
// This is to ensure we can verify the Summary we post to the registry as doing so precludes
1910+
// the registry from processing the publish.
1911+
Package::new("foo", "0.1.0")
1912+
.file("src/lib.rs", "")
1913+
.publish();
1914+
19041915
p.cargo("publish -Z bindeps --no-verify")
19051916
.replace_crates_io(registry.index_url())
19061917
.masquerade_as_nightly_cargo(&["bindeps"])
@@ -1909,6 +1920,7 @@ fn publish_artifact_dep() {
19091920
[UPDATING] [..]
19101921
[PACKAGING] foo v0.1.0 [..]
19111922
[UPLOADING] foo v0.1.0 [..]
1923+
[UPDATING] [..]
19121924
",
19131925
)
19141926
.run();

‎tests/testsuite/credential_process.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Tests for credential-process.
22
3-
use cargo_test_support::registry::TestRegistry;
3+
use cargo_test_support::registry::{Package, TestRegistry};
44
use cargo_test_support::{basic_manifest, cargo_process, paths, project, registry, Project};
55
use std::fs;
66

@@ -94,6 +94,16 @@ fn warn_both_token_and_process() {
9494
.file("src/lib.rs", "")
9595
.build();
9696

97+
// HACK: Inject `foo` directly into the index so `publish` won't block for it to be in
98+
// the index.
99+
//
100+
// This is to ensure we can verify the Summary we post to the registry as doing so precludes
101+
// the registry from processing the publish.
102+
Package::new("foo", "0.1.0")
103+
.file("src/lib.rs", "")
104+
.alternative(true)
105+
.publish();
106+
97107
p.cargo("publish --no-verify --registry alternative -Z credential-process")
98108
.masquerade_as_nightly_cargo(&["credential-process"])
99109
.with_status(101)
@@ -125,6 +135,7 @@ Only one of these values may be set, remove one or the other to proceed.
125135
[UPDATING] [..]
126136
[PACKAGING] foo v0.1.0 [..]
127137
[UPLOADING] foo v0.1.0 [..]
138+
[UPDATING] [..]
128139
",
129140
)
130141
.run();
@@ -198,6 +209,7 @@ fn publish() {
198209
[UPDATING] [..]
199210
[PACKAGING] foo v0.1.0 [..]
200211
[UPLOADING] foo v0.1.0 [..]
212+
[UPDATING] [..]
201213
",
202214
)
203215
.run();

‎tests/testsuite/cross_publish.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ fn publish_with_target() {
6666
return;
6767
}
6868

69-
let registry = registry::init();
69+
// `publish` generally requires a remote registry
70+
let registry = registry::RegistryBuilder::new().http_api().build();
7071

7172
let p = project()
7273
.file(
@@ -109,6 +110,7 @@ fn publish_with_target() {
109110
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
110111
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
111112
[UPLOADING] foo v0.0.0 ([CWD])
113+
[UPDATING] crates.io index
112114
",
113115
)
114116
.run();

‎tests/testsuite/features_namespaced.rs

+24
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,9 @@ bar v1.0.0
858858

859859
#[cargo_test]
860860
fn publish_no_implicit() {
861+
// HACK below allows us to use a local registry
861862
let registry = registry::init();
863+
862864
// Does not include implicit features or dep: syntax on publish.
863865
Package::new("opt-dep1", "1.0.0").publish();
864866
Package::new("opt-dep2", "1.0.0").publish();
@@ -885,13 +887,23 @@ fn publish_no_implicit() {
885887
.file("src/lib.rs", "")
886888
.build();
887889

890+
// HACK: Inject `foo` directly into the index so `publish` won't block for it to be in
891+
// the index.
892+
//
893+
// This is to ensure we can verify the Summary we post to the registry as doing so precludes
894+
// the registry from processing the publish.
895+
Package::new("foo", "0.1.0")
896+
.file("src/lib.rs", "")
897+
.publish();
898+
888899
p.cargo("publish --no-verify")
889900
.replace_crates_io(registry.index_url())
890901
.with_stderr(
891902
"\
892903
[UPDATING] [..]
893904
[PACKAGING] foo v0.1.0 [..]
894905
[UPLOADING] foo v0.1.0 [..]
906+
[UPDATING] [..]
895907
",
896908
)
897909
.run();
@@ -971,7 +983,9 @@ feat = ["opt-dep1"]
971983

972984
#[cargo_test]
973985
fn publish() {
986+
// HACK below allows us to use a local registry
974987
let registry = registry::init();
988+
975989
// Publish behavior with explicit dep: syntax.
976990
Package::new("bar", "1.0.0").publish();
977991
let p = project()
@@ -997,6 +1011,15 @@ fn publish() {
9971011
.file("src/lib.rs", "")
9981012
.build();
9991013

1014+
// HACK: Inject `foo` directly into the index so `publish` won't block for it to be in
1015+
// the index.
1016+
//
1017+
// This is to ensure we can verify the Summary we post to the registry as doing so precludes
1018+
// the registry from processing the publish.
1019+
Package::new("foo", "0.1.0")
1020+
.file("src/lib.rs", "")
1021+
.publish();
1022+
10001023
p.cargo("publish")
10011024
.replace_crates_io(registry.index_url())
10021025
.with_stderr(
@@ -1007,6 +1030,7 @@ fn publish() {
10071030
[COMPILING] foo v0.1.0 [..]
10081031
[FINISHED] [..]
10091032
[UPLOADING] foo v0.1.0 [..]
1033+
[UPDATING] [..]
10101034
",
10111035
)
10121036
.run();

0 commit comments

Comments
 (0)