Move fetch strategy retry deeper into internals

This avoids redoing the fast-path and such and only actually retries the fetch itself

# Conflicts:
#	crates/uv-git/src/git.rs

# Conflicts:
#	crates/uv-git/src/git.rs
This commit is contained in:
Zanie 2024-02-21 12:56:24 -06:00
parent 479864a942
commit 8b46579b9d
2 changed files with 70 additions and 31 deletions

View File

@ -44,6 +44,7 @@ pub(crate) enum GitReference {
} }
/// Strategy when fetching refspecs for a [`GitReference`] /// Strategy when fetching refspecs for a [`GitReference`]
#[derive(Debug, Clone, Copy)]
enum RefspecStrategy { enum RefspecStrategy {
// All refspecs should be fetched, if any fail then the fetch will fail // All refspecs should be fetched, if any fail then the fetch will fail
All, All,
@ -151,7 +152,7 @@ impl GitRemote {
db: Option<GitDatabase>, db: Option<GitDatabase>,
reference: &GitReference, reference: &GitReference,
locked_rev: Option<git2::Oid>, locked_rev: Option<git2::Oid>,
strategy: FetchStrategy, strategy: Option<FetchStrategy>,
client: &Client, client: &Client,
) -> Result<(GitDatabase, git2::Oid)> { ) -> Result<(GitDatabase, git2::Oid)> {
let locked_ref = locked_rev.map(|oid| GitReference::FullCommit(oid.to_string())); let locked_ref = locked_rev.map(|oid| GitReference::FullCommit(oid.to_string()));
@ -211,7 +212,7 @@ impl GitDatabase {
&self, &self,
rev: git2::Oid, rev: git2::Oid,
destination: &Path, destination: &Path,
strategy: FetchStrategy, strategy: Option<FetchStrategy>,
client: &Client, client: &Client,
) -> Result<GitCheckout<'_>> { ) -> Result<GitCheckout<'_>> {
// If the existing checkout exists, and it is fresh, use it. // If the existing checkout exists, and it is fresh, use it.
@ -437,7 +438,7 @@ impl<'a> GitCheckout<'a> {
/// Submodules set to `none` won't be fetched. /// Submodules set to `none` won't be fetched.
/// ///
/// [^1]: <https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-none> /// [^1]: <https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-none>
fn update_submodules(&self, strategy: FetchStrategy, client: &Client) -> Result<()> { fn update_submodules(&self, strategy: Option<FetchStrategy>, client: &Client) -> Result<()> {
/// Like `Cow`, but without a requirement on `Clone`. /// Like `Cow`, but without a requirement on `Clone`.
enum Repo<'a> { enum Repo<'a> {
Borrowed(&'a git2::Repository), Borrowed(&'a git2::Repository),
@ -891,15 +892,17 @@ pub(crate) fn with_fetch_options(
/// * Dispatches `git fetch` using libgit2 or git CLI. /// * Dispatches `git fetch` using libgit2 or git CLI.
/// ///
/// The `remote_url` argument is the git remote URL where we want to fetch from. /// The `remote_url` argument is the git remote URL where we want to fetch from.
///
/// Returns the successful fetch strategy used, if any.
pub(crate) fn fetch( pub(crate) fn fetch(
repo: &mut git2::Repository, repo: &mut git2::Repository,
remote_url: &str, remote_url: &str,
reference: &GitReference, reference: &GitReference,
strategy: FetchStrategy, strategy: Option<FetchStrategy>,
client: &Client, client: &Client,
) -> Result<()> { ) -> Result<Option<FetchStrategy>> {
let oid_to_fetch = match github_fast_path(repo, remote_url, reference, client) { let oid_to_fetch = match github_fast_path(repo, remote_url, reference, client) {
Ok(FastPathRev::UpToDate) => return Ok(()), Ok(FastPathRev::UpToDate) => return Ok(None),
Ok(FastPathRev::NeedsFetch(rev)) => Some(rev), Ok(FastPathRev::NeedsFetch(rev)) => Some(rev),
Ok(FastPathRev::Indeterminate) => None, Ok(FastPathRev::Indeterminate) => None,
Err(e) => { Err(e) => {
@ -977,11 +980,53 @@ pub(crate) fn fetch(
} }
} }
if let Some(strategy) = strategy {
fetch_with_strategy(
repo,
remote_url,
strategy,
refspecs.as_slice(),
refspec_strategy,
tags,
)
} else {
fetch_with_strategy(
repo,
remote_url,
FetchStrategy::Libgit2,
refspecs.as_slice(),
refspec_strategy,
tags,
)
.or_else(|_| {
fetch_with_strategy(
repo,
remote_url,
FetchStrategy::Cli,
refspecs.as_slice(),
refspec_strategy,
tags,
)
})
}
}
fn fetch_with_strategy(
repo: &mut git2::Repository,
remote_url: &str,
strategy: FetchStrategy,
refspecs: &[String],
refspec_strategy: RefspecStrategy,
tags: bool,
) -> Result<Option<FetchStrategy>> {
debug!("Performing a Git fetch for: {remote_url}"); debug!("Performing a Git fetch for: {remote_url}");
match strategy { match strategy {
FetchStrategy::Cli => { FetchStrategy::Cli => {
match refspec_strategy { match refspec_strategy {
RefspecStrategy::All => fetch_with_cli(repo, remote_url, refspecs.as_slice(), tags), RefspecStrategy::All => {
fetch_with_cli(repo, remote_url, refspecs, tags)?;
Ok(Some(strategy))
}
RefspecStrategy::First => { RefspecStrategy::First => {
let num_refspecs = refspecs.len(); let num_refspecs = refspecs.len();
@ -991,7 +1036,7 @@ pub(crate) fn fetch(
.map(|refspec| { .map(|refspec| {
( (
refspec.clone(), refspec.clone(),
fetch_with_cli(repo, remote_url, &[refspec], tags), fetch_with_cli(repo, remote_url, &[refspec.clone()], tags),
) )
}) })
// Stop after the first success // Stop after the first success
@ -1007,7 +1052,7 @@ pub(crate) fn fetch(
} }
Err(anyhow!("failed to fetch all refspecs")) Err(anyhow!("failed to fetch all refspecs"))
} else { } else {
Ok(()) Ok(Some(strategy))
} }
} }
} }
@ -1060,7 +1105,8 @@ pub(crate) fn fetch(
return Err(err.into()); return Err(err.into());
} }
Ok(()) Ok(())
}) })?;
Ok(Some(strategy))
} }
} }
} }

View File

@ -19,6 +19,8 @@ pub struct GitSource {
git: GitUrl, git: GitUrl,
/// The HTTP client to use for fetching. /// The HTTP client to use for fetching.
client: Client, client: Client,
/// The fetch strategy to use when cloning.
strategy: Option<FetchStrategy>,
/// The path to the Git source database. /// The path to the Git source database.
cache: PathBuf, cache: PathBuf,
/// The reporter to use for this source. /// The reporter to use for this source.
@ -31,6 +33,7 @@ impl GitSource {
Self { Self {
git, git,
client: Client::new(), client: Client::new(),
strategy: None,
cache: cache.into(), cache: cache.into(),
reporter: None, reporter: None,
} }
@ -46,25 +49,7 @@ impl GitSource {
} }
/// Fetch the underlying Git repository at the given revision. /// Fetch the underlying Git repository at the given revision.
///
/// Uses the Libgit2 backend first, then falls back to the CLI which supports
/// more authentication schemes.
pub fn fetch(self) -> Result<Fetch> { pub fn fetch(self) -> Result<Fetch> {
self.fetch_with_strategy(FetchStrategy::Libgit2)
.or_else(|_| {
debug!("fetch with libgit2 failed, trying git cli");
self.fetch_with_strategy(FetchStrategy::Cli)
})
.map(|(actual_rev, checkout_path)| Fetch {
git: self.git.with_precise(actual_rev),
path: checkout_path,
})
}
/// Fetch the underlying Git repository at the given revision.
///
/// Callers **should** update `self.git.with_precise` with the given SHA.
fn fetch_with_strategy(&self, strategy: FetchStrategy) -> Result<(GitSha, PathBuf)> {
// The path to the repo, within the Git database. // The path to the repo, within the Git database.
let ident = digest(&RepositoryUrl::new(&self.git.repository)); let ident = digest(&RepositoryUrl::new(&self.git.repository));
let db_path = self.cache.join("db").join(&ident); let db_path = self.cache.join("db").join(&ident);
@ -92,7 +77,7 @@ impl GitSource {
db, db,
&self.git.reference, &self.git.reference,
locked_rev.map(git2::Oid::from), locked_rev.map(git2::Oid::from),
strategy, self.strategy,
&self.client, &self.client,
)?; )?;
@ -112,7 +97,12 @@ impl GitSource {
.join("checkouts") .join("checkouts")
.join(&ident) .join(&ident)
.join(short_id.as_str()); .join(short_id.as_str());
db.copy_to(actual_rev.into(), &checkout_path, strategy, &self.client)?; db.copy_to(
actual_rev.into(),
&checkout_path,
self.strategy,
&self.client,
)?;
// Report the checkout operation to the reporter. // Report the checkout operation to the reporter.
if let Some(task) = task { if let Some(task) = task {
@ -121,7 +111,10 @@ impl GitSource {
} }
} }
Ok((actual_rev, checkout_path)) Ok(Fetch {
git: self.git.with_precise(actual_rev),
path: checkout_path,
})
} }
} }