diff --git a/crates/uv-git/src/git.rs b/crates/uv-git/src/git.rs index faddeefa17fb..cd37c9fc2a0c 100644 --- a/crates/uv-git/src/git.rs +++ b/crates/uv-git/src/git.rs @@ -25,20 +25,14 @@ const CHECKOUT_READY_LOCK: &str = ".ok"; /// A reference to commit or commit-ish. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum GitReference { - /// From a branch. - #[allow(unused)] - Branch(String), - /// From a tag. - #[allow(unused)] - Tag(String), /// From a reference that's ambiguously a branch or tag. BranchOrTag(String), + /// From a reference that's ambiguously a short commit, a branch, or a tag. + BranchOrTagOrCommit(String), + /// From a named reference, like `refs/pull/493/head`. + NamedRef(String), /// From a specific revision, using a full 40-character commit hash. FullCommit(String), - /// From a truncated revision. - ShortCommit(String), - /// From a named reference, like `refs/pull/493/head`. - Ref(String), /// The default branch of the repository, the reference named `HEAD`. DefaultBranch, } @@ -52,37 +46,28 @@ enum RefspecStrategy { } impl GitReference { + /// Creates a [`GitReference`] from a revision string. pub(crate) fn from_rev(rev: &str) -> Self { if rev.starts_with("refs/") { - Self::Ref(rev.to_owned()) + Self::NamedRef(rev.to_owned()) } else if looks_like_commit_hash(rev) { if rev.len() == 40 { Self::FullCommit(rev.to_owned()) } else { - Self::ShortCommit(rev.to_owned()) + Self::BranchOrTagOrCommit(rev.to_owned()) } } else { Self::BranchOrTag(rev.to_owned()) } } - pub fn precise(&self) -> Option<&str> { - match self { - Self::FullCommit(rev) => Some(rev), - Self::ShortCommit(rev) => Some(rev), - _ => None, - } - } - /// Converts the [`GitReference`] to a `str`. pub fn as_str(&self) -> Option<&str> { match self { - Self::Branch(rev) => Some(rev), - Self::Tag(rev) => Some(rev), Self::BranchOrTag(rev) => Some(rev), Self::FullCommit(rev) => Some(rev), - Self::ShortCommit(rev) => Some(rev), - Self::Ref(rev) => Some(rev), + Self::BranchOrTagOrCommit(rev) => Some(rev), + Self::NamedRef(rev) => Some(rev), Self::DefaultBranch => None, } } @@ -90,12 +75,10 @@ impl GitReference { /// Converts the [`GitReference`] to a `str` that can be used as a revision. pub(crate) fn as_rev(&self) -> &str { match self { - Self::Branch(rev) - | Self::Tag(rev) - | Self::BranchOrTag(rev) + Self::BranchOrTag(rev) | Self::FullCommit(rev) - | Self::ShortCommit(rev) - | Self::Ref(rev) => rev, + | Self::BranchOrTagOrCommit(rev) + | Self::NamedRef(rev) => rev, Self::DefaultBranch => "HEAD", } } @@ -103,12 +86,10 @@ impl GitReference { /// Returns the kind of this reference. pub(crate) fn kind_str(&self) -> &str { match self { - Self::Branch(_) => "branch", - Self::Tag(_) => "tag", Self::BranchOrTag(_) => "branch or tag", Self::FullCommit(_) => "commit", - Self::ShortCommit(_) => "short commit", - Self::Ref(_) => "ref", + Self::BranchOrTagOrCommit(_) => "branch, tag, or commit", + Self::NamedRef(_) => "ref", Self::DefaultBranch => "default branch", } } @@ -281,43 +262,53 @@ impl GitReference { pub(crate) fn resolve(&self, repo: &git2::Repository) -> Result { let refkind = self.kind_str(); let id = match self { - // Note that we resolve the named tag here in sync with where it's - // fetched into via `fetch` below. - Self::Tag(s) => (|| -> Result { - let refname = format!("refs/remotes/origin/tags/{s}"); - let id = repo.refname_to_id(&refname)?; - let obj = repo.find_object(id, None)?; - let obj = obj.peel(ObjectType::Commit)?; - Ok(obj.id()) - })() - .with_context(|| format!("failed to find {refkind} `{s}`"))?, - - // Resolve the remote name since that's all we're configuring in - // `fetch` below. - Self::Branch(s) => { + // Attempt to resolve the branch, then the tag. + Self::BranchOrTag(s) => { let name = format!("origin/{s}"); - let b = repo - .find_branch(&name, git2::BranchType::Remote) - .with_context(|| format!("failed to find {refkind} `{s}`"))?; - b.get() - .target() - .ok_or_else(|| anyhow::format_err!("{refkind} `{s}` did not have a target"))? + + // Resolve the remote name since that's all we're configuring in + // `fetch` below. + repo.find_branch(&name, git2::BranchType::Remote) + .ok() + .and_then(|b| b.get().target()) + .or_else(|| { + // Note that we resolve the named tag here in sync with where it's + // fetched into via `fetch` below. + let refname = format!("refs/remotes/origin/tags/{s}"); + let id = repo.refname_to_id(&refname).ok()?; + let obj = repo.find_object(id, None).ok()?; + let obj = obj.peel(ObjectType::Commit).ok()?; + Some(obj.id()) + }) + .ok_or_else(|| anyhow::format_err!("failed to find {refkind} `{s}`"))? } - // Attempt to resolve the branch, then the tag. - Self::BranchOrTag(s) => { + // Attempt to resolve the branch, then the tag, then the commit. + Self::BranchOrTagOrCommit(s) => { let name = format!("origin/{s}"); + // Resolve the remote name since that's all we're configuring in + // `fetch` below. repo.find_branch(&name, git2::BranchType::Remote) .ok() .and_then(|b| b.get().target()) .or_else(|| { + // Note that we resolve the named tag here in sync with where it's + // fetched into via `fetch` below. let refname = format!("refs/remotes/origin/tags/{s}"); let id = repo.refname_to_id(&refname).ok()?; let obj = repo.find_object(id, None).ok()?; let obj = obj.peel(ObjectType::Commit).ok()?; Some(obj.id()) }) + .or_else(|| { + // Resolve the commit. + let obj = repo.revparse_single(s).ok()?; + match obj.as_tag() { + Some(tag) => Some(tag.target_id()), + None => Some(obj.id()), + } + }) .ok_or_else(|| anyhow::format_err!("failed to find {refkind} `{s}`"))? } @@ -328,7 +319,7 @@ impl GitReference { head.peel(ObjectType::Commit)?.id() } - Self::FullCommit(s) | Self::ShortCommit(s) | Self::Ref(s) => { + Self::FullCommit(s) | Self::NamedRef(s) => { let obj = repo.revparse_single(s)?; match obj.as_tag() { Some(tag) => tag.target_id(), @@ -958,14 +949,6 @@ pub(crate) fn fetch( match reference { // For branches and tags we can fetch simply one reference and copy it // locally, no need to fetch other branches/tags. - GitReference::Branch(branch) => { - refspecs.push(format!("+refs/heads/{branch}:refs/remotes/origin/{branch}")); - } - - GitReference::Tag(tag) => { - refspecs.push(format!("+refs/tags/{tag}:refs/remotes/origin/tags/{tag}")); - } - GitReference::BranchOrTag(branch_or_tag) => { refspecs.push(format!( "+refs/heads/{branch_or_tag}:refs/remotes/origin/{branch_or_tag}" @@ -976,11 +959,31 @@ pub(crate) fn fetch( refspec_strategy = RefspecStrategy::First; } + // For ambiguous references, we can fetch the exact commit (if known); otherwise, + // we fetch all branches and tags. + GitReference::BranchOrTagOrCommit(branch_or_tag_or_commit) => { + // The `oid_to_fetch` is the exact commit we want to fetch. But it could be the exact + // commit of a branch or tag. We should only fetch it directly if it's the exact commit + // of a short commit hash. + if let Some(oid_to_fetch) = + oid_to_fetch.filter(|oid| is_short_hash_of(branch_or_tag_or_commit, *oid)) + { + refspecs.push(format!("+{oid_to_fetch}:refs/commit/{oid_to_fetch}")); + } else { + // We don't know what the rev will point to. To handle this + // situation we fetch all branches and tags, and then we pray + // it's somewhere in there. + refspecs.push(String::from("+refs/heads/*:refs/remotes/origin/*")); + refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD")); + tags = true; + } + } + GitReference::DefaultBranch => { refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD")); } - GitReference::Ref(rev) => { + GitReference::NamedRef(rev) => { refspecs.push(format!("+{rev}:{rev}")); } @@ -997,19 +1000,6 @@ pub(crate) fn fetch( refspecs.push(format!("+{rev}:refs/remotes/origin/HEAD")); } } - - GitReference::ShortCommit(_) => { - if let Some(oid_to_fetch) = oid_to_fetch { - refspecs.push(format!("+{oid_to_fetch}:refs/commit/{oid_to_fetch}")); - } else { - // We don't know what the rev will point to. To handle this - // situation we fetch all branches and tags, and then we pray - // it's somewhere in there. - refspecs.push(String::from("+refs/heads/*:refs/remotes/origin/*")); - refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD")); - tags = true; - } - } } debug!("Performing a Git fetch for: {remote_url}"); @@ -1022,8 +1012,12 @@ pub(crate) fn fetch( let mut errors = refspecs .iter() .map_while(|refspec| { - let fetch_result = - fetch_with_cli(repo, remote_url, &[refspec.clone()], tags); + let fetch_result = fetch_with_cli( + repo, + remote_url, + std::slice::from_ref(refspec), + tags, + ); // Stop after the first success and log failures match fetch_result { @@ -1335,40 +1329,33 @@ fn github_fast_path( let local_object = reference.resolve(repo).ok(); let github_branch_name = match reference { - GitReference::Branch(branch) => branch, - GitReference::Tag(tag) => tag, GitReference::BranchOrTag(branch_or_tag) => branch_or_tag, GitReference::DefaultBranch => "HEAD", - GitReference::Ref(rev) => rev, - GitReference::FullCommit(rev) | GitReference::ShortCommit(rev) => { - if looks_like_commit_hash(rev) { - // `revparse_single` (used by `resolve`) is the only way to turn - // short hash -> long hash, but it also parses other things, - // like branch and tag names, which might coincidentally be - // valid hex. - // - // We only return early if `rev` is a prefix of the object found - // by `revparse_single`. Don't bother talking to GitHub in that - // case, since commit hashes are permanent. If a commit with the - // requested hash is already present in the local clone, its - // contents must be the same as what is on the server for that - // hash. - // - // If `rev` is not found locally by `revparse_single`, we'll - // need GitHub to resolve it and get a hash. If `rev` is found - // but is not a short hash of the found object, it's probably a - // branch and we also need to get a hash from GitHub, in case - // the branch has moved. - if let Some(local_object) = local_object { - if is_short_hash_of(rev, local_object) { - return Ok(FastPathRev::UpToDate); - } + GitReference::NamedRef(rev) => rev, + GitReference::FullCommit(rev) | GitReference::BranchOrTagOrCommit(rev) => { + // `revparse_single` (used by `resolve`) is the only way to turn + // short hash -> long hash, but it also parses other things, + // like branch and tag names, which might coincidentally be + // valid hex. + // + // We only return early if `rev` is a prefix of the object found + // by `revparse_single`. Don't bother talking to GitHub in that + // case, since commit hashes are permanent. If a commit with the + // requested hash is already present in the local clone, its + // contents must be the same as what is on the server for that + // hash. + // + // If `rev` is not found locally by `revparse_single`, we'll + // need GitHub to resolve it and get a hash. If `rev` is found + // but is not a short hash of the found object, it's probably a + // branch and we also need to get a hash from GitHub, in case + // the branch has moved. + if let Some(local_object) = local_object { + if is_short_hash_of(rev, local_object) { + return Ok(FastPathRev::UpToDate); } - rev - } else { - debug!("can't use github fast path with `rev = \"{}\"`", rev); - return Ok(FastPathRev::Indeterminate); } + rev } }; diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index 084571dd2155..7198fd071832 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -95,12 +95,10 @@ impl From for Url { } else { // Otherwise, add the branch or tag name. match git.reference { - GitReference::Branch(rev) - | GitReference::Tag(rev) - | GitReference::BranchOrTag(rev) - | GitReference::Ref(rev) + GitReference::BranchOrTag(rev) + | GitReference::NamedRef(rev) | GitReference::FullCommit(rev) - | GitReference::ShortCommit(rev) => { + | GitReference::BranchOrTagOrCommit(rev) => { url.set_path(&format!("{}@{}", url.path(), rev)); } GitReference::DefaultBranch => {} diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 4cca0a55d9c7..a1259b62ae59 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -1079,13 +1079,15 @@ fn compile_sdist_url_dependency() -> Result<()> { Ok(()) } -/// Resolve a specific Flask source distribution via a Git HTTPS dependency. +/// Resolve a specific source distribution via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); - requirements_in.write_str("flask @ git+https://github.com/pallets/flask.git")?; + requirements_in.write_str( + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage", + )?; // In addition to the standard filters, remove the `main` commit, which will change frequently. let filters: Vec<_> = [(r"@(\d|\w){40}", "@[COMMIT]")] @@ -1100,36 +1102,24 @@ fn compile_git_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - blinker==1.7.0 - # via flask - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@[COMMIT] - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@[COMMIT] ----- stderr ----- - Resolved 7 packages in [TIME] + Resolved 1 package in [TIME] "###); Ok(()) } -/// Resolve a specific Flask branch via a Git HTTPS dependency. +/// Resolve a specific branch via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_branch_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); - requirements_in.write_str("flask @ git+https://github.com/pallets/flask.git@1.0.x")?; + requirements_in.write_str( + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@test-branch", + )?; uv_snapshot!(context.compile() .arg("requirements.in"), @r###" @@ -1138,35 +1128,25 @@ fn compile_git_branch_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@d92b64aa275841b0c9aea3903aba72fbc4275d91 - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 ----- stderr ----- - Resolved 6 packages in [TIME] + Resolved 1 package in [TIME] "### ); Ok(()) } -/// Resolve a specific Flask tag via a Git HTTPS dependency. +/// Resolve a specific tag via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_tag_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); - requirements_in.write_str("flask @ git+https://github.com/pallets/flask.git@3.0.0")?; + requirements_in.write_str( + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@test-tag", + )?; uv_snapshot!(context.compile() .arg("requirements.in"), @r###" @@ -1175,38 +1155,53 @@ fn compile_git_tag_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - blinker==1.7.0 - # via flask - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@735a4701d6d5e848241e7d7535db898efb62d400 - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 ----- stderr ----- - Resolved 7 packages in [TIME] + Resolved 1 package in [TIME] + "### + ); + + Ok(()) +} + +/// Resolve a specific tag via a Git HTTPS dependency. +/// +/// In this case, the tag is a date, and thus could feasibly refer to a short commit hash. +#[test] +#[cfg(feature = "git")] +fn compile_git_date_tag_https_dependency() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str( + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@20240402", + )?; + + uv_snapshot!(context.compile() + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 + + ----- stderr ----- + Resolved 1 package in [TIME] "### ); Ok(()) } -/// Resolve a specific Flask commit via a Git HTTPS dependency. +/// Resolve a specific commit via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_long_commit_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); requirements_in.write_str( - "flask @ git+https://github.com/pallets/flask.git@d92b64aa275841b0c9aea3903aba72fbc4275d91", + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979", )?; uv_snapshot!(context.compile() @@ -1216,35 +1211,25 @@ fn compile_git_long_commit_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@d92b64aa275841b0c9aea3903aba72fbc4275d91 - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 ----- stderr ----- - Resolved 6 packages in [TIME] + Resolved 1 package in [TIME] "### ); Ok(()) } -/// Resolve a specific Flask commit via a Git HTTPS dependency. +/// Resolve a specific commit via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_short_commit_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); - requirements_in.write_str("flask @ git+https://github.com/pallets/flask.git@d92b64a")?; + requirements_in.write_str( + "uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd6", + )?; uv_snapshot!(context.compile() .arg("requirements.in"), @r###" @@ -1253,36 +1238,24 @@ fn compile_git_short_commit_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@d92b64aa275841b0c9aea3903aba72fbc4275d91 - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 ----- stderr ----- - Resolved 6 packages in [TIME] + Resolved 1 package in [TIME] "### ); Ok(()) } -/// Resolve a specific Flask ref via a Git HTTPS dependency. +/// Resolve a specific ref via a Git HTTPS dependency. #[test] #[cfg(feature = "git")] fn compile_git_refs_https_dependency() -> Result<()> { let context = TestContext::new("3.12"); let requirements_in = context.temp_dir.child("requirements.in"); requirements_in - .write_str("flask @ git+https://github.com/pallets/flask.git@refs/pull/5313/head")?; + .write_str("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@refs/pull/4/head")?; uv_snapshot!(context.compile() .arg("requirements.in"), @r###" @@ -1291,24 +1264,10 @@ fn compile_git_refs_https_dependency() -> Result<()> { ----- stdout ----- # This file was autogenerated by uv via the following command: # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in - blinker==1.7.0 - # via flask - click==8.1.7 - # via flask - flask @ git+https://github.com/pallets/flask.git@7af0271f4703a71beef8e26d1f5f6f8da04100e6 - itsdangerous==2.1.2 - # via flask - jinja2==3.1.3 - # via flask - markupsafe==2.1.5 - # via - # jinja2 - # werkzeug - werkzeug==3.0.1 - # via flask + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@9d01a806f17ddacb9c7b66b1b68574adf790b63f ----- stderr ----- - Resolved 7 packages in [TIME] + Resolved 1 package in [TIME] "### );