Skip to content

Commit

Permalink
Merge markers when applying constraints (#4648)
Browse files Browse the repository at this point in the history 
## Summary

When a constraint is applied to a requirement with a marker, the marker
needs to be propagated to the constraint.

If both the constraint and the requirement have a marker, they need to
be merged together (via `and`).

Closes #4575.
  • Loading branch information
@charliermarsh
charliermarsh authored Jun 29, 2024
1 parent 0bb9995 commit ea6185e
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 120 deletions.
67 changes: 46 additions & 21 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
dev: Option<&'a GroupName>,
name: Option<&PackageName>,
markers: &'a MarkerTree,
) -> Vec<&'a Requirement> {
) -> Vec<Cow<'a, Requirement>> {
// Start with the requirements for the current extra of the package (for an extra
// requirement) or the non-extra (regular) dependencies (if extra is None), plus
// the constraints for the current package.
Expand All @@ -1227,9 +1227,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
} else {
Either::Right(dependencies.iter())
};
let mut requirements: Vec<&Requirement> = self
let mut requirements = self
.requirements_for_extra(regular_and_dev_dependencies, extra, markers)
.collect();
.collect::<Vec<_>>();

// Check if there are recursive self inclusions and we need to go into the expensive branch.
if !requirements
Expand All @@ -1245,21 +1245,23 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let mut queue: VecDeque<_> = requirements
.iter()
.filter(|req| name == Some(&req.name))
.flat_map(|req| &req.extras)
.flat_map(|req| req.extras.iter().cloned())
.collect();
while let Some(extra) = queue.pop_front() {
if !seen.insert(extra) {
if !seen.insert(extra.clone()) {
continue;
}
// Add each transitively included extra.
queue.extend(
self.requirements_for_extra(dependencies, Some(extra), markers)
.filter(|req| name == Some(&req.name))
.flat_map(|req| &req.extras),
);
// Add the requirements for that extra
requirements.extend(self.requirements_for_extra(dependencies, Some(extra), markers));
for requirement in self.requirements_for_extra(dependencies, Some(&extra), markers) {
if name == Some(&requirement.name) {
// Add each transitively included extra.
queue.extend(requirement.extras.iter().cloned());
} else {
// Add the requirements for that extra.
requirements.push(requirement);
}
}
}

// Drop all the self-requirements now that we flattened them out.
requirements.retain(|req| name != Some(&req.name));

Expand All @@ -1268,12 +1270,15 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag

/// The set of the regular and dev dependencies, filtered by Python version,
/// the markers of this fork and the requested extra.
fn requirements_for_extra<'a>(
&'a self,
dependencies: impl IntoIterator<Item = &'a Requirement>,
extra: Option<&'a ExtraName>,
markers: &'a MarkerTree,
) -> impl Iterator<Item = &'a Requirement> {
fn requirements_for_extra<'data, 'parameters>(
&'data self,
dependencies: impl IntoIterator<Item = &'data Requirement> + 'parameters,
extra: Option<&'parameters ExtraName>,
markers: &'parameters MarkerTree,
) -> impl Iterator<Item = Cow<'data, Requirement>> + 'parameters
where
'data: 'parameters,
{
self.overrides
.apply(dependencies)
.filter(move |requirement| {
Expand Down Expand Up @@ -1322,7 +1327,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
true
})
.flat_map(move |requirement| {
iter::once(requirement).chain(
iter::once(Cow::Borrowed(requirement)).chain(
self.constraints
.get(&requirement.name)
.into_iter()
Expand Down Expand Up @@ -1359,7 +1364,27 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}

true
}),
})
.map(move |constraint| {
// If the requirement is `requests ; sys_platform == 'darwin'` and the
// constraint is `requests ; python_version == '3.6'`, the constraint
// should only apply when _both_ markers are true.
if let Some(marker) = requirement.marker.as_ref() {
let marker = constraint.marker.as_ref().map(|m| {
MarkerTree::And(vec![marker.clone(), m.clone()])
}).or_else(|| Some(marker.clone()));

Cow::Owned(Requirement {
name: constraint.name.clone(),
extras: constraint.extras.clone(),
source: constraint.source.clone(),
origin: constraint.origin.clone(),
marker
})
} else {
Cow::Borrowed(constraint)
}
})
)
})
}
Expand Down
247 changes: 148 additions & 99 deletions crates/uv/tests/pip_compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6390,7 +6390,8 @@ fn universal_cycles() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
poetry
testtools==2.3.0
fixtures==3.0.0
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
Expand All @@ -6401,114 +6402,162 @@ fn universal_cycles() -> Result<()> {
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] requirements.in --universal
build==1.1.1
# via poetry
cachecontrol==0.14.0
# via poetry
certifi==2024.2.2
# via requests
cffi==1.16.0 ; sys_platform == 'darwin' or (platform_python_implementation != 'PyPy' and sys_platform == 'linux')
# via
# cryptography
# xattr
charset-normalizer==3.3.2
# via requests
cleo==2.1.0
# via poetry
colorama==0.4.6 ; os_name == 'nt'
# via build
crashtest==0.4.1
# via
# cleo
# poetry
cryptography==42.0.5 ; sys_platform == 'linux'
# via secretstorage
distlib==0.3.8
# via virtualenv
dulwich==0.21.7
# via poetry
fastjsonschema==2.19.1
# via poetry
filelock==3.13.1
argparse==1.4.0
# via unittest2
extras==1.0.0
# via testtools
fixtures==3.0.0
# via
# cachecontrol
# virtualenv
idna==3.6
# via requests
installer==0.7.0
# via poetry
jaraco-classes==3.3.1
# via keyring
jeepney==0.8.0 ; sys_platform == 'linux'
# via
# keyring
# secretstorage
keyring==24.3.1
# via poetry
more-itertools==10.2.0
# via jaraco-classes
msgpack==1.0.8
# via cachecontrol
packaging==24.0
# -r requirements.in
# testtools
linecache2==1.0.0
# via traceback2
pbr==6.0.0
# via
# build
# poetry
pexpect==4.9.0
# via poetry
pkginfo==1.10.0
# via poetry
platformdirs==4.2.0
# fixtures
# testtools
python-mimeparse==1.6.0
# via testtools
six==1.16.0
# via
# poetry
# virtualenv
poetry==1.8.2
# fixtures
# testtools
# unittest2
testtools==2.3.0
# via
# -r requirements.in
# poetry-plugin-export
poetry-core==1.9.0
# via
# poetry
# poetry-plugin-export
poetry-plugin-export==1.7.1
# via poetry
ptyprocess==0.7.0
# via pexpect
pycparser==2.21 ; sys_platform == 'darwin' or (platform_python_implementation != 'PyPy' and sys_platform == 'linux')
# via cffi
pyproject-hooks==1.0.0
# fixtures
traceback2==1.4.0
# via
# build
# poetry
pywin32-ctypes==0.2.2 ; sys_platform == 'win32'
# via keyring
rapidfuzz==3.7.0
# via cleo
requests==2.31.0
# testtools
# unittest2
unittest2==1.1.0
# via testtools
----- stderr -----
Resolved 10 packages in [TIME]
"###
);

