From 29c2be3e97af83c2a137caad02891c05f71af6d6 Mon Sep 17 00:00:00 2001 From: konsti Date: Tue, 18 Feb 2025 03:14:06 +0100 Subject: [PATCH] Eagerly reject unsupported Git schemes (#11514) Initially, we were limiting Git schemes to HTTPS and SSH as only supported schemes. We lost this validation in #3429. This incidentally allowed file schemes, which apparently work with Git out of the box. A caveat for this is that in tool.uv.sources, we parse the git field always as URL. This caused a problem with #11425: repo = { git = 'c:\path\to\repo', rev = "xxxxx" } was parsed as a URL where c: is the scheme, causing a bad error message down the line. This PR: * Puts Git URL validation back in place. It bans everything but HTTPS, SSH, and file URLs. This could be a breaking change, if users were using a git transport protocol were not aware of, even though never intentionally supported. * Allows file: URL in Git: This seems to be supported by Git and we were supporting it albeit unintentionally, so it's reasonable to continue to support it. * It does not allow relative paths in the git field in tool.uv.sources. Absolute file URLs are supported, whether we want relative file URLs for Git too should be discussed separately. Closes #3429: We reject the input with a proper error message, while hinting the user towards file:. If there's still desire for relative path support, we can keep it open. --------- Co-authored-by: Charlie Marsh --- Cargo.lock | 1 - crates/uv-distribution-types/src/lib.rs | 4 +- .../uv-distribution-types/src/resolution.rs | 4 +- .../uv-distribution/src/metadata/lowering.rs | 16 ++-- crates/uv-git-types/src/lib.rs | 55 ++++++++++--- crates/uv-installer/src/satisfies.rs | 16 ++-- crates/uv-pypi-types/src/parsed_url.rs | 21 ++--- crates/uv-pypi-types/src/requirement.rs | 79 +++++++------------ crates/uv-requirements/Cargo.toml | 1 - crates/uv-requirements/src/lib.rs | 24 ++---- crates/uv-resolver/src/lock/mod.rs | 33 ++++---- .../uv-resolver/src/lock/requirements_txt.rs | 3 +- .../uv-resolver/src/pubgrub/dependencies.rs | 12 +-- crates/uv-workspace/src/pyproject.rs | 25 +++--- crates/uv/src/commands/project/add.rs | 18 ++--- crates/uv/tests/it/edit.rs | 20 +++++ crates/uv/tests/it/pip_install.rs | 18 +++++ crates/uv/tests/it/sync.rs | 33 ++++++++ 18 files changed, 212 insertions(+), 171 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe32a38eb..baa1f4816 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5520,7 +5520,6 @@ dependencies = [ "uv-distribution-types", "uv-fs", "uv-git", - "uv-git-types", "uv-normalize", "uv-pep508", "uv-pypi-types", diff --git a/crates/uv-distribution-types/src/lib.rs b/crates/uv-distribution-types/src/lib.rs index bf098c73c..7b553b17c 100644 --- a/crates/uv-distribution-types/src/lib.rs +++ b/crates/uv-distribution-types/src/lib.rs @@ -714,9 +714,7 @@ impl GitSourceDist { /// Return the [`ParsedUrl`] for the distribution. pub fn parsed_url(&self) -> ParsedUrl { ParsedUrl::Git(ParsedGitUrl::from_source( - self.git.repository().clone(), - self.git.reference().clone(), - self.git.precise(), + (*self.git).clone(), self.subdirectory.clone(), )) } diff --git a/crates/uv-distribution-types/src/resolution.rs b/crates/uv-distribution-types/src/resolution.rs index 63c58581d..ae50849ba 100644 --- a/crates/uv-distribution-types/src/resolution.rs +++ b/crates/uv-distribution-types/src/resolution.rs @@ -266,10 +266,8 @@ impl From<&ResolvedDist> for RequirementSource { } } Dist::Source(SourceDist::Git(sdist)) => RequirementSource::Git { + git: (*sdist.git).clone(), url: sdist.url.clone(), - repository: sdist.git.repository().clone(), - reference: sdist.git.reference().clone(), - precise: sdist.git.precise(), subdirectory: sdist.subdirectory.clone(), }, Dist::Source(SourceDist::Path(sdist)) => RequirementSource::Path { diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs index fc0243644..7c1767037 100644 --- a/crates/uv-distribution/src/metadata/lowering.rs +++ b/crates/uv-distribution/src/metadata/lowering.rs @@ -8,7 +8,7 @@ use url::Url; use uv_distribution_filename::DistExtension; use uv_distribution_types::{Index, IndexLocations, IndexName, Origin}; -use uv_git_types::GitReference; +use uv_git_types::{GitReference, GitUrl, GitUrlParseError}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::VersionSpecifiers; use uv_pep508::{looks_like_git_repository, MarkerTree, VerbatimUrl, VersionOrUrl}; @@ -291,9 +291,7 @@ impl LoweredRequirement { .expect("Workspace member must be relative"); let subdirectory = uv_fs::normalize_path_buf(subdirectory); RequirementSource::Git { - repository: git_member.git_source.git.repository().clone(), - reference: git_member.git_source.git.reference().clone(), - precise: git_member.git_source.git.precise(), + git: git_member.git_source.git.clone(), subdirectory: if subdirectory == PathBuf::new() { None } else { @@ -497,6 +495,8 @@ pub enum LoweringError { UndeclaredWorkspacePackage(PackageName), #[error("Can only specify one of: `rev`, `tag`, or `branch`")] MoreThanOneGitRef, + #[error(transparent)] + GitUrlParse(#[from] GitUrlParseError), #[error("Package `{0}` references an undeclared index: `{1}`")] MissingIndex(PackageName, IndexName), #[error("Workspace members are not allowed in non-workspace contexts")] @@ -575,9 +575,7 @@ fn git_source( Ok(RequirementSource::Git { url, - repository, - reference, - precise: None, + git: GitUrl::from_reference(repository, reference)?, subdirectory, }) } @@ -679,9 +677,7 @@ fn path_source( .expect("Workspace member must be relative"); let subdirectory = uv_fs::normalize_path_buf(subdirectory); return Ok(RequirementSource::Git { - repository: git_member.git_source.git.repository().clone(), - reference: git_member.git_source.git.reference().clone(), - precise: git_member.git_source.git.precise(), + git: git_member.git_source.git.clone(), subdirectory: if subdirectory == PathBuf::new() { None } else { diff --git a/crates/uv-git-types/src/lib.rs b/crates/uv-git-types/src/lib.rs index 927e81869..ee1ffb27f 100644 --- a/crates/uv-git-types/src/lib.rs +++ b/crates/uv-git-types/src/lib.rs @@ -1,12 +1,22 @@ pub use crate::github::GitHubRepository; pub use crate::oid::{GitOid, OidParseError}; pub use crate::reference::GitReference; + +use thiserror::Error; use url::Url; mod github; mod oid; mod reference; +#[derive(Debug, Error)] +pub enum GitUrlParseError { + #[error( + "Unsupported Git URL scheme `{0}:` in `{1}` (expected one of `https:`, `ssh:`, or `file:`)" + )] + UnsupportedGitScheme(String, Url), +} + /// A URL reference to a Git repository. #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Hash, Ord)] pub struct GitUrl { @@ -21,21 +31,42 @@ pub struct GitUrl { impl GitUrl { /// Create a new [`GitUrl`] from a repository URL and a reference. - pub fn from_reference(repository: Url, reference: GitReference) -> Self { - Self { - repository, - reference, - precise: None, - } + pub fn from_reference( + repository: Url, + reference: GitReference, + ) -> Result { + Self::from_fields(repository, reference, None) } /// Create a new [`GitUrl`] from a repository URL and a precise commit. - pub fn from_commit(repository: Url, reference: GitReference, precise: GitOid) -> Self { - Self { + pub fn from_commit( + repository: Url, + reference: GitReference, + precise: GitOid, + ) -> Result { + Self::from_fields(repository, reference, Some(precise)) + } + + /// Create a new [`GitUrl`] from a repository URL and a precise commit, if known. + pub fn from_fields( + repository: Url, + reference: GitReference, + precise: Option, + ) -> Result { + match repository.scheme() { + "https" | "ssh" | "file" => {} + unsupported => { + return Err(GitUrlParseError::UnsupportedGitScheme( + unsupported.to_string(), + repository, + )) + } + } + Ok(Self { repository, reference, - precise: Some(precise), - } + precise, + }) } /// Set the precise [`GitOid`] to use for this Git URL. @@ -69,7 +100,7 @@ impl GitUrl { } impl TryFrom for GitUrl { - type Error = OidParseError; + type Error = GitUrlParseError; /// Initialize a [`GitUrl`] source from a URL. fn try_from(mut url: Url) -> Result { @@ -89,7 +120,7 @@ impl TryFrom for GitUrl { url.set_path(&prefix); } - Ok(Self::from_reference(url, reference)) + Self::from_reference(url, reference) } } diff --git a/crates/uv-installer/src/satisfies.rs b/crates/uv-installer/src/satisfies.rs index f30b41017..07ba32bcb 100644 --- a/crates/uv-installer/src/satisfies.rs +++ b/crates/uv-installer/src/satisfies.rs @@ -97,9 +97,7 @@ impl RequirementSatisfaction { } RequirementSource::Git { url: _, - repository: requested_repository, - reference: _, - precise: requested_precise, + git: requested_git, subdirectory: requested_subdirectory, } => { let InstalledDist::Url(InstalledDirectUrlDist { direct_url, .. }) = &distribution @@ -129,21 +127,25 @@ impl RequirementSatisfaction { } if !RepositoryUrl::parse(installed_url).is_ok_and(|installed_url| { - installed_url == RepositoryUrl::new(requested_repository) + installed_url == RepositoryUrl::new(requested_git.repository()) }) { debug!( "Repository mismatch: {:?} vs. {:?}", - installed_url, requested_repository + installed_url, + requested_git.repository() ); return Ok(Self::Mismatch); } // TODO(charlie): It would be more consistent for us to compare the requested // revisions here. - if installed_precise.as_deref() != requested_precise.as_ref().map(GitOid::as_str) { + if installed_precise.as_deref() + != requested_git.precise().as_ref().map(GitOid::as_str) + { debug!( "Precise mismatch: {:?} vs. {:?}", - installed_precise, requested_precise + installed_precise, + requested_git.precise() ); return Ok(Self::OutOfDate); } diff --git a/crates/uv-pypi-types/src/parsed_url.rs b/crates/uv-pypi-types/src/parsed_url.rs index ca1d441b4..f72bf095f 100644 --- a/crates/uv-pypi-types/src/parsed_url.rs +++ b/crates/uv-pypi-types/src/parsed_url.rs @@ -5,7 +5,7 @@ use thiserror::Error; use url::{ParseError, Url}; use uv_distribution_filename::{DistExtension, ExtensionError}; -use uv_git_types::{GitOid, GitReference, GitUrl, OidParseError}; +use uv_git_types::{GitUrl, GitUrlParseError}; use uv_pep508::{ looks_like_git_repository, Pep508Url, UnnamedRequirementUrl, VerbatimUrl, VerbatimUrlError, }; @@ -22,8 +22,8 @@ pub enum ParsedUrlError { }, #[error("Invalid path in file URL: `{0}`")] InvalidFileUrl(String), - #[error("Failed to parse Git reference from URL: `{0}`")] - GitOidParse(String, #[source] OidParseError), + #[error(transparent)] + GitUrlParse(#[from] GitUrlParseError), #[error("Not a valid URL: `{0}`")] UrlParse(String, #[source] ParseError), #[error(transparent)] @@ -244,17 +244,7 @@ pub struct ParsedGitUrl { impl ParsedGitUrl { /// Construct a [`ParsedGitUrl`] from a Git requirement source. - pub fn from_source( - repository: Url, - reference: GitReference, - precise: Option, - subdirectory: Option, - ) -> Self { - let url = if let Some(precise) = precise { - GitUrl::from_commit(repository, reference, precise) - } else { - GitUrl::from_reference(repository, reference) - }; + pub fn from_source(url: GitUrl, subdirectory: Option) -> Self { Self { url, subdirectory } } } @@ -274,8 +264,7 @@ impl TryFrom for ParsedGitUrl { .strip_prefix("git+") .unwrap_or(url_in.as_str()); let url = Url::parse(url).map_err(|err| ParsedUrlError::UrlParse(url.to_string(), err))?; - let url = GitUrl::try_from(url) - .map_err(|err| ParsedUrlError::GitOidParse(url_in.to_string(), err))?; + let url = GitUrl::try_from(url)?; Ok(Self { url, subdirectory }) } } diff --git a/crates/uv-pypi-types/src/requirement.rs b/crates/uv-pypi-types/src/requirement.rs index 778f42c33..00190e8a5 100644 --- a/crates/uv-pypi-types/src/requirement.rs +++ b/crates/uv-pypi-types/src/requirement.rs @@ -8,7 +8,7 @@ use url::Url; use uv_distribution_filename::DistExtension; use uv_fs::{relative_to, PortablePath, PortablePathBuf, CWD}; -use uv_git_types::{GitOid, GitReference, GitUrl, OidParseError}; +use uv_git_types::{GitOid, GitReference, GitUrl, GitUrlParseError, OidParseError}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::VersionSpecifiers; use uv_pep508::{ @@ -30,6 +30,8 @@ pub enum RequirementError { UrlParseError(#[from] url::ParseError), #[error(transparent)] OidParseError(#[from] OidParseError), + #[error(transparent)] + GitUrlParse(#[from] GitUrlParseError), } /// A representation of dependency on a package, an extension over a PEP 508's requirement. @@ -226,25 +228,16 @@ impl From for uv_pep508::Requirement { verbatim: url, })), RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url, - } => { - let git_url = if let Some(precise) = precise { - GitUrl::from_commit(repository, reference, precise) - } else { - GitUrl::from_reference(repository, reference) - }; - Some(VersionOrUrl::Url(VerbatimParsedUrl { - parsed_url: ParsedUrl::Git(ParsedGitUrl { - url: git_url, - subdirectory, - }), - verbatim: url, - })) - } + } => Some(VersionOrUrl::Url(VerbatimParsedUrl { + parsed_url: ParsedUrl::Git(ParsedGitUrl { + url: git, + subdirectory, + }), + verbatim: url, + })), RequirementSource::Path { install_path, ext, @@ -336,13 +329,11 @@ impl Display for Requirement { } RequirementSource::Git { url: _, - repository, - reference, - precise: _, + git, subdirectory, } => { - write!(f, " @ git+{repository}")?; - if let Some(reference) = reference.as_str() { + write!(f, " @ git+{}", git.repository())?; + if let Some(reference) = git.reference().as_str() { write!(f, "@{reference}")?; } if let Some(subdirectory) = subdirectory { @@ -401,12 +392,8 @@ pub enum RequirementSource { }, /// A remote Git repository, over either HTTPS or SSH. Git { - /// The repository URL (without the `git+` prefix). - repository: Url, - /// Optionally, the revision, tag, or branch to use. - reference: GitReference, - /// The precise commit to use, if known. - precise: Option, + /// The repository URL and reference to the commit to use. + git: GitUrl, /// The path to the source distribution if it is not in the repository root. subdirectory: Option, /// The PEP 508 style url in the format @@ -457,10 +444,8 @@ impl RequirementSource { url, }, ParsedUrl::Git(git) => RequirementSource::Git { + git: git.url.clone(), url, - repository: git.url.repository().clone(), - reference: git.url.reference().clone(), - precise: git.url.precise(), subdirectory: git.subdirectory, }, ParsedUrl::Archive(archive) => RequirementSource::Url { @@ -516,16 +501,12 @@ impl RequirementSource { verbatim: url.clone(), }), Self::Git { - repository, - reference, - precise, + git, subdirectory, url, } => Some(VerbatimParsedUrl { parsed_url: ParsedUrl::Git(ParsedGitUrl::from_source( - repository.clone(), - reference.clone(), - *precise, + git.clone(), subdirectory.clone(), )), verbatim: url.clone(), @@ -628,13 +609,11 @@ impl Display for RequirementSource { } Self::Git { url: _, - repository, - reference, - precise: _, + git, subdirectory, } => { - write!(f, " git+{repository}")?; - if let Some(reference) = reference.as_str() { + write!(f, " git+{}", git.repository())?; + if let Some(reference) = git.reference().as_str() { write!(f, "@{reference}")?; } if let Some(subdirectory) = subdirectory { @@ -706,13 +685,11 @@ impl From for RequirementSourceWire { subdirectory: subdirectory.map(PortablePathBuf::from), }, RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url: _, } => { - let mut url = repository; + let mut url = git.repository().clone(); // Redact the credentials. redact_credentials(&mut url); @@ -733,7 +710,7 @@ impl From for RequirementSourceWire { } // Put the requested reference in the query. - match reference { + match git.reference() { GitReference::Branch(branch) => { url.query_pairs_mut().append_pair("branch", branch.as_str()); } @@ -749,7 +726,7 @@ impl From for RequirementSourceWire { } // Put the precise commit in the fragment. - if let Some(precise) = precise { + if let Some(precise) = git.precise() { url.set_fragment(Some(&precise.to_string())); } @@ -839,9 +816,7 @@ impl TryFrom for RequirementSource { let url = VerbatimUrl::from_url(url); Ok(Self::Git { - repository, - reference, - precise, + git: GitUrl::from_fields(repository, reference, precise)?, subdirectory: subdirectory.map(PathBuf::from), url, }) diff --git a/crates/uv-requirements/Cargo.toml b/crates/uv-requirements/Cargo.toml index ea71c6ce8..d3fbf929b 100644 --- a/crates/uv-requirements/Cargo.toml +++ b/crates/uv-requirements/Cargo.toml @@ -25,7 +25,6 @@ uv-distribution-filename = { workspace = true } uv-distribution-types = { workspace = true } uv-fs = { workspace = true } uv-git = { workspace = true } -uv-git-types = { workspace = true } uv-normalize = { workspace = true } uv-pep508 = { workspace = true } uv-pypi-types = { workspace = true } diff --git a/crates/uv-requirements/src/lib.rs b/crates/uv-requirements/src/lib.rs index ad9f9ed39..bbe76a1b9 100644 --- a/crates/uv-requirements/src/lib.rs +++ b/crates/uv-requirements/src/lib.rs @@ -6,7 +6,6 @@ pub use crate::specification::*; pub use crate::unnamed::*; use uv_distribution_types::{Dist, DistErrorKind, GitSourceDist, SourceDist}; -use uv_git_types::GitUrl; use uv_pypi_types::{Requirement, RequirementSource}; mod extras; @@ -58,24 +57,15 @@ pub(crate) fn required_dist( *ext, )?, RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url, - } => { - let git_url = if let Some(precise) = precise { - GitUrl::from_commit(repository.clone(), reference.clone(), *precise) - } else { - GitUrl::from_reference(repository.clone(), reference.clone()) - }; - Dist::Source(SourceDist::Git(GitSourceDist { - name: requirement.name.clone(), - git: Box::new(git_url), - subdirectory: subdirectory.clone(), - url: url.clone(), - })) - } + } => Dist::Source(SourceDist::Git(GitSourceDist { + name: requirement.name.clone(), + git: Box::new(git.clone()), + subdirectory: subdirectory.clone(), + url: url.clone(), + })), RequirementSource::Path { install_path, ext, diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index bb692f46d..d5903e487 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -32,7 +32,7 @@ use uv_distribution_types::{ }; use uv_fs::{relative_to, PortablePath, PortablePathBuf}; use uv_git::{RepositoryReference, ResolvedRepositoryReference}; -use uv_git_types::{GitOid, GitReference}; +use uv_git_types::{GitOid, GitReference, GitUrl, GitUrlParseError}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; use uv_pep508::{split_scheme, MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError}; @@ -2273,7 +2273,7 @@ impl Package { url, GitReference::from(git.kind.clone()), git.precise, - ); + )?; // Reconstruct the PEP 508-compatible URL from the `GitSource`. let url = Url::from(ParsedGitUrl { @@ -4331,22 +4331,27 @@ fn normalize_requirement( // Normalize the requirement source. match requirement.source { RequirementSource::Git { - mut repository, - reference, - precise, + git, subdirectory, url: _, } => { - // Redact the credentials. - redact_credentials(&mut repository); + // Reconstruct the Git URL. + let git = { + let mut repository = git.repository().clone(); - // Remove the fragment and query from the URL; they're already present in the source. - repository.set_fragment(None); - repository.set_query(None); + // Redact the credentials. + redact_credentials(&mut repository); + + // Remove the fragment and query from the URL; they're already present in the source. + repository.set_fragment(None); + repository.set_query(None); + + GitUrl::from_fields(repository, git.reference().clone(), git.precise())? + }; // Reconstruct the PEP 508 URL from the underlying data. let url = Url::from(ParsedGitUrl { - url: uv_git_types::GitUrl::from_reference(repository.clone(), reference.clone()), + url: git.clone(), subdirectory: subdirectory.clone(), }); @@ -4356,9 +4361,7 @@ fn normalize_requirement( groups: requirement.groups, marker: requirement.marker, source: RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url: VerbatimUrl::from_url(url), }, @@ -5036,6 +5039,8 @@ enum LockErrorKind { package2: PackageName, extra2: ExtraName, }, + #[error(transparent)] + GitUrlParse(#[from] GitUrlParseError), } /// An error that occurs when a source string could not be parsed. diff --git a/crates/uv-resolver/src/lock/requirements_txt.rs b/crates/uv-resolver/src/lock/requirements_txt.rs index 1462c6a3e..040b2407e 100644 --- a/crates/uv-resolver/src/lock/requirements_txt.rs +++ b/crates/uv-resolver/src/lock/requirements_txt.rs @@ -317,7 +317,8 @@ impl std::fmt::Display for RequirementsTxtExport<'_> { url, GitReference::from(git.kind.clone()), git.precise, - ); + ) + .expect("Internal Git URLs must have supported schemes"); // Reconstruct the PEP 508-compatible URL from the `GitSource`. let url = Url::from(ParsedGitUrl { diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs index 8d2997756..a390aab80 100644 --- a/crates/uv-resolver/src/pubgrub/dependencies.rs +++ b/crates/uv-resolver/src/pubgrub/dependencies.rs @@ -181,18 +181,12 @@ impl PubGrubRequirement { (url, parsed_url) } RequirementSource::Git { - repository, - reference, - precise, + git, url, subdirectory, } => { - let parsed_url = ParsedUrl::Git(ParsedGitUrl::from_source( - repository.clone(), - reference.clone(), - *precise, - subdirectory.clone(), - )); + let parsed_url = + ParsedUrl::Git(ParsedGitUrl::from_source(git.clone(), subdirectory.clone())); (url, parsed_url) } RequirementSource::Path { diff --git a/crates/uv-workspace/src/pyproject.rs b/crates/uv-workspace/src/pyproject.rs index 4d7733e63..758d10d48 100644 --- a/crates/uv-workspace/src/pyproject.rs +++ b/crates/uv-workspace/src/pyproject.rs @@ -6,10 +6,10 @@ //! //! Then lowers them into a dependency specification. +use std::collections::BTreeMap; use std::ops::Deref; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::{collections::BTreeMap, mem}; use glob::Pattern; use owo_colors::OwoColorize; @@ -1500,25 +1500,22 @@ impl Source { group: None, }, RequirementSource::Git { - repository, - mut reference, - subdirectory, - .. + git, subdirectory, .. } => { if rev.is_none() && tag.is_none() && branch.is_none() { - let rev = match reference { - GitReference::Branch(ref mut rev) => Some(mem::take(rev)), - GitReference::Tag(ref mut rev) => Some(mem::take(rev)), - GitReference::BranchOrTag(ref mut rev) => Some(mem::take(rev)), - GitReference::BranchOrTagOrCommit(ref mut rev) => Some(mem::take(rev)), - GitReference::NamedRef(ref mut rev) => Some(mem::take(rev)), + let rev = match git.reference() { + GitReference::Branch(rev) => Some(rev), + GitReference::Tag(rev) => Some(rev), + GitReference::BranchOrTag(rev) => Some(rev), + GitReference::BranchOrTagOrCommit(rev) => Some(rev), + GitReference::NamedRef(rev) => Some(rev), GitReference::DefaultBranch => None, }; Source::Git { - rev, + rev: rev.cloned(), tag, branch, - git: repository, + git: git.repository().clone(), subdirectory: subdirectory.map(PortablePathBuf::from), marker: MarkerTree::TRUE, extra: None, @@ -1529,7 +1526,7 @@ impl Source { rev, tag, branch, - git: repository, + git: git.repository().clone(), subdirectory: subdirectory.map(PortablePathBuf::from), marker: MarkerTree::TRUE, extra: None, diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index ec608a22b..97173cb49 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -907,25 +907,21 @@ fn augment_requirement( UnresolvedRequirement::Named(uv_pypi_types::Requirement { source: match requirement.source { RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url, } => { - let reference = if let Some(rev) = rev { - GitReference::from_rev(rev.to_string()) + let git = if let Some(rev) = rev { + git.with_reference(GitReference::from_rev(rev.to_string())) } else if let Some(tag) = tag { - GitReference::Tag(tag.to_string()) + git.with_reference(GitReference::Tag(tag.to_string())) } else if let Some(branch) = branch { - GitReference::Branch(branch.to_string()) + git.with_reference(GitReference::Branch(branch.to_string())) } else { - reference + git }; RequirementSource::Git { - repository, - reference, - precise, + git, subdirectory, url, } diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index a61933014..7411ee2f3 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -9770,3 +9770,23 @@ fn add_with_build_constraints() -> Result<()> { Ok(()) } + +#[test] +#[cfg(feature = "git")] +fn add_unsupported_git_scheme() { + let context = TestContext::new("3.12"); + + context.init().arg(".").assert().success(); + + uv_snapshot!(context.filters(), context.add().arg("git+fantasy://ferris/dreams/of/urls@7701ffcbae245819b828dc5f885a5201158897ef"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse: `git+fantasy://ferris/dreams/of/urls@7701ffcbae245819b828dc5f885a5201158897ef` + Caused by: Unsupported Git URL scheme `fantasy:` in `fantasy://ferris/dreams/of/urls` (expected one of `https:`, `ssh:`, or `file:`) + git+fantasy://ferris/dreams/of/urls@7701ffcbae245819b828dc5f885a5201158897ef + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "###); +} diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 7e3ac854a..42111acac 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -8940,3 +8940,21 @@ fn no_sources_workspace_discovery() -> Result<()> { Ok(()) } + +#[test] +fn unsupported_git_scheme() { + let context = TestContext::new("3.12"); + uv_snapshot!(context.filters(), context.pip_install() + .arg("git+fantasy://foo"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse: `git+fantasy://foo` + Caused by: Unsupported Git URL scheme `fantasy:` in `fantasy://foo` (expected one of `https:`, `ssh:`, or `file:`) + git+fantasy://foo + ^^^^^^^^^^^^^^^^^ + "### + ); +} diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs index ff44943b0..9f530ecd0 100644 --- a/crates/uv/tests/it/sync.rs +++ b/crates/uv/tests/it/sync.rs @@ -7681,3 +7681,36 @@ fn sync_locked_script() -> Result<()> { Ok(()) } + +#[test] +fn unsupported_git_scheme() -> Result<()> { + let context = TestContext::new_with_versions(&["3.12"]); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "foo" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["foo"] + + [tool.uv.sources] + # `c:/...` looks like an absolute path, but this field requires a URL such as `file:///...`. + foo = { git = "c:/home/ferris/projects/foo", rev = "7701ffcbae245819b828dc5f885a5201158897ef" } + "#}, + )?; + + uv_snapshot!(context.filters(), context.sync(), @r###" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + × Failed to build `foo @ file://[TEMP_DIR]/` + ├─▶ Failed to parse entry: `foo` + ╰─▶ Unsupported Git URL scheme `c:` in `c:/home/ferris/projects/foo` (expected one of `https:`, `ssh:`, or `file:`) + "###); + Ok(()) +}