Skip to content

Commit

Permalink
fix(publish): Block until it is in index
Browse files Browse the repository at this point in the history
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 is done unconditionally because cargo used to block and that
  didn't seem to be a problem, blocking by default is the less error
  prone case, and there doesn't seem to be enough justification for a
  "don't block" flag.

The timeout was 5min but I dropped it to 1m.  Unfortunately, I don't
have data from `cargo-release` to know what a reasonable timeout is, so
going ahead and dropping to 60s and assuming anything more is an outage.

Fixes #9507
  • Loading branch information
epage committed Oct 13, 2022
1 parent 487d7e5 commit f2fc5ca
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 32 deletions.
2 changes: 1 addition & 1 deletion src/cargo/ops/registry.rs
Expand Up @@ -186,7 +186,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
opts.dry_run,
)?;
if !opts.dry_run {
const DEFAULT_TIMEOUT: u64 = 0;
const DEFAULT_TIMEOUT: u64 = 60;
let timeout = if opts.config.cli_unstable().publish_timeout {
let timeout: Option<u64> = opts.config.get("publish.timeout")?;
timeout.unwrap_or(DEFAULT_TIMEOUT)
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/artifact_dep.rs
Expand Up @@ -1920,6 +1920,7 @@ fn publish_artifact_dep() {
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/credential_process.rs
Expand Up @@ -135,6 +135,7 @@ Only one of these values may be set, remove one or the other to proceed.
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down Expand Up @@ -208,6 +209,7 @@ fn publish() {
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cross_publish.rs
Expand Up @@ -110,6 +110,7 @@ fn publish_with_target() {
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[UPLOADING] foo v0.0.0 ([CWD])
[UPDATING] crates.io index
",
)
.run();
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/features_namespaced.rs
Expand Up @@ -903,6 +903,7 @@ fn publish_no_implicit() {
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down Expand Up @@ -1029,6 +1030,7 @@ fn publish() {
[COMPILING] foo v0.1.0 [..]
[FINISHED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down
75 changes: 46 additions & 29 deletions tests/testsuite/publish.rs
Expand Up @@ -123,6 +123,7 @@ fn simple() {
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();
Expand Down Expand Up @@ -176,6 +177,7 @@ fn old_token_location() {
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();
Expand Down Expand Up @@ -215,7 +217,9 @@ fn simple_with_index() {
[..]
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -410,7 +414,9 @@ fn publish_clean() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -453,7 +459,9 @@ fn publish_in_sub_repo() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -496,7 +504,9 @@ fn publish_when_ignored() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -538,7 +548,9 @@ fn ignore_when_crate_ignored() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -727,7 +739,9 @@ fn publish_allowed_registry() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] `alternative` index
",
)
.run();

Expand Down Expand Up @@ -789,7 +803,9 @@ fn publish_implicitly_to_only_allowed_registry() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] `alternative` index
",
)
.run();

Expand Down Expand Up @@ -912,7 +928,9 @@ The registry `alternative` is not listed in the `publish` value in Cargo.toml.
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] crates.io index
",
)
.run();
}
Expand Down Expand Up @@ -958,6 +976,7 @@ fn publish_with_select_features() {
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -1004,6 +1023,7 @@ fn publish_with_all_features() {
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -1112,7 +1132,9 @@ fn publish_with_patch() {
[..]
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] crates.io index
",
)
.run();

Expand Down Expand Up @@ -1316,7 +1338,9 @@ fn publish_git_with_version() {
[..]
[..]
[..]
[UPLOADING] foo v0.1.0 ([CWD])",
[UPLOADING] foo v0.1.0 ([CWD])
[UPDATING] crates.io index
",
)
.run();

Expand Down Expand Up @@ -1443,6 +1467,7 @@ fn publish_dev_dep_no_version() {
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -1526,6 +1551,7 @@ fn credentials_ambiguous_filename() {
[..]
[..]
[UPLOADING] foo v0.0.1 [..]
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -1934,6 +1960,7 @@ fn in_package_workspace() {
See [..]
[PACKAGING] li v0.0.1 ([CWD]/li)
[UPLOADING] li v0.0.1 ([CWD]/li)
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -2039,6 +2066,7 @@ fn in_package_workspace_with_members_with_features_old() {
See [..]
[PACKAGING] li v0.0.1 ([CWD]/li)
[UPLOADING] li v0.0.1 ([CWD]/li)
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -2129,6 +2157,7 @@ fn in_virtual_workspace_with_p() {
See [..]
[PACKAGING] li v0.0.1 ([CWD]/li)
[UPLOADING] li v0.0.1 ([CWD]/li)
[UPDATING] crates.io index
",
)
.run();
Expand Down Expand Up @@ -2313,7 +2342,9 @@ fn http_api_not_noop() {
[VERIFYING] foo v0.0.1 ([CWD])
[..]
[..]
[UPLOADING] foo v0.0.1 ([CWD])",
[UPLOADING] foo v0.0.1 ([CWD])
[UPDATING] [..]
",
)
.run();

Expand Down Expand Up @@ -2380,17 +2411,10 @@ fn wait_for_publish() {
"#,
)
.file("src/lib.rs", "")
.file(
".cargo/config",
"
[publish]
timeout = 60
",
)
.build();

p.cargo("publish --no-verify -Z sparse-registry -Z publish-timeout")
.masquerade_as_nightly_cargo(&["sparse-registry", "publish-timeout"])
p.cargo("publish --no-verify -Z sparse-registry")
.masquerade_as_nightly_cargo(&["sparse-registry"])
.replace_crates_io(registry.index_url())
.with_status(0)
.with_stderr(
Expand Down Expand Up @@ -2477,17 +2501,10 @@ fn wait_for_publish_underscore() {
"#,
)
.file("src/lib.rs", "")
.file(
".cargo/config",
"
[publish]
timeout = 60
",
)
.build();

p.cargo("publish --no-verify -Z sparse-registry -Z publish-timeout")
.masquerade_as_nightly_cargo(&["sparse-registry", "publish-timeout"])
p.cargo("publish --no-verify -Z sparse-registry")
.masquerade_as_nightly_cargo(&["sparse-registry"])
.replace_crates_io(registry.index_url())
.with_status(0)
.with_stderr(
Expand Down
7 changes: 5 additions & 2 deletions tests/testsuite/source_replacement.rs
Expand Up @@ -206,7 +206,8 @@ fn publish_with_replacement() {
p.cargo("publish --registry crates-io")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"[UPDATING] crates.io index
"\
[UPDATING] crates.io index
[WARNING] manifest has no documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
[PACKAGING] foo v0.0.1 ([..])
Expand All @@ -217,7 +218,9 @@ See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for
[COMPILING] bar v1.0.0
[COMPILING] foo v0.0.1 ([..]foo-0.0.1)
[FINISHED] dev [..]
[UPLOADING] foo v0.0.1 ([..])",
[UPLOADING] foo v0.0.1 ([..])
[UPDATING] crates.io index
",
)
.run();
}
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/weak_dep_features.rs
Expand Up @@ -569,6 +569,7 @@ fn publish() {
[COMPILING] foo v0.1.0 [..]
[FINISHED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
)
.run();
Expand Down

0 comments on commit f2fc5ca

Please sign in to comment.