Avoid reusing incompatible distributions across lock and sync (#5845)

## Summary

We need to avoid using incompatible versions for build dependencies that
are also part of the resolved
environment. This is a very subtle issue, but: when locking, we don't
enforce platform
compatibility. So, if we reuse the resolver state to install, and the
install itself has to
preform a resolution (e.g., for the build dependencies of a source
distribution), that
resolution may choose incompatible versions.

The key property here is that there's a shared package between the build
dependencies and the
project dependencies.

Closes https://github.com/astral-sh/uv/issues/5836.
This commit is contained in:
Charlie Marsh 2024-08-06 20:54:51 -04:00 committed by GitHub
parent f3cc8e4790
commit cccb6820db
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 89 additions and 20 deletions

View file

@ -259,9 +259,6 @@ pub(crate) async fn add(
.with_pyproject_toml(pyproject.to_toml()?) .with_pyproject_toml(pyproject.to_toml()?)
.context("Failed to update `pyproject.toml`")?; .context("Failed to update `pyproject.toml`")?;
// Initialize any shared state.
let state = SharedState::default();
// Lock and sync the environment, if necessary. // Lock and sync the environment, if necessary.
let lock = match project::lock::do_safe_lock( let lock = match project::lock::do_safe_lock(
locked, locked,
@ -269,7 +266,6 @@ pub(crate) async fn add(
project.workspace(), project.workspace(),
venv.interpreter(), venv.interpreter(),
settings.as_ref().into(), settings.as_ref().into(),
&state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,
@ -387,6 +383,9 @@ pub(crate) async fn add(
} }
}; };
// Initialize any shared state.
let state = SharedState::default();
project::sync::do_sync( project::sync::do_sync(
&project, &project,
&venv, &venv,

View file

@ -84,7 +84,6 @@ pub(crate) async fn lock(
&workspace, &workspace,
&interpreter, &interpreter,
settings.as_ref(), settings.as_ref(),
&SharedState::default(),
preview, preview,
connectivity, connectivity,
concurrency, concurrency,
@ -118,7 +117,6 @@ pub(super) async fn do_safe_lock(
workspace: &Workspace, workspace: &Workspace,
interpreter: &Interpreter, interpreter: &Interpreter,
settings: ResolverSettingsRef<'_>, settings: ResolverSettingsRef<'_>,
state: &SharedState,
preview: PreviewMode, preview: PreviewMode,
connectivity: Connectivity, connectivity: Connectivity,
concurrency: Concurrency, concurrency: Concurrency,
@ -126,6 +124,17 @@ pub(super) async fn do_safe_lock(
cache: &Cache, cache: &Cache,
printer: Printer, printer: Printer,
) -> Result<LockResult, ProjectError> { ) -> Result<LockResult, ProjectError> {
// Use isolate state for universal resolution. When resolving, we don't enforce that the
// prioritized distributions match the current platform. So if we lock here, then try to
// install from the same state, and we end up performing a resolution during the sync (i.e.,
// for the build dependencies of a source distribution), we may try to use incompatible
// distributions.
// TODO(charlie): In universal resolution, we should still track version compatibility! We
// just need to accept versions that are platform-incompatible. That would also make us more
// likely to (e.g.) download a wheel that we'll end up using when installing. This would
// make it safe to share the state.
let state = SharedState::default();
if frozen { if frozen {
// Read the existing lockfile, but don't attempt to lock the project. // Read the existing lockfile, but don't attempt to lock the project.
let existing = read(workspace) let existing = read(workspace)
@ -147,7 +156,7 @@ pub(super) async fn do_safe_lock(
interpreter, interpreter,
Some(&existing), Some(&existing),
settings, settings,
state, &state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,
@ -176,7 +185,7 @@ pub(super) async fn do_safe_lock(
interpreter, interpreter,
existing.as_ref(), existing.as_ref(),
settings, settings,
state, &state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,

View file

@ -104,9 +104,6 @@ pub(crate) async fn remove(
) )
.await?; .await?;
// Initialize any shared state.
let state = SharedState::default();
// Lock and sync the environment, if necessary. // Lock and sync the environment, if necessary.
let lock = project::lock::do_safe_lock( let lock = project::lock::do_safe_lock(
locked, locked,
@ -114,7 +111,6 @@ pub(crate) async fn remove(
project.workspace(), project.workspace(),
venv.interpreter(), venv.interpreter(),
settings.as_ref().into(), settings.as_ref().into(),
&state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,
@ -129,6 +125,9 @@ pub(crate) async fn remove(
let extras = ExtrasSpecification::All; let extras = ExtrasSpecification::All;
let dev = true; let dev = true;
// Initialize any shared state.
let state = SharedState::default();
project::sync::do_sync( project::sync::do_sync(
&VirtualProject::Project(project), &VirtualProject::Project(project),
&venv, &venv,

View file

@ -259,7 +259,6 @@ pub(crate) async fn run(
project.workspace(), project.workspace(),
venv.interpreter(), venv.interpreter(),
settings.as_ref().into(), settings.as_ref().into(),
&state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,

View file

@ -72,16 +72,12 @@ pub(crate) async fn sync(
) )
.await?; .await?;
// Initialize any shared state.
let state = SharedState::default();
let lock = match do_safe_lock( let lock = match do_safe_lock(
locked, locked,
frozen, frozen,
project.workspace(), project.workspace(),
venv.interpreter(), venv.interpreter(),
settings.as_ref().into(), settings.as_ref().into(),
&state,
preview, preview,
connectivity, connectivity,
concurrency, concurrency,
@ -102,6 +98,9 @@ pub(crate) async fn sync(
Err(err) => return Err(err.into()), Err(err) => return Err(err.into()),
}; };
// Initialize any shared state.
let state = SharedState::default();
// Perform the sync operation. // Perform the sync operation.
do_sync( do_sync(
&project, &project,

View file

@ -18,8 +18,6 @@ use crate::commands::{project, ExitStatus};
use crate::printer::Printer; use crate::printer::Printer;
use crate::settings::ResolverSettings; use crate::settings::ResolverSettings;
use super::SharedState;
/// Run a command. /// Run a command.
#[allow(clippy::fn_params_excessive_bools)] #[allow(clippy::fn_params_excessive_bools)]
pub(crate) async fn tree( pub(crate) async fn tree(
@ -72,7 +70,6 @@ pub(crate) async fn tree(
&workspace, &workspace,
&interpreter, &interpreter,
settings.as_ref(), settings.as_ref(),
&SharedState::default(),
preview, preview,
connectivity, connectivity,
concurrency, concurrency,

View file

@ -516,3 +516,70 @@ fn sync_build_isolation() -> Result<()> {
Ok(()) Ok(())
} }
/// Avoid using incompatible versions for build dependencies that are also part of the resolved
/// environment. This is a very subtle issue, but: when locking, we don't enforce platform
/// compatibility. So, if we reuse the resolver state to install, and the install itself has to
/// preform a resolution (e.g., for the build dependencies of a source distribution), that
/// resolution may choose incompatible versions.
///
/// The key property here is that there's a shared package between the build dependencies and the
/// project dependencies.
#[test]
fn sync_reset_state() -> Result<()> {
let context = TestContext::new("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["pydantic-core"]
[build-system]
requires = ["setuptools", "pydantic-core"]
build-backend = "setuptools.build_meta:__legacy__"
"#,
)?;
let setup_py = context.temp_dir.child("setup.py");
setup_py.write_str(indoc::indoc! { r#"
from setuptools import setup
import pydantic_core
setup(
name="project",
version="0.1.0",
packages=["project"],
install_requires=["pydantic-core"],
)
"# })?;
let src = context.temp_dir.child("project");
src.create_dir_all()?;
let init = src.child("__init__.py");
init.touch()?;
// Running `uv sync` should succeed.
uv_snapshot!(context.filters(), context.sync(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Resolved 3 packages in [TIME]
Prepared 3 packages in [TIME]
Installed 3 packages in [TIME]
+ project==0.1.0 (from file://[TEMP_DIR]/)
+ pydantic-core==2.17.0
+ typing-extensions==4.10.0
"###);
assert!(context.temp_dir.child("uv.lock").exists());
Ok(())
}