Skip to content

Commit

Permalink
fix(publish): Check remote git registry more than once post-publish
Browse files Browse the repository at this point in the history
With `publish-timeout` on remote git registries, we were checking once
and never checking again.  There were 3 layers of guards preventing the
cache from being updating
- `needs_update`, handled via `invalidate_cache`
- `config.updated_sources()`,. handled by removing from the HashSet
- `updated`, inaccessible

This change collapses `updated` into `config.updated_sources()`,
allowing the cache to be updated

Tested by publishing `epage-publish-test`.  After about 7 registry
updates, it succeded.  Before, it just hit the timeout of 60s.  No tests
are added as we don't have the plumbing setup to be able to control when
the test remote git registry publishes.  All of our tests focus on the
remote http registry.

Fixes #11253
  • Loading branch information
epage committed Oct 18, 2022
1 parent 3ff0443 commit dd69674
Showing 1 changed file with 20 additions and 15 deletions.
35 changes: 20 additions & 15 deletions src/cargo/sources/registry/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub struct RemoteRegistry<'cfg> {
head: Cell<Option<git2::Oid>>,
current_sha: Cell<Option<InternedString>>,
needs_update: bool, // Does this registry need to be updated?
updated: bool, // Has this registry been updated this session?
}

impl<'cfg> RemoteRegistry<'cfg> {
Expand All @@ -49,7 +48,6 @@ impl<'cfg> RemoteRegistry<'cfg> {
head: Cell::new(None),
current_sha: Cell::new(None),
needs_update: false,
updated: false,
}
}

Expand Down Expand Up @@ -141,6 +139,14 @@ impl<'cfg> RemoteRegistry<'cfg> {
self.current_sha.set(Some(sha));
Some(sha)
}

fn is_updated(&self) -> bool {
self.config.updated_sources().contains(&self.source_id)
}

fn mark_updated(&self) {
self.config.updated_sources().insert(self.source_id);
}
}

const LAST_UPDATED_FILE: &str = ".last-updated";
Expand Down Expand Up @@ -214,7 +220,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {

match load_helper(&self, path, index_version) {
Ok(result) => Poll::Ready(Ok(result)),
Err(_) if !self.updated => {
Err(_) if !self.is_updated() => {
// If git returns an error and we haven't updated the repo, return
// pending to allow an update to try again.
self.needs_update = true;
Expand Down Expand Up @@ -250,19 +256,20 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
return Ok(());
}

self.updated = true;
self.needs_update = false;

if self.config.offline() {
// Make sure the index is only updated once per session since it is an
// expensive operation. This generally only happens when the resolver
// is run multiple times, such as during `cargo publish`.
if self.is_updated() {
return Ok(());
}
if self.config.cli_unstable().no_index_update {
self.mark_updated();

if self.config.offline() {
return Ok(());
}
// Make sure the index is only updated once per session since it is an
// expensive operation. This generally only happens when the resolver
// is run multiple times, such as during `cargo publish`.
if self.config.updated_sources().contains(&self.source_id) {
if self.config.cli_unstable().no_index_update {
return Ok(());
}

Expand Down Expand Up @@ -291,7 +298,6 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
let repo = self.repo.borrow_mut().unwrap();
git::fetch(repo, url.as_str(), &self.index_git_ref, self.config)
.with_context(|| format!("failed to fetch `{}`", url))?;
self.config.updated_sources().insert(self.source_id);

// Create a dummy file to record the mtime for when we updated the
// index.
Expand All @@ -301,13 +307,12 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
}

fn invalidate_cache(&mut self) {
if !self.updated {
self.needs_update = true;
}
// To fully invalidate, undo `mark_updated`s work
self.needs_update = true;
}

fn is_updated(&self) -> bool {
self.updated
self.is_updated()
}

fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
Expand Down

0 comments on commit dd69674

Please sign in to comment.