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.

Fixes #9507
  • Loading branch information
epage committed Sep 13, 2022
1 parent e74d32c commit 795236d
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 41 deletions.
1 change: 1 addition & 0 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 &macros {
Expand Down
60 changes: 60 additions & 0 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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
Expand Down
52 changes: 11 additions & 41 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2051,7 +2051,7 @@ error: package ID specification `bar` did not match any packages
}

#[cargo_test]
fn delayed_publish_errors() {
fn wait_for_publish() {
// Counter for number of tries before the package is "published"
let arc: Arc<Mutex<u32>> = Arc::new(Mutex::new(0));
let arc2 = arc.clone();
Expand Down Expand Up @@ -2107,13 +2107,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()
Expand All @@ -2131,23 +2133,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)
Expand All @@ -2158,7 +2143,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<Mutex<u32>> = Arc::new(Mutex::new(0));
let arc2 = arc.clone();
Expand Down Expand Up @@ -2214,13 +2199,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()
Expand All @@ -2238,24 +2226,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)
Expand Down

0 comments on commit 795236d

Please sign in to comment.