From 2a212eb6a93bd32f264e0fa6868087154f57a58e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 4 May 2024 17:13:11 -0400 Subject: [PATCH] Add branch and tag variants to Git reference (#3374) ## Summary Closes https://github.com/astral-sh/uv/issues/3368. --- crates/uv-git/src/git.rs | 69 ++++++- crates/uv-git/src/lib.rs | 5 +- crates/uv-requirements/src/pyproject.rs | 13 +- crates/uv/tests/pip_compile.rs | 263 ++++++++++++++++++++++++ 4 files changed, 333 insertions(+), 17 deletions(-) diff --git a/crates/uv-git/src/git.rs b/crates/uv-git/src/git.rs index cd37c9fc2..583de6e88 100644 --- a/crates/uv-git/src/git.rs +++ b/crates/uv-git/src/git.rs @@ -25,6 +25,12 @@ const CHECKOUT_READY_LOCK: &str = ".ok"; /// A reference to commit or commit-ish. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum GitReference { + /// A specific branch. + Branch(String), + /// A specific tag. + Tag(String), + /// A specific (short) commit. + ShortCommit(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. @@ -46,7 +52,8 @@ enum RefspecStrategy { } impl GitReference { - /// Creates a [`GitReference`] from a revision string. + /// Creates a [`GitReference`] from an arbitrary revision string, which could represent a + /// branch, tag, commit, or named ref. pub(crate) fn from_rev(rev: &str) -> Self { if rev.starts_with("refs/") { Self::NamedRef(rev.to_owned()) @@ -64,9 +71,12 @@ impl GitReference { /// Converts the [`GitReference`] to a `str`. pub fn as_str(&self) -> Option<&str> { match self { + Self::Tag(rev) => Some(rev), + Self::Branch(rev) => Some(rev), + Self::ShortCommit(rev) => Some(rev), Self::BranchOrTag(rev) => Some(rev), - Self::FullCommit(rev) => Some(rev), Self::BranchOrTagOrCommit(rev) => Some(rev), + Self::FullCommit(rev) => Some(rev), Self::NamedRef(rev) => Some(rev), Self::DefaultBranch => None, } @@ -75,10 +85,13 @@ 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::BranchOrTag(rev) - | Self::FullCommit(rev) - | Self::BranchOrTagOrCommit(rev) - | Self::NamedRef(rev) => rev, + Self::Tag(rev) => rev, + Self::Branch(rev) => rev, + Self::ShortCommit(rev) => rev, + Self::BranchOrTag(rev) => rev, + Self::BranchOrTagOrCommit(rev) => rev, + Self::FullCommit(rev) => rev, + Self::NamedRef(rev) => rev, Self::DefaultBranch => "HEAD", } } @@ -86,6 +99,9 @@ impl GitReference { /// Returns the kind of this reference. pub(crate) fn kind_str(&self) -> &str { match self { + Self::Branch(_) => "branch", + Self::Tag(_) => "tag", + Self::ShortCommit(_) => "short commit", Self::BranchOrTag(_) => "branch or tag", Self::FullCommit(_) => "commit", Self::BranchOrTagOrCommit(_) => "branch, tag, or commit", @@ -262,6 +278,26 @@ impl GitReference { pub(crate) fn resolve(&self, repo: &git2::Repository) -> Result { let refkind = self.kind_str(); let id = match self { + 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 tag `{s}`"))?, + + Self::Branch(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()) + .ok_or_else(|| anyhow::format_err!("failed to find {refkind} `{s}`"))? + } + // Attempt to resolve the branch, then the tag. Self::BranchOrTag(s) => { let name = format!("origin/{s}"); @@ -312,14 +348,14 @@ impl GitReference { .ok_or_else(|| anyhow::format_err!("failed to find {refkind} `{s}`"))? } - // We'll be using the HEAD commit + // We'll be using the HEAD commit. Self::DefaultBranch => { let head_id = repo.refname_to_id("refs/remotes/origin/HEAD")?; let head = repo.find_object(head_id, None)?; head.peel(ObjectType::Commit)?.id() } - Self::FullCommit(s) | Self::NamedRef(s) => { + Self::FullCommit(s) | Self::ShortCommit(s) | Self::NamedRef(s) => { let obj = repo.revparse_single(s)?; match obj.as_tag() { Some(tag) => tag.target_id(), @@ -949,6 +985,14 @@ 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}" @@ -961,7 +1005,8 @@ pub(crate) fn fetch( // 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) => { + GitReference::ShortCommit(branch_or_tag_or_commit) + | 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. @@ -1329,10 +1374,14 @@ 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::NamedRef(rev) => rev, - GitReference::FullCommit(rev) | GitReference::BranchOrTagOrCommit(rev) => { + GitReference::FullCommit(rev) + | GitReference::ShortCommit(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 diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index f058c8149..dd4168a70 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -104,7 +104,10 @@ impl From for Url { } else { // Otherwise, add the branch or tag name. match git.reference { - GitReference::BranchOrTag(rev) + GitReference::Branch(rev) + | GitReference::Tag(rev) + | GitReference::ShortCommit(rev) + | GitReference::BranchOrTag(rev) | GitReference::NamedRef(rev) | GitReference::FullCommit(rev) | GitReference::BranchOrTagOrCommit(rev) => { diff --git a/crates/uv-requirements/src/pyproject.rs b/crates/uv-requirements/src/pyproject.rs index 291d94c52..7f314a660 100644 --- a/crates/uv-requirements/src/pyproject.rs +++ b/crates/uv-requirements/src/pyproject.rs @@ -426,18 +426,19 @@ pub(crate) fn lower_requirement( if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) { return Err(LoweringError::ConflictingUrls); } - // TODO(konsti): We know better than this enum let reference = match (rev, tag, branch) { (None, None, None) => GitReference::DefaultBranch, (Some(rev), None, None) => { - if rev.len() == 40 { - GitReference::FullCommit(rev) + if rev.starts_with("refs/") { + GitReference::NamedRef(rev.clone()) + } else if rev.len() == 40 { + GitReference::FullCommit(rev.clone()) } else { - GitReference::BranchOrTagOrCommit(rev) + GitReference::ShortCommit(rev.clone()) } } - (None, Some(tag), None) => GitReference::BranchOrTag(tag), - (None, None, Some(branch)) => GitReference::BranchOrTag(branch), + (None, Some(tag), None) => GitReference::Tag(tag), + (None, None, Some(branch)) => GitReference::Branch(branch), _ => return Err(LoweringError::MoreThanOneGitRef), }; diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index ce6cd40dd..8bfa8f9cf 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -8389,3 +8389,266 @@ fn resolve_configuration() -> Result<()> { Ok(()) } + +/// Resolve a specific source distribution via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_default_branch() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + 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]")] + .into_iter() + .chain(context.filters()) + .collect(); + + uv_snapshot!(filters, context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@[COMMIT] + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Resolve a specific branch via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_branch() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", branch = "test-branch" } + "#})?; + + uv_snapshot!(context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Resolve a specific tag via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_tag() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", tag = "test-tag" } + "#})?; + + uv_snapshot!(context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Resolve a specific commit via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_long_commit() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", rev = "0dacfd662c64cb4ceb16e6cf65a157a8b715b979" } + "#})?; + + uv_snapshot!(context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Resolve a specific commit via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_short_commit() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", rev = "0dacfd6" } + "#})?; + + uv_snapshot!(context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979 + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Resolve a specific ref via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +fn git_source_refs() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", rev = "refs/pull/4/head" } + "#})?; + + uv_snapshot!(context.compile() + .arg("--preview") + .arg("pyproject.toml"), @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 --preview pyproject.toml + uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@9d01a806f17ddacb9c7b66b1b68574adf790b63f + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + Ok(()) +} + +/// Request a non-existent tag via a Git HTTPS dependency. +#[test] +#[cfg(feature = "git")] +#[cfg_attr(windows, ignore = "Git error messages differ on Windows")] +fn git_source_missing_tag() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.0.0" + dependencies = [ + "uv-public-pypackage", + ] + + [tool.uv.sources] + uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", tag = "missing" } + "#})?; + + uv_snapshot!(context.filters(), context.compile() + .arg("--preview") + .arg("pyproject.toml"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to download and build: `uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@missing` + Caused by: Git operation failed + Caused by: failed to clone into: [CACHE_DIR]/git-v0/db/8dab139913c4b566 + Caused by: failed to fetch branch or tag `missing` + Caused by: process didn't exit successfully: `git fetch --force --update-head-ok 'https://github.com/astral-test/uv-public-pypackage' '+refs/tags/missing:refs/remotes/origin/tags/missing'` (exit status: 128) + --- stderr + fatal: couldn't find remote ref refs/tags/missing + + "###); + + Ok(()) +}