Skip to content

Commit

Permalink
fix bug where python_version < '0' could appear in a final resoluti…
Browse files Browse the repository at this point in the history 
…on (#8759)

This PR fixes a bug where it was possible for dependencies to be
included in a final resolution with markers that always evaluate to
false. Specifically, `python_version < '0'`.

While we do filter based on Python markers during forking, it turns out
that the markers for each fork are "combined" *after* this filtering
step. But the process of combination can result in a more specific
marker that is always false for the configured Python requirement. This
could result in dependencies with markers that are always false (like
`python_version < '0'`) appearing in the resolution.

The first commit in this PR adds a regression test (with an undesirable
result), and the second commit fixes the regression and updates the
test.

Fixes #8676
  • Loading branch information
@BurntSushi
BurntSushi authored Nov 1, 2024
1 parent a90a8e7 commit 58a9811
Show file tree
Hide file tree
Showing 2 changed files with 262 additions and 5 deletions.
23 changes: 18 additions & 5 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
fork_preferences
.iter()
.rev()
.map(|fork_preference| state.clone().with_markers(fork_preference.clone()))
.filter_map(|fork_preference| {
state.clone().with_markers(fork_preference.clone())
})
.collect()
}
} else {
Expand Down Expand Up @@ -693,14 +695,17 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
forks
.into_iter()
.enumerate()
.map(move |(i, fork)| {
.filter_map(move |(i, fork)| {
let is_last = i == forks_len - 1;
let forked_state = cur_state.take().unwrap();
if !is_last {
cur_state = Some(forked_state.clone());
}

let mut forked_state = forked_state.with_markers(fork.markers);
let markers = fork.markers.clone();
Some((fork, forked_state.with_markers(markers)?))
})
.map(move |(fork, mut forked_state)| {
forked_state.add_package_version_dependencies(
for_package,
version,
Expand Down Expand Up @@ -2320,8 +2325,16 @@ impl ForkState {

/// Subset the current markers with the new markers and update the python requirements fields
/// accordingly.
fn with_markers(mut self, markers: MarkerTree) -> Self {
fn with_markers(mut self, markers: MarkerTree) -> Option<Self> {
let combined_markers = self.markers.and(markers);
let python_marker = self.python_requirement.to_marker_tree();
if combined_markers.is_disjoint(&python_marker) {
debug!(
"Skipping split {combined_markers:?} \
because of Python requirement {python_marker:?}",
);
return None;
}

// If the fork contains a narrowed Python requirement, apply it.
let python_requirement = marker::requires_python(&combined_markers)
Expand All @@ -2335,7 +2348,7 @@ impl ForkState {
}

self.markers = ResolverMarkers::Fork(combined_markers);
self
Some(self)
}

fn into_resolution(self) -> Resolution {
Expand Down
244 changes: 244 additions & 0 deletions crates/uv/tests/it/pip_compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,250 @@ fn compile_constraint_extra() -> Result<()> {
Ok(())
}

/// This is a regression test for a case where `uv pip compile --universal`
/// would include dependencies with marker expressions that always evaluate
/// to false (for example, `python_version < '0'`).
///
/// See: <https://github.com/astral-sh/uv/issues/8676>
#[test]
fn compile_constraints_omit_impossible_dependencies() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str(
"\
apache-airflow[microsoft.azure]==2.3.4
scikit-learn==1.2.2
ipython>=8.4.0
mypy
black
",
)?;

let constraints_txt = context.temp_dir.child("constraints.txt");
constraints_txt.write_str(
"\
apache-airflow-providers-microsoft-azure==4.2.0
attrs==22.1.0
azure-identity==1.10.0
click==8.1.3
colorama==0.4.5
markdown-it-py==2.1.0
msal-extensions==1.0.0
msrestazure==0.6.4
numpy==1.22.4
packaging==21.3
portalocker==2.5.1
typing_extensions==4.3.0
",
)?;

let filters: Vec<_> = [
// 3.10 may not be installed
(
"warning: The requested Python version 3.10 is not available; .* will be used to build dependencies instead.\n",
"",
),
// These aren't used on Windows, so we filter them out.
(".*colorama==.*\n", ""),
(".*tzdata==.*\n", ""),
]
.into_iter()
.chain(context.filters())
.collect();

uv_snapshot!(filters, context.pip_compile()
.arg("requirements.in")
.arg("--constraint")
.arg("constraints.txt")
.arg("--annotation-style")
.arg("line")
.arg("--python-version")
.arg("3.10")
.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 --constraint constraints.txt --annotation-style line --python-version 3.10 --universal
a2wsgi==1.10.4 # via connexion
adal==1.2.7 # via azure-kusto-data, msrestazure
aiohttp==3.9.3 # via apache-airflow-providers-http
aiosignal==1.3.1 # via aiohttp
alembic==1.13.1 # via apache-airflow
anyio==4.3.0 # via httpx, starlette
apache-airflow==2.3.4 # via apache-airflow-providers-microsoft-azure, -r requirements.in
apache-airflow-providers-common-sql==1.4.0 # via apache-airflow-providers-sqlite
apache-airflow-providers-ftp==3.3.1 # via apache-airflow
apache-airflow-providers-http==4.3.0 # via apache-airflow
apache-airflow-providers-imap==3.1.1 # via apache-airflow
apache-airflow-providers-microsoft-azure==4.2.0 # via apache-airflow, -c constraints.txt
apache-airflow-providers-sqlite==3.3.2 # via apache-airflow
apispec==3.3.2 # via flask-appbuilder
argcomplete==3.2.3 # via apache-airflow
asgiref==3.8.1 # via apache-airflow-providers-http, connexion, flask
asttokens==2.4.1 # via stack-data
async-timeout==4.0.3 ; python_full_version < '3.11' # via aiohttp
attrs==22.1.0 # via aiohttp, cattrs, jsonschema, -c constraints.txt
azure-batch==14.1.0 # via apache-airflow-providers-microsoft-azure
azure-common==1.1.28 # via azure-batch, azure-keyvault-secrets, azure-mgmt-containerinstance, azure-mgmt-datafactory, azure-mgmt-datalake-store, azure-mgmt-resource, azure-storage-common, azure-storage-file
azure-core==1.29.1 # via azure-cosmos, azure-identity, azure-keyvault-secrets, azure-mgmt-core, azure-servicebus, azure-storage-blob, msrest
azure-cosmos==4.5.1 # via apache-airflow-providers-microsoft-azure
azure-datalake-store==0.0.53 # via apache-airflow-providers-microsoft-azure
azure-identity==1.10.0 # via apache-airflow-providers-microsoft-azure, -c constraints.txt
azure-keyvault-secrets==4.7.0 # via apache-airflow-providers-microsoft-azure
azure-kusto-data==0.0.45 # via apache-airflow-providers-microsoft-azure
azure-mgmt-containerinstance==1.5.0 # via apache-airflow-providers-microsoft-azure
azure-mgmt-core==1.4.0 # via azure-mgmt-datafactory, azure-mgmt-resource
azure-mgmt-datafactory==1.1.0 # via apache-airflow-providers-microsoft-azure
azure-mgmt-datalake-nspkg==3.0.1 # via azure-mgmt-datalake-store
azure-mgmt-datalake-store==0.5.0 # via apache-airflow-providers-microsoft-azure
azure-mgmt-nspkg==3.0.2 # via azure-mgmt-datalake-nspkg
azure-mgmt-resource==23.0.1 # via apache-airflow-providers-microsoft-azure
azure-nspkg==3.0.2 # via azure-mgmt-nspkg
azure-servicebus==7.12.1 ; platform_machine != 'aarch64' # via apache-airflow-providers-microsoft-azure
azure-storage-blob==12.8.1 # via apache-airflow-providers-microsoft-azure
azure-storage-common==2.1.0 # via apache-airflow-providers-microsoft-azure, azure-storage-file
azure-storage-file==2.1.0 # via apache-airflow-providers-microsoft-azure
babel==2.14.0 # via flask-babel
black==22.12.0 # via -r requirements.in
blinker==1.7.0 # via apache-airflow
cachelib==0.9.0 # via flask-caching
cattrs==23.1.2 # via apache-airflow
certifi==2024.2.2 # via httpcore, httpx, msrest, requests
cffi==1.16.0 # via azure-datalake-store, cryptography
charset-normalizer==3.3.2 # via requests
click==8.1.3 # via black, flask, flask-appbuilder, -c constraints.txt
colorlog==4.8.0 # via apache-airflow
connexion==3.0.6 # via apache-airflow
cron-descriptor==1.4.3 # via apache-airflow
croniter==2.0.3 # via apache-airflow
cryptography==42.0.5 # via adal, apache-airflow, azure-identity, azure-storage-blob, azure-storage-common, msal, pyjwt
decorator==5.1.1 # via ipython
deprecated==1.2.14 # via apache-airflow
dill==0.3.8 # via apache-airflow
dnspython==2.6.1 # via email-validator
docutils==0.20.1 # via python-daemon
email-validator==1.3.1 # via flask-appbuilder
exceptiongroup==1.2.0 ; python_full_version < '3.11' # via anyio, cattrs, ipython
executing==2.0.1 # via stack-data
flask==2.2.5 # via apache-airflow, connexion, flask-appbuilder, flask-babel, flask-caching, flask-jwt-extended, flask-login, flask-session, flask-sqlalchemy, flask-wtf
flask-appbuilder==4.1.3 # via apache-airflow
flask-babel==2.0.0 # via flask-appbuilder
flask-caching==2.1.0 # via apache-airflow
flask-jwt-extended==4.6.0 # via flask-appbuilder
flask-login==0.6.3 # via apache-airflow, flask-appbuilder
flask-session==0.7.0 # via apache-airflow
flask-sqlalchemy==2.5.1 # via flask-appbuilder
flask-wtf==0.15.1 # via apache-airflow, flask-appbuilder
frozenlist==1.4.1 # via aiohttp, aiosignal
graphviz==0.20.3 # via apache-airflow
greenlet==3.0.3 ; platform_machine == 'AMD64' or platform_machine == 'WIN32' or platform_machine == 'aarch64' or platform_machine == 'amd64' or platform_machine == 'ppc64le' or platform_machine == 'win32' or platform_machine == 'x86_64' # via sqlalchemy
gunicorn==21.2.0 # via apache-airflow
h11==0.14.0 # via httpcore
httpcore==1.0.4 # via httpx
httpx==0.27.0 # via apache-airflow, connexion
idna==3.6 # via anyio, email-validator, httpx, requests, yarl
inflection==0.5.1 # via connexion
ipython==8.22.2 # via -r requirements.in
isodate==0.6.1 # via azure-keyvault-secrets, azure-mgmt-resource, azure-servicebus, msrest
itsdangerous==2.1.2 # via apache-airflow, flask, flask-wtf
jedi==0.19.1 # via ipython
jinja2==3.1.3 # via apache-airflow, connexion, flask, flask-babel, python-nvd3, swagger-ui-bundle
joblib==1.3.2 # via scikit-learn
jsonschema==4.17.3 # via apache-airflow, connexion, flask-appbuilder
lazy-object-proxy==1.10.0 # via apache-airflow
linkify-it-py==2.0.3 # via apache-airflow
lockfile==0.12.2 # via apache-airflow, python-daemon
mako==1.3.2 # via alembic
markdown==3.6 # via apache-airflow
markdown-it-py==2.1.0 # via apache-airflow, mdit-py-plugins, rich, -c constraints.txt
markupsafe==2.1.5 # via apache-airflow, jinja2, mako, werkzeug, wtforms
marshmallow==3.21.1 # via flask-appbuilder, marshmallow-enum, marshmallow-oneofschema, marshmallow-sqlalchemy
marshmallow-enum==1.5.1 # via flask-appbuilder
marshmallow-oneofschema==3.1.1 # via apache-airflow
marshmallow-sqlalchemy==0.26.1 # via flask-appbuilder
matplotlib-inline==0.1.6 # via ipython
mdit-py-plugins==0.4.0 # via apache-airflow
mdurl==0.1.2 # via markdown-it-py
msal==1.28.0 # via azure-datalake-store, azure-identity, msal-extensions
msal-extensions==1.0.0 # via azure-identity, -c constraints.txt
msgspec==0.18.6 # via flask-session
msrest==0.7.1 # via azure-mgmt-containerinstance, azure-mgmt-datafactory, azure-storage-blob, msrestazure
msrestazure==0.6.4 # via azure-batch, azure-kusto-data, azure-mgmt-containerinstance, azure-mgmt-datalake-store, -c constraints.txt
multidict==6.0.5 # via aiohttp, yarl
mypy==1.9.0 # via -r requirements.in
mypy-extensions==1.0.0 # via black, mypy
numpy==1.22.4 # via scikit-learn, scipy, -c constraints.txt
oauthlib==3.2.2 # via requests-oauthlib
packaging==21.3 # via apache-airflow, gunicorn, marshmallow, -c constraints.txt
parso==0.8.3 # via jedi
pathspec==0.9.0 # via apache-airflow, black
pendulum==3.0.0 # via apache-airflow
pexpect==4.9.0 ; sys_platform != 'emscripten' and sys_platform != 'win32' # via ipython
platformdirs==4.2.0 # via black
pluggy==1.4.0 # via apache-airflow
portalocker==2.5.1 # via msal-extensions, -c constraints.txt
prison==0.2.1 # via flask-appbuilder
prompt-toolkit==3.0.43 # via ipython
psutil==5.9.8 # via apache-airflow
ptyprocess==0.7.0 ; sys_platform != 'emscripten' and sys_platform != 'win32' # via pexpect
pure-eval==0.2.2 # via stack-data
pycparser==2.21 # via cffi
pygments==2.17.2 # via apache-airflow, ipython, rich
pyjwt==2.8.0 # via adal, apache-airflow, flask-appbuilder, flask-jwt-extended, msal
pyparsing==3.1.2 # via packaging
pyrsistent==0.20.0 # via jsonschema
python-daemon==3.0.1 # via apache-airflow
python-dateutil==2.9.0.post0 # via adal, apache-airflow, azure-kusto-data, azure-storage-common, croniter, flask-appbuilder, pendulum, time-machine
python-multipart==0.0.9 # via connexion
python-nvd3==0.15.0 # via apache-airflow
python-slugify==8.0.4 # via apache-airflow, python-nvd3
pytz==2024.1 # via croniter, flask-babel
pywin32==306 ; platform_system == 'Windows' # via portalocker
pyyaml==6.0.1 # via apispec, connexion
requests==2.31.0 # via adal, apache-airflow-providers-http, azure-core, azure-datalake-store, azure-kusto-data, azure-storage-common, connexion, msal, msrest, requests-oauthlib, requests-toolbelt
requests-oauthlib==2.0.0 # via msrest
requests-toolbelt==1.0.0 # via apache-airflow-providers-http
rich==13.3.1 # via apache-airflow
scikit-learn==1.2.2 # via -r requirements.in
scipy==1.12.0 # via scikit-learn
setproctitle==1.3.3 # via apache-airflow
setuptools==69.2.0 # via python-daemon
six==1.16.0 # via asttokens, azure-core, azure-identity, isodate, msrestazure, prison, python-dateutil
sniffio==1.3.1 # via anyio, httpx
sqlalchemy==1.4.52 # via alembic, apache-airflow, flask-appbuilder, flask-sqlalchemy, marshmallow-sqlalchemy, sqlalchemy-jsonfield, sqlalchemy-utils
sqlalchemy-jsonfield==1.0.2 # via apache-airflow
sqlalchemy-utils==0.41.2 # via flask-appbuilder
sqlparse==0.4.4 # via apache-airflow-providers-common-sql
stack-data==0.6.3 # via ipython
starlette==0.37.2 # via connexion
swagger-ui-bundle==1.1.0 # via connexion
tabulate==0.9.0 # via apache-airflow
tenacity==8.2.3 # via apache-airflow
termcolor==2.4.0 # via apache-airflow
text-unidecode==1.3 # via python-slugify
threadpoolctl==3.4.0 # via scikit-learn
time-machine==2.14.1 ; implementation_name != 'pypy' # via pendulum
tomli==2.0.1 ; python_full_version < '3.11' # via black, mypy
traitlets==5.14.2 # via ipython, matplotlib-inline
typing-extensions==4.3.0 # via alembic, anyio, apache-airflow, asgiref, azure-core, azure-keyvault-secrets, azure-servicebus, cattrs, connexion, mypy, -c constraints.txt
uc-micro-py==1.0.3 # via linkify-it-py
unicodecsv==0.14.1 # via apache-airflow
urllib3==2.2.1 # via requests
wcwidth==0.2.13 # via prompt-toolkit
werkzeug==3.0.1 # via apache-airflow, connexion, flask, flask-jwt-extended, flask-login
wrapt==1.16.0 # via deprecated
wtforms==2.3.3 # via flask-appbuilder, flask-wtf
yarl==1.9.4 # via aiohttp
----- stderr -----
Resolved 172 packages in [TIME]
"###
);

Ok(())
}

/// Resolve a package from an optional extra in a `pyproject.toml` file.
#[test]
fn compile_pyproject_toml_extra() -> Result<()> {
Expand Down

0 comments on commit 58a9811

Please sign in to comment.