From 8fadfe7f338825282dda25ce6c625862bfd2bd43 Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Thu, 8 Sep 2022 09:40:40 -0600 Subject: [PATCH 1/3] test(registry): Allow `custom_responders` to call normal responders --- tests/testsuite/publish.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index a694d3fa620..5fc8dbc5ef3 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -5,6 +5,7 @@ use cargo_test_support::paths; use cargo_test_support::registry::{self, Package, Response}; use cargo_test_support::{basic_manifest, no_such_file_err_msg, project, publish}; use std::fs; +use std::sync::{Arc, Mutex}; const CLEAN_FOO_JSON: &str = r#" { From 66b62d274567921f3164f510475520a576110389 Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Thu, 8 Sep 2022 09:41:54 -0600 Subject: [PATCH 2/3] test(publish): Demonstrate the impact of non-blocking publish --- tests/testsuite/publish.rs | 208 +++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 5fc8dbc5ef3..28f48ba7e5c 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -2261,3 +2261,211 @@ fn http_api_not_noop() { p.cargo("build").run(); } + +#[cargo_test] +fn delayed_publish_errors() { + // Counter for number of tries before the package is "published" + let arc: Arc> = Arc::new(Mutex::new(0)); + let arc2 = arc.clone(); + + // Registry returns an invalid response. + let registry = registry::RegistryBuilder::new() + .http_index() + .http_api() + .add_responder("/index/de/la/delay", move |req, server| { + let mut lock = arc.lock().unwrap(); + *lock += 1; + // if the package name contains _ or - + if *lock <= 1 { + server.not_found(req) + } else { + server.index(req) + } + }) + .build(); + + // The sparse-registry test server does not know how to publish on its own. + // So let us call publish for it. + Package::new("delay", "0.0.1") + .file("src/lib.rs", "") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "delay" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + + "#, + ) + .file("src/lib.rs", "") + .build(); + + 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( + "\ +[UPDATING] `crates-io` index +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] delay v0.0.1 ([CWD]) +[UPLOADING] delay v0.0.1 ([CWD]) +", + ) + .run(); + + // Check nothing has touched the responder + let lock = arc2.lock().unwrap(); + assert_eq!(*lock, 0); + drop(lock); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + [dependencies] + delay = "0.0.1" + "#, + ) + .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) + .run(); +} + +/// A separate test is needed for package names with - or _ as they hit +/// 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() { + // Counter for number of tries before the package is "published" + let arc: Arc> = Arc::new(Mutex::new(0)); + let arc2 = arc.clone(); + + // Registry returns an invalid response. + let registry = registry::RegistryBuilder::new() + .http_index() + .http_api() + .add_responder("/index/de/la/delay_with_underscore", move |req, server| { + let mut lock = arc.lock().unwrap(); + *lock += 1; + // package names with - or _ hit the responder twice per cargo invocation + if *lock <= 2 { + server.not_found(req) + } else { + server.index(req) + } + }) + .build(); + + // The sparse-registry test server does not know how to publish on its own. + // So let us call publish for it. + Package::new("delay_with_underscore", "0.0.1") + .file("src/lib.rs", "") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "delay_with_underscore" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + + "#, + ) + .file("src/lib.rs", "") + .build(); + + 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( + "\ +[UPDATING] `crates-io` index +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] delay_with_underscore v0.0.1 ([CWD]) +[UPLOADING] delay_with_underscore v0.0.1 ([CWD]) +", + ) + .run(); + + // Check nothing has touched the responder + let lock = arc2.lock().unwrap(); + assert_eq!(*lock, 0); + drop(lock); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + [dependencies] + delay_with_underscore = "0.0.1" + "#, + ) + .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) + .run(); +} From 04d836fa71c63f2344e2c18cc67f24142bc11d57 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Jul 2022 11:48:29 -0500 Subject: [PATCH 3/3] feat(publish): Support 'publish.timeout' config behind '-Zpublish-timeout' 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 This introduces unstable support for blocking by setting `publish.timeout` to non-zero value. A step towards #9507 --- crates/cargo-test-support/src/compare.rs | 1 + src/cargo/core/features.rs | 2 + src/cargo/ops/registry.rs | 81 +++++++++++++++++ src/doc/src/reference/unstable.md | 18 ++++ tests/testsuite/publish.rs | 110 ++++++++++++++--------- 5 files changed, 170 insertions(+), 42 deletions(-) diff --git a/crates/cargo-test-support/src/compare.rs b/crates/cargo-test-support/src/compare.rs index 6d0e299f568..9d33904d430 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/core/features.rs b/src/cargo/core/features.rs index 020485355ce..c2046ec3db5 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -686,6 +686,7 @@ unstable_cli_options!( rustdoc_map: bool = ("Allow passing external documentation mappings to rustdoc"), separate_nightlies: bool = (HIDDEN), terminal_width: Option> = ("Provide a terminal width to rustc for error truncation"), + publish_timeout: bool = ("Enable the `publish.timeout` key in .cargo/config.toml file"), unstable_options: bool = ("Allow the usage of unstable options"), // TODO(wcrichto): move scrape example configuration into Cargo.toml before stabilization // See: https://github.com/rust-lang/cargo/pull/9525#discussion_r728470927 @@ -930,6 +931,7 @@ impl CliUnstable { "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?, "host-config" => self.host_config = parse_empty(k, v)?, "target-applies-to-host" => self.target_applies_to_host = parse_empty(k, v)?, + "publish-timeout" => self.publish_timeout = parse_empty(k, v)?, "features" => { // `-Z features` has been stabilized since 1.51, // but `-Z features=compare` is still allowed for convenience diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 5a225c6c6f9..3606e708d89 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,19 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { reg_ids.original, opts.dry_run, )?; + if !opts.dry_run { + const DEFAULT_TIMEOUT: u64 = 0; + let timeout = if opts.config.cli_unstable().publish_timeout { + let timeout: Option = opts.config.get("publish.timeout")?; + timeout.unwrap_or(DEFAULT_TIMEOUT) + } else { + DEFAULT_TIMEOUT + }; + if 0 < timeout { + let timeout = std::time::Duration::from_secs(timeout); + wait_for_publish(opts.config, reg_ids.original, pkg, timeout)?; + } + } Ok(()) } @@ -374,6 +389,72 @@ 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 = SourceConfigMap::empty(config)?.load(registry_src, &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()?; + // Force re-fetching the source + // + // As pulling from a git source is expensive, we track when we've done it within the + // process to only do it once, but we are one of the rare cases that needs to do it + // multiple times + config + .updated_sources() + .remove(&source.replaced_source_id()); + 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 {} (ctrl-c to wait asynchronously)", + 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/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index f97b1f93c50..9f75a971f49 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -99,6 +99,7 @@ Each new feature described below should explain how to use it. * [credential-process](#credential-process) — Adds support for fetching registry tokens from an external authentication program. * [`cargo logout`](#cargo-logout) — Adds the `logout` command to remove the currently saved registry token. * [sparse-registry](#sparse-registry) — Adds support for fetching from static-file HTTP registries (`sparse+`) + * [publish-timeout](#publish-timeout) — Controls the timeout between uploading the crate and being available in the index ### allow-features @@ -841,6 +842,23 @@ crates, which can save significant time and bandwidth. The format of the sparse index is identical to a checkout of a git-based index. +### publish-timeout +* Tracking Issue: [11222](https://github.com/rust-lang/cargo/issues/11222) + +The `publish.timeout` key in a config file can be used to control how long +`cargo publish` waits between posting a package to the registry and it being +available in the local index. + +A timeout of `0` prevents any checks from occurring. + +It requires the `-Zpublish-timeout` command-line options to be set. + +```toml +# config.toml +[publish] +timeout = 300 # in seconds +``` + ### credential-process * Tracking Issue: [#8933](https://github.com/rust-lang/cargo/issues/8933) * RFC: [#2730](https://github.com/rust-lang/rfcs/pull/2730) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 28f48ba7e5c..d0851f62407 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -2263,7 +2263,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(); @@ -2304,10 +2304,17 @@ fn delayed_publish_errors() { "#, ) .file("src/lib.rs", "") + .file( + ".cargo/config", + " + [publish] + timeout = 60 + ", + ) .build(); - p.cargo("publish --no-verify -Z sparse-registry") - .masquerade_as_nightly_cargo(&["sparse-registry"]) + p.cargo("publish --no-verify -Z sparse-registry -Z publish-timeout") + .masquerade_as_nightly_cargo(&["sparse-registry", "publish-timeout"]) .replace_crates_io(registry.index_url()) .with_status(0) .with_stderr( @@ -2317,13 +2324,15 @@ fn delayed_publish_errors() { See [..] [PACKAGING] delay v0.0.1 ([CWD]) [UPLOADING] delay v0.0.1 ([CWD]) +[UPDATING] `crates-io` index +[WAITING] on `delay` to propagate to `crates-io` index (which is replacing registry `crates-io`) (ctrl-c to wait asynchronously) ", ) .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() @@ -2341,23 +2350,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) @@ -2368,7 +2360,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(); @@ -2409,10 +2401,17 @@ fn delayed_publish_errors_underscore() { "#, ) .file("src/lib.rs", "") + .file( + ".cargo/config", + " + [publish] + timeout = 60 + ", + ) .build(); - p.cargo("publish --no-verify -Z sparse-registry") - .masquerade_as_nightly_cargo(&["sparse-registry"]) + p.cargo("publish --no-verify -Z sparse-registry -Z publish-timeout") + .masquerade_as_nightly_cargo(&["sparse-registry", "publish-timeout"]) .replace_crates_io(registry.index_url()) .with_status(0) .with_stderr( @@ -2422,13 +2421,16 @@ fn delayed_publish_errors_underscore() { See [..] [PACKAGING] delay_with_underscore v0.0.1 ([CWD]) [UPLOADING] delay_with_underscore v0.0.1 ([CWD]) +[UPDATING] `crates-io` index +[WAITING] on `delay_with_underscore` to propagate to `crates-io` index (which is replacing registry `crates-io`) (ctrl-c to wait asynchronously) ", ) .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() @@ -2448,24 +2450,48 @@ See [..] p.cargo("build -Z sparse-registry") .masquerade_as_nightly_cargo(&["sparse-registry"]) - .with_status(101) + .with_status(0) + .run(); +} + +#[cargo_test] +fn skip_wait_for_publish() { + // Intentionally using local registry so the crate never makes it to the index + let registry = registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + ".cargo/config", + " + [publish] + timeout = 0 + ", + ) + .build(); + + p.cargo("publish --no-verify -Zpublish-timeout") + .replace_crates_io(registry.index_url()) + .masquerade_as_nightly_cargo(&["publish-timeout"]) .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)` +[UPDATING] crates.io index +[WARNING] manifest has no documentation, [..] +See [..] +[PACKAGING] foo v0.0.1 ([CWD]) +[UPLOADING] foo v0.0.1 ([CWD]) ", ) .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) - .run(); }