Ok(())
}

/// Perform a universal resolution with a constraint.
#[test]
fn universal_constraint() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
anyio ; sys_platform == 'win32'
"})?;

let constraints_txt = context.temp_dir.child("constraints.txt");
constraints_txt.write_str(indoc::indoc! {r"
anyio==3.0.0
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
.arg("requirements.in")
.arg("-c")
.arg("constraints.txt")
.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 -c constraints.txt --universal
anyio==3.0.0 ; sys_platform == 'win32'
# via
# cachecontrol
# poetry
# requests-toolbelt
requests-toolbelt==1.0.0
# via poetry
secretstorage==3.3.3 ; sys_platform == 'linux'
# via keyring
shellingham==1.5.4
# via poetry
tomlkit==0.12.4
# via poetry
trove-classifiers==2024.3.3
# via poetry
urllib3==2.2.1
# -c constraints.txt
# -r requirements.in
idna==3.6 ; sys_platform == 'win32'
# via anyio
sniffio==1.3.1 ; sys_platform == 'win32'
# via anyio
----- stderr -----
Resolved 3 packages in [TIME]
"###
);

Ok(())
}

/// Perform a universal resolution with a constraint, where the constraint itself has a marker.
#[test]
fn universal_constraint_marker() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
anyio ; sys_platform == 'win32'
"})?;

let constraints_txt = context.temp_dir.child("constraints.txt");
constraints_txt.write_str(indoc::indoc! {r"
anyio==3.0.0 ; os_name == 'nt'
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
.arg("requirements.in")
.arg("-c")
.arg("constraints.txt")
.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 -c constraints.txt --universal
anyio==3.0.0 ; sys_platform == 'win32' or (os_name == 'nt' and sys_platform == 'win32')
# via
# dulwich
# requests
virtualenv==20.25.1
# via poetry
xattr==1.1.0 ; sys_platform == 'darwin'
# via poetry
# -c constraints.txt
# -r requirements.in
idna==3.6 ; sys_platform == 'win32' or (os_name == 'nt' and sys_platform == 'win32')
# via anyio
sniffio==1.3.1 ; sys_platform == 'win32' or (os_name == 'nt' and sys_platform == 'win32')
# via anyio
----- stderr -----
Resolved 3 packages in [TIME]
"###
);

Ok(())
}

/// Perform a universal resolution with a divergent requirement, and a third requirement that's
/// compatible with both forks.
///
/// This currently fails, but should succeed.
///
/// See: <https://github.com/astral-sh/uv/issues/4640>
#[test]
fn universal_multi_version() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
iniconfig
iniconfig==2.0.0 ; python_version > '3.12'
iniconfig==1.0.0 ; python_version == '3.12'
"})?;

let constraints_txt = context.temp_dir.child("constraints.txt");
constraints_txt.write_str(indoc::indoc! {r"
anyio==3.0.0 ; os_name == 'nt'
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
.arg("requirements.in")
.arg("-c")
.arg("constraints.txt")
.arg("--universal"), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
Resolved 41 packages in [TIME]
× No solution found when resolving dependencies:
╰─▶ Because iniconfig{python_version > '3.12'}==2.0.0 depends on iniconfig==2.0.0 and you require iniconfig{python_version > '3.12'}==2.0.0, we can conclude that your requirements and iniconfig{python_version == '3.12'}==1.0.0 are incompatible.
And because you require iniconfig{python_version == '3.12'}==1.0.0, we can conclude that the requirements are unsatisfiable.
"###
);

Expand Down

0 comments on commit ea6185e

Please sign in to comment.