Skip to content

Commit

Permalink
Re-enable requires-python narrowing in forks (#5583)
Browse files Browse the repository at this point in the history
See: #4669
See: #4668
See: #4902
  • Loading branch information
charliermarsh authored Jul 30, 2024
1 parent 3e2ae93 commit dfa780d
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 53 deletions.
15 changes: 1 addition & 14 deletions crates/uv-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,7 @@ impl PythonRequirement {

/// Narrow the [`PythonRequirement`] to the given version, if it's stricter (i.e., greater)
/// than the current `Requires-Python` minimum.
pub fn narrow(&self, _target: &RequiresPythonBound) -> Option<Self> {
// This represents a "small revert" of the PR that added
// Requires-Python version narrowing[1]. But narrowing has
// led to at least one bug[2] whose fix is not clear. We
// decided to revert narrowing under the idea that it is better
// to be strict (i.e., fail to resolve in some cases, like
// universal_requires_python in uv/tests/pip_compile) than it
// is to output an incorrect lock, as in [2].
//
// [1]: https://github.com/astral-sh/uv/pull/4707
// [2]: https://github.com/astral-sh/uv/issues/4885
None
/*
pub fn narrow(&self, target: &RequiresPythonBound) -> Option<Self> {
let Some(PythonTarget::RequiresPython(requires_python)) = self.target.as_ref() else {
return None;
};
Expand All @@ -71,7 +59,6 @@ impl PythonRequirement {
installed: self.installed.clone(),
target: Some(PythonTarget::RequiresPython(requires_python)),
})
*/
}

/// Return the installed version of Python.
Expand Down
136 changes: 97 additions & 39 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7749,14 +7749,6 @@ fn universal_disjoint_prerelease_requirement() -> Result<()> {

/// Perform a universal resolution that requires narrowing the supported Python range in one of the
/// fork branches.
///
/// Note that this test is currently asserted to be a failed resolution, which is part of
/// a small revert of the PR[1] that added Requires-Python version narrowing. This test
/// should ideally pass, but we aren't sure how to make it pass without producing
/// incorrect answers in other cases[2].
///
/// [1]: https://github.com/astral-sh/uv/pull/4707
/// [2]: https://github.com/astral-sh/uv/issues/4885
#[test]
fn universal_requires_python() -> Result<()> {
let context = TestContext::new("3.12");
Expand All @@ -7771,39 +7763,91 @@ fn universal_requires_python() -> Result<()> {
.arg("-p")
.arg("3.8")
.arg("--universal"), @r###"
success: false
exit_code: 1
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.24.4 ; python_version < '3.9'
# via -r requirements.in
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.
× No solution found when resolving dependencies for split (python_version >= '3.9'):
╰─▶ Because only the following versions of numpy{python_version >= '3.9'} are available:
numpy{python_version >= '3.9'}<=1.26.0
numpy{python_version >= '3.9'}==1.26.1
numpy{python_version >= '3.9'}==1.26.2
numpy{python_version >= '3.9'}==1.26.3
numpy{python_version >= '3.9'}==1.26.4
and the requested Python version (>=3.8) does not satisfy Python>=3.9, we can conclude that any of:
numpy{python_version >= '3.9'}>=1.26.0,<1.26.2
numpy{python_version >= '3.9'}>1.26.2,<1.26.3
numpy{python_version >= '3.9'}>1.26.3,<1.26.4
numpy{python_version >= '3.9'}>1.26.4
are incompatible.
And because the requested Python version (>=3.8) does not satisfy Python>=3.9 and you require numpy{python_version >= '3.9'}>=1.26, we can conclude that the requirements are unsatisfiable.
Resolved 2 packages in [TIME]
"###
);

Ok(())
}

// This test captures a case[1] that was broken by Requires-Python version
// narrowing[2] in the universal resolver. When version narrowing is enabled
// (at time of writing), the `requirements.txt` generated includes several
// duplicate and unconditional dependencies without marker expressions.
//
// [1]: https://github.com/astral-sh/uv/issues/4885
// [2]: https://github.com/astral-sh/uv/pull/4707
/// Perform a universal resolution that requires narrowing the supported Python range in a non-fork.
///
/// This should resolve successfully, but currently fails [1].
///
/// [1]: https://github.com/astral-sh/uv/issues/4668
#[test]
fn universal_requires_python_incomplete() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(indoc::indoc! {r"
uv; python_version >= '3.8'
"})?;

uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile()
.arg("requirements.in")
.arg("-p")
.arg("3.7")
.arg("--universal"), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
warning: The requested Python version 3.7 is not available; 3.12.[X] will be used to build dependencies instead.
× No solution found when resolving dependencies:
╰─▶ Because only the following versions of uv{python_version >= '3.8'} are available:
uv{python_version >= '3.8'}==0.0.5
uv{python_version >= '3.8'}==0.1.0
uv{python_version >= '3.8'}==0.1.1
uv{python_version >= '3.8'}==0.1.2
uv{python_version >= '3.8'}==0.1.3
uv{python_version >= '3.8'}==0.1.4
uv{python_version >= '3.8'}==0.1.5
uv{python_version >= '3.8'}==0.1.6
uv{python_version >= '3.8'}==0.1.7
uv{python_version >= '3.8'}==0.1.8
uv{python_version >= '3.8'}==0.1.9
uv{python_version >= '3.8'}==0.1.10
uv{python_version >= '3.8'}==0.1.11
uv{python_version >= '3.8'}==0.1.12
uv{python_version >= '3.8'}==0.1.13
uv{python_version >= '3.8'}==0.1.14
uv{python_version >= '3.8'}==0.1.15
uv{python_version >= '3.8'}==0.1.16
uv{python_version >= '3.8'}==0.1.17
uv{python_version >= '3.8'}==0.1.18
uv{python_version >= '3.8'}==0.1.19
uv{python_version >= '3.8'}==0.1.20
uv{python_version >= '3.8'}==0.1.21
uv{python_version >= '3.8'}==0.1.22
uv{python_version >= '3.8'}==0.1.23
uv{python_version >= '3.8'}==0.1.24
and the requested Python version (>=3.7) does not satisfy Python>=3.8, we can conclude that all versions of uv{python_version >= '3.8'} are incompatible.
And because you require uv{python_version >= '3.8'}, we can conclude that the requirements are unsatisfiable.
"###
);

Ok(())
}

/// This test captures a case[1] that was broken by Requires-Python version
/// narrowing[2] in the universal resolver, and was later fixed by [3].
///
/// [1]: https://github.com/astral-sh/uv/issues/4885
/// [2]: https://github.com/astral-sh/uv/pull/4707
/// [3]: https://github.com/astral-sh/uv/pull/5597
#[test]
fn universal_no_repeated_unconditional_distributions() -> Result<()> {
let context = TestContext::new("3.12");
Expand All @@ -7823,7 +7867,9 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] requirements.in -p 3.8 --universal
alabaster==0.7.13
alabaster==0.7.13 ; python_version < '3.11'
# via sphinx
alabaster==0.7.16 ; python_version >= '3.11'
# via sphinx
astroid==3.1.0
# via pylint
Expand Down Expand Up @@ -7869,19 +7915,31 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# via sphinx
snowballstemmer==2.2.0
# via sphinx
sphinx==7.1.2
sphinx==7.1.2 ; python_version < '3.11'
# via -r requirements.in
sphinxcontrib-applehelp==1.0.4
sphinx==7.2.6 ; python_version >= '3.11'
# via -r requirements.in
sphinxcontrib-applehelp==1.0.4 ; python_version < '3.11'
# via sphinx
sphinxcontrib-applehelp==1.0.8 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-devhelp==1.0.2 ; python_version < '3.11'
# via sphinx
sphinxcontrib-htmlhelp==2.0.1
sphinxcontrib-devhelp==1.0.6 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-htmlhelp==2.0.1 ; python_version < '3.11'
# via sphinx
sphinxcontrib-htmlhelp==2.0.5 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-jsmath==1.0.1
# via sphinx
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-qthelp==1.0.3 ; python_version < '3.11'
# via sphinx
sphinxcontrib-qthelp==1.0.7 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-serializinghtml==1.1.5 ; python_version < '3.11'
# via sphinx
sphinxcontrib-serializinghtml==1.1.5
sphinxcontrib-serializinghtml==1.1.10 ; python_version >= '3.11'
# via sphinx
tomli==2.0.1 ; python_version < '3.11'
# via pylint
Expand All @@ -7898,7 +7956,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
----- stderr -----
warning: The requested Python version 3.8 is not available; 3.12.[X] will be used to build dependencies instead.
Resolved 34 packages in [TIME]
Resolved 41 packages in [TIME]
"###
);

Expand Down

0 comments on commit dfa780d

Please sign in to comment.