From 584a793935f67a6e466eb8d954621290a728435b Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Mon, 8 Jul 2024 11:55:13 -0400 Subject: [PATCH] uv-resolver: partially revert Requires-Python version narrowing The PR #4707 introduced the notion of "version narrowing," where a Requires-Python constraint was _possibly_ narrowed whenever the universal resolver created a fork. The version narrowing would occur when the fork was a result of a marker expression on `python_version` that is *stricter* than the configured `Requires-Python` (via, say, `pyproject.toml`). The crucial conceptual change made by #4707 is therefore that `Requires-Python` is no longer an invariant configuration of resolution, but rather a mutable constraint that can vary from fork to fork. This in turn can result in some cases, such as in #4885, where different versions of dependencies are selected. We aren't sure whether we can fix those or not, with version narrowing, so for now, we do this revert to restore the previous behavior and we'll try to address the version narrowing some other time. This also adds the case from #4885 as a regression test, ensuring that we don't break that in the future. I confirmed that with version narrowing, this test outputs duplicate distributions. Without narrowing, there are no duplicates. Ref #4707, Fixes #4885 --- crates/uv-resolver/src/python_requirement.rs | 15 ++- crates/uv/tests/pip_compile.rs | 129 ++++++++++++++++++- 2 files changed, 140 insertions(+), 4 deletions(-) diff --git a/crates/uv-resolver/src/python_requirement.rs b/crates/uv-resolver/src/python_requirement.rs index 66cc7c9aa96e9..caabeabf77c9a 100644 --- a/crates/uv-resolver/src/python_requirement.rs +++ b/crates/uv-resolver/src/python_requirement.rs @@ -50,7 +50,19 @@ 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 { + pub fn narrow(&self, _target: &RequiresPythonBound) -> Option { + // 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 + /* let Some(PythonTarget::RequiresPython(requires_python)) = self.target.as_ref() else { return None; }; @@ -59,6 +71,7 @@ impl PythonRequirement { installed: self.installed.clone(), target: Some(PythonTarget::RequiresPython(requires_python)), }) + */ } /// Return the installed version of Python. diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index c9db3e3556575..3d20c295f0f68 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -6624,6 +6624,14 @@ fn universal_multi_version() -> 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"); @@ -6633,6 +6641,53 @@ fn universal_requires_python() -> Result<()> { 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: false + exit_code: 1 + ----- stdout ----- + + ----- 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: + ╰─▶ 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. + "### + ); + + 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 +#[test] +fn universal_no_repeated_unconditional_distributions() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r" + pylint + sphinx + "})?; + uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile() .arg("requirements.in") .arg("-p") @@ -6643,14 +6698,82 @@ fn universal_requires_python() -> 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 - numpy==1.24.4 ; python_version < '3.9' + alabaster==0.7.13 + # via sphinx + astroid==3.1.0 + # via pylint + babel==2.14.0 + # via sphinx + certifi==2024.2.2 + # via requests + charset-normalizer==3.3.2 + # via requests + colorama==0.4.6 ; sys_platform == 'win32' + # via + # pylint + # sphinx + dill==0.3.8 + # via pylint + docutils==0.20.1 + # via sphinx + idna==3.6 + # via requests + imagesize==1.4.1 + # via sphinx + importlib-metadata==7.1.0 ; python_version < '3.10' + # via sphinx + isort==5.13.2 + # via pylint + jinja2==3.1.3 + # via sphinx + markupsafe==2.1.5 + # via jinja2 + mccabe==0.7.0 + # via pylint + packaging==24.0 + # via sphinx + platformdirs==4.2.0 + # via pylint + pygments==2.17.2 + # via sphinx + pylint==3.1.0 # via -r requirements.in - numpy==1.26.4 ; python_version >= '3.9' + pytz==2024.1 ; python_version < '3.9' + # via babel + requests==2.31.0 + # via sphinx + snowballstemmer==2.2.0 + # via sphinx + sphinx==7.1.2 # via -r requirements.in + sphinxcontrib-applehelp==1.0.4 + # via sphinx + sphinxcontrib-devhelp==1.0.2 + # via sphinx + sphinxcontrib-htmlhelp==2.0.1 + # via sphinx + sphinxcontrib-jsmath==1.0.1 + # via sphinx + sphinxcontrib-qthelp==1.0.3 + # via sphinx + sphinxcontrib-serializinghtml==1.1.5 + # via sphinx + tomli==2.0.1 ; python_version < '3.11' + # via pylint + tomlkit==0.12.4 + # via pylint + typing-extensions==4.10.0 ; python_version < '3.11' + # via + # astroid + # pylint + urllib3==2.2.1 + # via requests + zipp==3.18.1 ; python_version < '3.10' + # via importlib-metadata ----- stderr ----- warning: The requested Python version 3.8 is not available; 3.12.[X] will be used to build dependencies instead. - Resolved 2 packages in [TIME] + Resolved 34 packages in [TIME] "### );