Skip to content

Commit

Permalink
Fork resolution when Python requirement is narrowed
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 1, 2024
1 parent 7191a09 commit a0b7f87
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 22 deletions.
55 changes: 34 additions & 21 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
.map(ToString::to_string)
.join(", ")
);
assert!(forks.len() >= 2);

// This is a somewhat tortured technique to ensure
// that our resolver state is only cloned as much
// as it needs to be. We basically move the state
Expand All @@ -576,23 +576,21 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
.unwrap_or(MarkerTree::And(Vec::new()));

// If the fork contains a narrowed Python requirement, apply it.
if let Some(python_version) =
requires_python_marker(&forked_state.markers)
{
if let Some(python_requirement) =
forked_state.python_requirement.narrow(&python_version)
{
if let Some(target) = python_requirement.target() {
debug!("Narrowed `requires-python` bound to: {target}");
}
forked_state.requires_python =
if forked_state.requires_python.is_some() {
python_requirement.to_marker_tree()
} else {
None
};
forked_state.python_requirement = python_requirement;
let python_requirement = requires_python_marker(
&forked_state.markers,
)
.and_then(|marker| forked_state.python_requirement.narrow(&marker));
if let Some(python_requirement) = python_requirement {
if let Some(target) = python_requirement.target() {
debug!("Narrowed `requires-python` bound to: {target}");
}
forked_state.requires_python =
if forked_state.requires_python.is_some() {
python_requirement.to_marker_tree()
} else {
None
};
forked_state.python_requirement = python_requirement;
}

forked_state.add_package_version_dependencies(
Expand Down Expand Up @@ -2424,7 +2422,6 @@ impl Dependencies {
continue;
}
};
assert!(fork_groups.forks.len() >= 2, "expected definitive fork");
let mut new_forks: Vec<Fork> = vec![];
for group in fork_groups.forks {
let mut new_forks_for_group = forks.clone();
Expand Down Expand Up @@ -2602,7 +2599,7 @@ impl<'a> PossibleForks<'a> {
let PossibleForks::PossiblyForking(ref fork_groups) = *self else {
return false;
};
fork_groups.forks.len() > 1
fork_groups.has_fork()
}

/// Consumes this possible set of forks and converts a "possibly forking"
Expand All @@ -2615,9 +2612,8 @@ impl<'a> PossibleForks<'a> {
let PossibleForks::PossiblyForking(ref fork_groups) = self else {
return self;
};
if fork_groups.forks.len() == 1 {
if !fork_groups.has_fork() {
self.make_no_forks_possible();
return self;
}
self
}
Expand Down Expand Up @@ -2680,6 +2676,23 @@ impl<'a> PossibleForkGroups<'a> {
.iter_mut()
.find(|fork| fork.is_overlapping(marker))
}

/// Returns `true` if the fork group has a fork.
fn has_fork(&self) -> bool {
if self.forks.len() > 1 {
return true;
}

if self.forks.iter().any(|fork| {
fork.packages
.iter()
.any(|(_, markers)| requires_python_marker(markers).is_some())
}) {
return true;
}

false
}
}

/// Intermediate state representing a single possible fork.
Expand Down
34 changes: 33 additions & 1 deletion crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6567,7 +6567,7 @@ fn universal_multi_version() -> Result<()> {
/// Perform a universal resolution that requires narrowing the supported Python range in one of the
/// fork branches.
#[test]
fn universal_requires_python() -> Result<()> {
fn universal_requires_python_fork() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
Expand Down Expand Up @@ -6599,6 +6599,38 @@ fn universal_requires_python() -> Result<()> {
Ok(())
}

/// Perform a universal resolution that requires narrowing the supported Python range without
/// forking.
#[test]
fn universal_requires_python_non_fork() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
numpy >=1.26 ; python_version >= '3.9'
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
.arg("requirements.in")
.arg("-p")
.arg("3.8")
.arg("--universal"), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] requirements.in -p 3.8 --universal
numpy==1.26.4 ; python_version >= '3.9'
# via -r requirements.in
----- stderr -----
warning: The requested Python version 3.8 is not available; 3.12.[X] will be used to build dependencies instead.
Resolved 1 package in [TIME]
"###
);

Ok(())
}

/// Resolve a package from a `requirements.in` file, with a `constraints.txt` file pinning one of
/// its transitive dependencies to a specific version.
#[test]
Expand Down

0 comments on commit a0b7f87

Please sign in to comment.