From 77af68ea0878c3a9f632b5c3f7839f687fa17df7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Jul 2022 11:48:29 -0500 Subject: [PATCH] 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 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. Fixes #9507 --- crates/cargo-test-support/src/compare.rs | 1 + src/cargo/ops/registry.rs | 60 ++++++++++++++++++++++++ tests/testsuite/publish.rs | 52 +++++--------------- 3 files changed, 72 insertions(+), 41 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 6d0e299f5681..9d33904d4304 100644 --- a/crates/cargo-test-support/src/compare.rs +++ b/crates/cargo-test-support/src/compare.rs @@ -197,6 +197,7 @@ fn substitute_macros(input: &str) -> String { ("[MIGRATING]", " Migrating"), ("[EXECUTABLE]", " Executable"), ("[SKIPPING]", " Skipping"), + ("[WAITING]", " Waiting"), ]; let mut result = input.to_owned(); for &(pat, subst) in ¯os { diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index e0aae14e0e89..175f9827fccd 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -18,9 +18,11 @@ use termcolor::Color::Green; use termcolor::ColorSpec; use crate::core::dependency::DepKind; +use crate::core::dependency::Dependency; use crate::core::manifest::ManifestMetadata; use crate::core::resolver::CliFeatures; use crate::core::source::Source; +use crate::core::QueryKind; use crate::core::{Package, SourceId, Workspace}; use crate::ops; use crate::ops::Packages; @@ -183,6 +185,10 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { reg_id, opts.dry_run, )?; + if !opts.dry_run { + let timeout = std::time::Duration::from_secs(300); + wait_for_publish(opts.config, reg_id, pkg, timeout)?; + } Ok(()) } @@ -374,6 +380,60 @@ fn transmit( Ok(()) } +fn wait_for_publish( + config: &Config, + registry_src: SourceId, + pkg: &Package, + timeout: std::time::Duration, +) -> CargoResult<()> { + let version_req = format!("={}", pkg.version()); + let mut source = registry_src.load(config, &HashSet::new())?; + let source_description = source.describe(); + let query = Dependency::parse(pkg.name(), Some(&version_req), registry_src)?; + + let now = std::time::Instant::now(); + let sleep_time = std::time::Duration::from_secs(1); + let mut logged = false; + loop { + { + let _lock = config.acquire_package_cache_lock()?; + source.invalidate_cache(); + let summaries = loop { + // Exact to avoid returning all for path/git + match source.query_vec(&query, QueryKind::Exact) { + std::task::Poll::Ready(res) => { + break res?; + } + std::task::Poll::Pending => source.block_until_ready()?, + } + }; + if !summaries.is_empty() { + break; + } + } + + if timeout < now.elapsed() { + config.shell().warn(format!( + "timed out waiting for `{}` to be in {}", + pkg.name(), + source_description + ))?; + break; + } + + if !logged { + config.shell().status( + "Waiting", + format!("on `{}` to propagate to {}", pkg.name(), source_description), + )?; + logged = true; + } + std::thread::sleep(sleep_time); + } + + Ok(()) +} + /// Returns the index and token from the config file for the given registry. /// /// `registry` is typically the registry specified on the command-line. If diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index cb879ffffd1a..3be111be8fca 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -2093,7 +2093,7 @@ fn http_api_not_noop() { } #[cargo_test] -fn delayed_publish_errors() { +fn wait_for_publish() { // Counter for number of tries before the package is "published" let arc: Arc> = Arc::new(Mutex::new(0)); let arc2 = arc.clone(); @@ -2149,13 +2149,15 @@ Use the --token command-line flag to remove this warning. See [..] [PACKAGING] delay v0.0.1 ([CWD]) [UPLOADING] delay v0.0.1 ([CWD]) +[UPDATING] `dummy-registry` index +[WAITING] on `delay` to propagate to `dummy-registry` index ", ) .run(); - // Check nothing has touched the responder + // Verify the responder has been pinged let lock = arc2.lock().unwrap(); - assert_eq!(*lock, 0); + assert_eq!(*lock, 2); drop(lock); let p = project() @@ -2173,23 +2175,6 @@ See [..] .file("src/main.rs", "fn main() {}") .build(); - p.cargo("build -Z sparse-registry") - .masquerade_as_nightly_cargo(&["sparse-registry"]) - .with_status(101) - .with_stderr( - "\ -[UPDATING] [..] -[ERROR] no matching package named `delay` found -location searched: registry `crates-io` -required by package `foo v0.0.1 ([..]/foo)` -", - ) - .run(); - - let lock = arc2.lock().unwrap(); - assert_eq!(*lock, 1); - drop(lock); - p.cargo("build -Z sparse-registry") .masquerade_as_nightly_cargo(&["sparse-registry"]) .with_status(0) @@ -2200,7 +2185,7 @@ required by package `foo v0.0.1 ([..]/foo)` /// the responder twice per cargo invocation. If that ever gets changed /// this test will need to be changed accordingly. #[cargo_test] -fn delayed_publish_errors_underscore() { +fn wait_for_publish_underscore() { // Counter for number of tries before the package is "published" let arc: Arc> = Arc::new(Mutex::new(0)); let arc2 = arc.clone(); @@ -2256,13 +2241,16 @@ Use the --token command-line flag to remove this warning. See [..] [PACKAGING] delay_with_underscore v0.0.1 ([CWD]) [UPLOADING] delay_with_underscore v0.0.1 ([CWD]) +[UPDATING] `dummy-registry` index +[WAITING] on `delay_with_underscore` to propagate to `dummy-registry` index ", ) .run(); - // Check nothing has touched the responder + // Verify the repsponder has been pinged let lock = arc2.lock().unwrap(); - assert_eq!(*lock, 0); + // NOTE: package names with - or _ hit the responder twice per cargo invocation + assert_eq!(*lock, 3); drop(lock); let p = project() @@ -2280,24 +2268,6 @@ See [..] .file("src/main.rs", "fn main() {}") .build(); - p.cargo("build -Z sparse-registry") - .masquerade_as_nightly_cargo(&["sparse-registry"]) - .with_status(101) - .with_stderr( - "\ -[UPDATING] [..] -[ERROR] no matching package named `delay_with_underscore` found -location searched: registry `crates-io` -required by package `foo v0.0.1 ([..]/foo)` -", - ) - .run(); - - let lock = arc2.lock().unwrap(); - // package names with - or _ hit the responder twice per cargo invocation - assert_eq!(*lock, 2); - drop(lock); - p.cargo("build -Z sparse-registry") .masquerade_as_nightly_cargo(&["sparse-registry"]) .with_status(0)