Skip to content

Commit

Permalink
Use intersection rather than union for requires-python (#5644)
Browse files Browse the repository at this point in the history
## Summary

As-is, if you have a workspace with mixed `requires-python`
requirements, resolution will _never_ succeed, since we'll use the union
as the `requires-python` bound (i.e., take the lowest value), and fail
when we see the package that only supports some more narrow range.

This PR modifies the behavior to take the intersection (i.e., the
highest value), so if you have one package that supports Python 3.12 and
later, and another that supports Python 3.8 and later, we lock for
Python 3.12. If you try to sync or run with Python 3.8, we raise an
error, since the lockfile will be incompatible with that request.

Konsti has a write-up in #5594
that outlines what could be a longer-term strategy.

Closes #5578.
  • Loading branch information
charliermarsh committed Jul 31, 2024
1 parent dfec262 commit bf8934e
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 23 deletions.
39 changes: 17 additions & 22 deletions crates/uv-resolver/src/requires_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ impl RequiresPython {
})
}

/// Returns a [`RequiresPython`] to express the union of the given version specifiers.
/// Returns a [`RequiresPython`] to express the intersection of the given version specifiers.
///
/// For example, given `>=3.8` and `>=3.9`, this would return `>=3.8`.
pub fn union<'a>(
/// For example, given `>=3.8` and `>=3.9`, this would return `>=3.9`.
pub fn intersection<'a>(
specifiers: impl Iterator<Item = &'a VersionSpecifiers>,
) -> Result<Option<Self>, RequiresPythonError> {
// Convert to PubGrub range and perform a union.
// Convert to PubGrub range and perform an intersection.
let range = specifiers
.into_iter()
.map(crate::pubgrub::PubGrubSpecifier::from_release_specifiers)
.fold_ok(None, |range: Option<Range<Version>>, requires_python| {
if let Some(range) = range {
Some(range.union(&requires_python.into()))
Some(range.intersection(&requires_python.into()))
} else {
Some(requires_python.into())
}
Expand Down Expand Up @@ -107,11 +107,14 @@ impl RequiresPython {
/// Narrow the [`RequiresPython`] to the given version, if it's stricter (i.e., greater) than
/// the current target.
pub fn narrow(&self, target: &RequiresPythonBound) -> Option<Self> {
let target = VersionSpecifiers::from(VersionSpecifier::from_lower_bound(target)?);
Self::union(std::iter::once(&target))
.ok()
.flatten()
.filter(|next| next.bound > self.bound)
if target > &self.bound {
Some(Self {
specifiers: VersionSpecifiers::from(VersionSpecifier::from_lower_bound(target)?),
bound: target.clone(),
})
} else {
None
}
}

/// Returns `true` if the `Requires-Python` is compatible with the given version.
Expand Down Expand Up @@ -418,9 +421,7 @@ mod tests {
#[test]
fn requires_python_included() {
let version_specifiers = VersionSpecifiers::from_str("==3.10.*").unwrap();
let requires_python = RequiresPython::union(std::iter::once(&version_specifiers))
.unwrap()
.unwrap();
let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap();
let wheel_names = &[
"bcrypt-4.1.3-cp37-abi3-macosx_10_12_universal2.whl",
"black-24.4.2-cp310-cp310-win_amd64.whl",
Expand All @@ -437,9 +438,7 @@ mod tests {
}

let version_specifiers = VersionSpecifiers::from_str(">=3.12.3").unwrap();
let requires_python = RequiresPython::union(std::iter::once(&version_specifiers))
.unwrap()
.unwrap();
let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap();
let wheel_names = &["dearpygui-1.11.1-cp312-cp312-win_amd64.whl"];
for wheel_name in wheel_names {
assert!(
Expand All @@ -452,9 +451,7 @@ mod tests {
#[test]
fn requires_python_dropped() {
let version_specifiers = VersionSpecifiers::from_str("==3.10.*").unwrap();
let requires_python = RequiresPython::union(std::iter::once(&version_specifiers))
.unwrap()
.unwrap();
let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap();
let wheel_names = &[
"PySocks-1.7.1-py27-none-any.whl",
"black-24.4.2-cp39-cp39-win_amd64.whl",
Expand All @@ -471,9 +468,7 @@ mod tests {
}

let version_specifiers = VersionSpecifiers::from_str(">=3.12.3").unwrap();
let requires_python = RequiresPython::union(std::iter::once(&version_specifiers))
.unwrap()
.unwrap();
let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap();
let wheel_names = &["dearpygui-1.11.1-cp310-cp310-win_amd64.whl"];
for wheel_name in wheel_names {
assert!(
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub(crate) enum ProjectError {
pub(crate) fn find_requires_python(
workspace: &Workspace,
) -> Result<Option<RequiresPython>, uv_resolver::RequiresPythonError> {
RequiresPython::union(workspace.packages().values().filter_map(|member| {
RequiresPython::intersection(workspace.packages().values().filter_map(|member| {
member
.pyproject_toml()
.project
Expand Down
89 changes: 89 additions & 0 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,92 @@ fn package() -> Result<()> {

Ok(())
}

/// Ensure that we use the maximum Python version when a workspace contains mixed requirements.
#[test]
fn mixed_requires_python() -> Result<()> {
let context = TestContext::new_with_versions(&["3.8", "3.12"]);

// Create a workspace root with a minimum Python requirement of Python 3.12.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "albatross"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["bird-feeder", "anyio>3"]
[tool.uv.sources]
bird-feeder = { workspace = true }
[tool.uv.workspace]
members = ["packages/*"]
[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"
"#,
)?;

let src = context.temp_dir.child("src").child("albatross");
src.create_dir_all()?;

let init = src.child("__init__.py");
init.touch()?;

// Create a child with a minimum Python requirement of Python 3.8.
let child = context.temp_dir.child("packages").child("bird-feeder");
child.create_dir_all()?;

let src = context.temp_dir.child("src").child("bird_feeder");
src.create_dir_all()?;

let init = src.child("__init__.py");
init.touch()?;

let pyproject_toml = child.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "bird-feeder"
version = "0.1.0"
requires-python = ">=3.8"
"#,
)?;

// Running `uv sync` should succeed, locking for Python 3.12.
uv_snapshot!(context.filters(), context.sync().arg("-p").arg("3.12"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
warning: `uv.sources` is experimental and may change without warning
Resolved 5 packages in [TIME]
Prepared 5 packages in [TIME]
Installed 5 packages in [TIME]
+ albatross==0.1.0 (from file://[TEMP_DIR]/)
+ anyio==4.3.0
+ bird-feeder==0.1.0 (from file://[TEMP_DIR]/packages/bird-feeder)
+ idna==3.6
+ sniffio==1.3.1
"###);

// Running `uv sync` again should succeed.
uv_snapshot!(context.filters(), context.sync().arg("-p").arg("3.8"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Using Python 3.8.[X] interpreter at: [PYTHON-3.8]
error: The requested Python interpreter (3.8.[X]) is incompatible with the project Python requirement: `>=3.12`
"###);

Ok(())
}

0 comments on commit bf8934e

Please sign in to comment.