pioduino/uv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Allow conflicting Git URLs that refer to the same commit SHA (#2769)

Browse Source 
## Summary

This PR leverages our lookahead direct URL resolution to significantly
improve the range of Git URLs that we can accept (e.g., if a user
provides the same requirement, once as a direct dependency, and once as
a tag). We did some of this in #2285, but the solution here is more
general and works for arbitrary transitive URLs.

Closes https://github.com/astral-sh/uv/issues/2614.
This commit is contained in:
Charlie Marsh
2024-04-02 19:36:35 -04:00
committed by GitHub
parent 20d4762776
commit c30a65ee0c
9 changed files with 267 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/uv-resolver/src/resolver/urls.rs
+23 -120
View File
@@ -3,36 +3,28 @@ use tracing::debug;
use distribution_types::Verbatim;
use pep508_rs::{MarkerEnvironment, VerbatimUrl};
use uv_distribution::is_same_reference;
use uv_normalize::PackageName;
use crate::{Manifest, ResolveError};
/// A map of package names to their associated, required URLs.
#[derive(Debug, Default)]
pub(crate) struct Urls {
/// A map of package names to their associated, required URLs.
required: FxHashMap<PackageName, VerbatimUrl>,
/// A map from required URL to URL that is assumed to be a less precise variant.
allowed: FxHashMap<VerbatimUrl, VerbatimUrl>,
}
pub(crate) struct Urls(FxHashMap<PackageName, VerbatimUrl>);
impl Urls {
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: &MarkerEnvironment,
) -> Result<Self, ResolveError> {
let mut required: FxHashMap<PackageName, VerbatimUrl> = FxHashMap::default();
let mut allowed: FxHashMap<VerbatimUrl, VerbatimUrl> = FxHashMap::default();
let mut urls: FxHashMap<PackageName, VerbatimUrl> = FxHashMap::default();
// Add the themselves to the list of required URLs.
for (editable, metadata) in &manifest.editables {
if let Some(previous) = required.insert(metadata.name.clone(), editable.url.clone()) {
if let Some(previous) = urls.insert(metadata.name.clone(), editable.url.clone()) {
if !is_equal(&previous, &editable.url) {
if is_precise(&previous, &editable.url) {
debug!(
"Assuming {} is a precise variant of {previous}",
editable.url
);
allowed.insert(editable.url.clone(), previous);
if is_same_reference(&previous, &editable.url) {
debug!("Allowing {} as a variant of {previous}", editable.url);
} else {
return Err(ResolveError::ConflictingUrlsDirect(
metadata.name.clone(),
@@ -47,47 +39,38 @@ impl Urls {
// Add all direct requirements and constraints. If there are any conflicts, return an error.
for requirement in manifest.requirements(markers) {
if let Some(pep508_rs::VersionOrUrl::Url(url)) = &requirement.version_or_url {
if let Some(previous) = required.insert(requirement.name.clone(), url.clone()) {
if is_equal(&previous, url) {
continue;
if let Some(previous) = urls.insert(requirement.name.clone(), url.clone()) {
if !is_equal(&previous, url) {
if is_same_reference(&previous, url) {
debug!("Allowing {url} as a variant of {previous}");
} else {
return Err(ResolveError::ConflictingUrlsDirect(
requirement.name.clone(),
previous.verbatim().to_string(),
url.verbatim().to_string(),
));
}
}
if is_precise(&previous, url) {
debug!("Assuming {url} is a precise variant of {previous}");
allowed.insert(url.clone(), previous);
continue;
}
return Err(ResolveError::ConflictingUrlsDirect(
requirement.name.clone(),
previous.verbatim().to_string(),
url.verbatim().to_string(),
));
}
}
}
Ok(Self { required, allowed })
Ok(Self(urls))
}
/// Return the [`VerbatimUrl`] associated with the given package name, if any.
pub(crate) fn get(&self, package: &PackageName) -> Option<&VerbatimUrl> {
self.required.get(package)
self.0.get(package)
}
/// Returns `true` if the provided URL is compatible with the given "allowed" URL.
pub(crate) fn is_allowed(&self, expected: &VerbatimUrl, provided: &VerbatimUrl) -> bool {
pub(crate) fn is_allowed(expected: &VerbatimUrl, provided: &VerbatimUrl) -> bool {
#[allow(clippy::if_same_then_else)]
if is_equal(expected, provided) {
// If the URLs are canonically equivalent, they're compatible.
true
} else if self
.allowed
.get(expected)
.is_some_and(|allowed| is_equal(allowed, provided))
{
// If the URL is canonically equivalent to the imprecise variant of the URL, they're
// compatible.
} else if is_same_reference(expected, provided) {
// If the URLs refer to the same commit, they're compatible.
true
} else {
// Otherwise, they're incompatible.
@@ -103,53 +86,6 @@ fn is_equal(previous: &VerbatimUrl, url: &VerbatimUrl) -> bool {
cache_key::CanonicalUrl::new(previous.raw()) == cache_key::CanonicalUrl::new(url.raw())
}
/// Returns `true` if the [`VerbatimUrl`] appears to be a more precise variant of the previous
/// [`VerbatimUrl`].
///
/// Primarily, this method intends to accept URLs that map to the same repository, but with a
/// precise Git commit hash overriding a looser tag or branch. For example, if the previous URL
/// is `git+https://github.com/pallets/werkzeug.git@main`, this method would accept
/// `git+https://github.com/pallets/werkzeug@32e69512134c2f8183c6438b2b2e13fd24e9d19f`, and
/// assume that the latter is a more precise variant of the former. This is particularly useful
/// for workflows in which the output of `uv pip compile` is used as an input constraint on a
/// subsequent resolution, since `uv` will pin the exact commit hash of the package.
fn is_precise(previous: &VerbatimUrl, url: &VerbatimUrl) -> bool {
if cache_key::RepositoryUrl::new(previous.raw()) != cache_key::RepositoryUrl::new(url.raw()) {
return false;
}
// If there's no tag in the overriding URL, consider it incompatible.
let Some(url_tag) = url
.raw()
.path()
.rsplit_once('@')
.map(|(_prefix, suffix)| suffix)
else {
return false;
};
// Accept the overriding URL, as long as it's a full commit hash...
let url_is_commit = url_tag.len() == 40 && url_tag.chars().all(|ch| ch.is_ascii_hexdigit());
if !url_is_commit {
return false;
}
// If there's no tag in the previous URL, consider it compatible.
let Some(previous_tag) = previous
.raw()
.path()
.rsplit_once('@')
.map(|(_prefix, suffix)| suffix)
else {
return true;
};
// If the previous URL is a full commit hash, consider it incompatible.
let previous_is_commit =
previous_tag.len() == 40 && previous_tag.chars().all(|ch| ch.is_ascii_hexdigit());
!previous_is_commit
}
#[cfg(test)]
mod tests {
use super::*;
@@ -183,37 +119,4 @@ mod tests {
Ok(())
}
#[test]
fn url_precision() -> Result<(), url::ParseError> {
// Same repository, no tag on the previous URL, non-SHA on the overriding URL.
let previous = VerbatimUrl::parse_url("git+https://example.com/MyProject.git")?;
let url = VerbatimUrl::parse_url("git+https://example.com/MyProject.git@v1.0")?;
assert!(!is_precise(&previous, &url));
// Same repository, no tag on the previous URL, SHA on the overriding URL.
let previous = VerbatimUrl::parse_url("git+https://example.com/MyProject.git")?;
let url = VerbatimUrl::parse_url(
"git+https://example.com/MyProject.git@c3cd550a7a7c41b2c286ca52fbb6dec5fea195ef",
)?;
assert!(is_precise(&previous, &url));
// Same repository, tag on the previous URL, SHA on the overriding URL.
let previous = VerbatimUrl::parse_url("git+https://example.com/MyProject.git@v1.0")?;
let url = VerbatimUrl::parse_url(
"git+https://example.com/MyProject.git@c3cd550a7a7c41b2c286ca52fbb6dec5fea195ef",
)?;
assert!(is_precise(&previous, &url));
// Same repository, SHA on the previous URL, different SHA on the overriding URL.
let previous = VerbatimUrl::parse_url(
"git+https://example.com/MyProject.git@5ae5980c885e350a34ca019a84ba14a2a228d262",
)?;
let url = VerbatimUrl::parse_url(
"git+https://example.com/MyProject.git@c3cd550a7a7c41b2c286ca52fbb6dec5fea195ef",
)?;
assert!(!is_precise(&previous, &url));
Ok(())
}
}