Skip to content

Commit

Permalink
Respect lockfile preferences for --with requirements (#7627)
Browse files Browse the repository at this point in the history
## Summary

This is a long-time TODO to respect versions from the base environment
when resolving `--with` requirements.

Closes #7416
  • Loading branch information
charliermarsh authored Sep 23, 2024
1 parent ab2bba2 commit ff066c8
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 35 deletions.
18 changes: 9 additions & 9 deletions crates/uv/src/commands/project/environment.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
use tracing::debug;

use crate::commands::pip::loggers::{InstallLogger, ResolveLogger};
use crate::commands::project::{
resolve_environment, sync_environment, EnvironmentSpecification, ProjectError,
};
use crate::commands::SharedState;
use crate::printer::Printer;
use crate::settings::ResolverInstallerSettings;
use cache_key::{cache_digest, hash_digest};
use distribution_types::Resolution;
use uv_cache::{Cache, CacheBucket};
use uv_client::Connectivity;
use uv_configuration::Concurrency;
use uv_python::{Interpreter, PythonEnvironment};
use uv_requirements::RequirementsSpecification;

use crate::commands::pip::loggers::{InstallLogger, ResolveLogger};
use crate::commands::project::{resolve_environment, sync_environment, ProjectError};
use crate::commands::SharedState;
use crate::printer::Printer;
use crate::settings::ResolverInstallerSettings;

/// A [`PythonEnvironment`] stored in the cache.
#[derive(Debug)]
Expand All @@ -28,7 +28,7 @@ impl CachedEnvironment {
/// Get or create an [`CachedEnvironment`] based on a given set of requirements and a base
/// interpreter.
pub(crate) async fn get_or_create(
spec: RequirementsSpecification,
spec: EnvironmentSpecification<'_>,
interpreter: Interpreter,
settings: &ResolverInstallerSettings,
state: &SharedState,
Expand Down Expand Up @@ -58,8 +58,8 @@ impl CachedEnvironment {

// Resolve the requirements with the interpreter.
let graph = resolve_environment(
&interpreter,
spec,
&interpreter,
settings.as_ref().into(),
state,
resolve,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ async fn commit(lock: &Lock, workspace: &Workspace) -> Result<(), ProjectError>
/// Read the lockfile from the workspace.
///
/// Returns `Ok(None)` if the lockfile does not exist.
async fn read(workspace: &Workspace) -> Result<Option<Lock>, ProjectError> {
pub(crate) async fn read(workspace: &Workspace) -> Result<Option<Lock>, ProjectError> {
match fs_err::tokio::read_to_string(&workspace.install_path().join("uv.lock")).await {
Ok(encoded) => match toml::from_str(&encoded) {
Ok(lock) => Ok(Some(lock)),
Expand Down
48 changes: 43 additions & 5 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,20 @@ use uv_configuration::{Concurrency, Constraints, ExtrasSpecification, Reinstall,
use uv_dispatch::BuildDispatch;
use uv_distribution::DistributionDatabase;
use uv_fs::Simplified;
use uv_git::ResolvedRepositoryReference;
use uv_installer::{SatisfiesResult, SitePackages};
use uv_normalize::PackageName;
use uv_python::{
EnvironmentPreference, Interpreter, InvalidEnvironmentKind, PythonDownloads, PythonEnvironment,
PythonInstallation, PythonPreference, PythonRequest, PythonVersionFile, VersionRequest,
};
use uv_requirements::upgrade::{read_lock_requirements, LockedRequirements};
use uv_requirements::{
NamedRequirementsError, NamedRequirementsResolver, RequirementsSpecification,
};
use uv_resolver::{
FlatIndex, OptionsBuilder, PythonRequirement, RequiresPython, ResolutionGraph, ResolverMarkers,
FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython, ResolutionGraph,
ResolverMarkers,
};
use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy};
use uv_warnings::{warn_user, warn_user_once};
Expand Down Expand Up @@ -693,10 +696,34 @@ pub(crate) async fn resolve_names(
Ok(requirements)
}

#[derive(Debug)]
pub(crate) struct EnvironmentSpecification<'lock> {
/// The requirements to include in the environment.
requirements: RequirementsSpecification,
/// The lockfile from which to extract preferences.
lock: Option<&'lock Lock>,
}

impl From<RequirementsSpecification> for EnvironmentSpecification<'_> {
fn from(requirements: RequirementsSpecification) -> Self {
Self {
requirements,
lock: None,
}
}
}

impl<'lock> EnvironmentSpecification<'lock> {
#[must_use]
pub(crate) fn with_lock(self, lock: Option<&'lock Lock>) -> Self {
Self { lock, ..self }
}
}

/// Run dependency resolution for an interpreter, returning the [`ResolutionGraph`].
pub(crate) async fn resolve_environment<'a>(
spec: EnvironmentSpecification<'_>,
interpreter: &Interpreter,
spec: RequirementsSpecification,
settings: ResolverSettingsRef<'_>,
state: &SharedState,
logger: Box<dyn ResolveLogger>,
Expand All @@ -706,7 +733,7 @@ pub(crate) async fn resolve_environment<'a>(
cache: &Cache,
printer: Printer,
) -> Result<ResolutionGraph, ProjectError> {
warn_on_requirements_txt_setting(&spec, settings);
warn_on_requirements_txt_setting(&spec.requirements, settings);

let ResolverSettingsRef {
index_locations,
Expand Down Expand Up @@ -734,7 +761,7 @@ pub(crate) async fn resolve_environment<'a>(
overrides,
source_trees,
..
} = spec;
} = spec.requirements;

// Determine the tags, markers, and interpreter to use for resolution.
let tags = interpreter.tags()?;
Expand Down Expand Up @@ -782,7 +809,6 @@ pub(crate) async fn resolve_environment<'a>(
let dev = Vec::default();
let extras = ExtrasSpecification::default();
let hasher = HashStrategy::default();
let preferences = Vec::default();
let build_constraints = Constraints::default();
let build_hasher = HashStrategy::default();

Expand All @@ -791,6 +817,18 @@ pub(crate) async fn resolve_environment<'a>(
let reinstall = Reinstall::default();
let upgrade = Upgrade::default();

// If an existing lockfile exists, build up a set of preferences.
let LockedRequirements { preferences, git } = spec
.lock
.map(|lock| read_lock_requirements(lock, &upgrade))
.unwrap_or_default();

// Populate the Git resolver.
for ResolvedRepositoryReference { reference, sha } in git {
debug!("Inserting Git reference into resolver: `{reference:?}` at `{sha}`");
state.git.insert(reference, sha);
}

// Resolve the flat indexes from `--find-links`.
let flat_index = {
let client = FlatIndexClient::new(&client, cache);
Expand Down
22 changes: 19 additions & 3 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use uv_python::{
PythonPreference, PythonRequest, PythonVersionFile, VersionRequest,
};
use uv_requirements::{RequirementsSource, RequirementsSpecification};
use uv_resolver::Lock;
use uv_scripts::Pep723Script;
use uv_warnings::warn_user;
use uv_workspace::{DiscoveryOptions, InstallTarget, VirtualProject, Workspace, WorkspaceError};
Expand All @@ -37,7 +38,8 @@ use crate::commands::pip::operations;
use crate::commands::pip::operations::Modifications;
use crate::commands::project::environment::CachedEnvironment;
use crate::commands::project::{
validate_requires_python, ProjectError, PythonRequestSource, WorkspacePython,
validate_requires_python, EnvironmentSpecification, ProjectError, PythonRequestSource,
WorkspacePython,
};
use crate::commands::reporters::PythonDownloadReporter;
use crate::commands::{project, ExitStatus, SharedState};
Expand Down Expand Up @@ -205,7 +207,7 @@ pub(crate) async fn run(
.collect::<Result<_, _>>()?;
let spec = RequirementsSpecification::from_requirements(requirements);
let result = CachedEnvironment::get_or_create(
spec,
EnvironmentSpecification::from(spec),
interpreter,
&settings,
&state,
Expand Down Expand Up @@ -260,6 +262,9 @@ pub(crate) async fn run(
None
};

// The lockfile used for the base environment.
let mut lock: Option<Lock> = None;

// Discover and sync the base environment.
let temp_dir;
let base_interpreter = if let Some(script_interpreter) = script_interpreter {
Expand Down Expand Up @@ -466,6 +471,15 @@ pub(crate) async fn run(

if no_sync {
debug!("Skipping environment synchronization due to `--no-sync`");

// If we're not syncing, we should still attempt to respect the locked preferences
// in any `--with` requirements.
if !isolated && !requirements.is_empty() {
lock = project::lock::read(project.workspace())
.await
.ok()
.flatten();
}
} else {
let result = match project::lock::do_safe_lock(
locked,
Expand Down Expand Up @@ -522,6 +536,8 @@ pub(crate) async fn run(
printer,
)
.await?;

lock = Some(result.into_lock());
}

venv.into_interpreter()
Expand Down Expand Up @@ -626,7 +642,7 @@ pub(crate) async fn run(
debug!("Syncing ephemeral requirements");

let result = CachedEnvironment::get_or_create(
spec,
EnvironmentSpecification::from(spec).with_lock(lock.as_ref()),
base_interpreter.clone(),
&settings,
&state,
Expand Down
3 changes: 2 additions & 1 deletion crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger};

use crate::commands::project::{
resolve_environment, resolve_names, sync_environment, update_environment,
EnvironmentSpecification,
};
use crate::commands::tool::common::remove_entrypoints;
use crate::commands::tool::Target;
Expand Down Expand Up @@ -384,8 +385,8 @@ pub(crate) async fn install(
// If we're creating a new environment, ensure that we can resolve the requirements prior
// to removing any existing tools.
let resolution = resolve_environment(
EnvironmentSpecification::from(spec),
&interpreter,
spec,
settings.as_ref().into(),
&state,
Box::new(DefaultResolveLogger),
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/src/commands/tool/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::commands::pip::loggers::{
DefaultInstallLogger, DefaultResolveLogger, SummaryInstallLogger, SummaryResolveLogger,
};
use crate::commands::pip::operations;
use crate::commands::project::{resolve_names, ProjectError};
use crate::commands::project::{resolve_names, EnvironmentSpecification, ProjectError};
use crate::commands::reporters::PythonDownloadReporter;
use crate::commands::tool::Target;
use crate::commands::{
Expand Down Expand Up @@ -488,7 +488,7 @@ async fn get_or_create_environment(
// TODO(zanieb): Determine if we should layer on top of the project environment if it is present.

let environment = CachedEnvironment::get_or_create(
spec,
EnvironmentSpecification::from(spec),
interpreter,
settings,
&state,
Expand Down
60 changes: 46 additions & 14 deletions crates/uv/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ fn run_with() -> Result<()> {
name = "foo"
version = "1.0.0"
requires-python = ">=3.8"
dependencies = ["anyio", "sniffio==1.3.1"]
dependencies = ["sniffio==1.3.0"]
[build-system]
requires = ["setuptools>=42"]
Expand All @@ -628,13 +628,11 @@ fn run_with() -> Result<()> {
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Prepared 4 packages in [TIME]
Installed 4 packages in [TIME]
+ anyio==4.3.0
Resolved 2 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
+ foo==1.0.0 (from file://[TEMP_DIR]/)
+ idna==3.6
+ sniffio==1.3.1
+ sniffio==1.3.0
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
Expand All @@ -648,22 +646,56 @@ fn run_with() -> Result<()> {
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Audited 4 packages in [TIME]
Resolved 2 packages in [TIME]
Audited 2 packages in [TIME]
"###);

// Unless the user requests a different version.
uv_snapshot!(context.filters(), context.run().arg("--with").arg("sniffio<1.3.1").arg("main.py"), @r###"
uv_snapshot!(context.filters(), context.run().arg("--with").arg("sniffio<1.3.0").arg("main.py"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Audited 4 packages in [TIME]
Resolved 2 packages in [TIME]
Audited 2 packages in [TIME]
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ sniffio==1.2.0
"###);

// If we request a dependency that isn't in the base environment, we should still respect any
// other dependencies. In this case, `sniffio==1.3.0` is not the latest-compatible version, but
// we should use it anyway.
uv_snapshot!(context.filters(), context.run().arg("--with").arg("anyio").arg("main.py"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Audited 2 packages in [TIME]
Resolved 3 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 3 packages in [TIME]
+ anyio==4.3.0
+ idna==3.6
+ sniffio==1.3.0
"###);

// Even if we run with` --no-sync`.
uv_snapshot!(context.filters(), context.run().arg("--with").arg("anyio==4.2.0").arg("--no-sync").arg("main.py"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 3 packages in [TIME]
Prepared 1 package in [TIME]
Installed 3 packages in [TIME]
+ anyio==4.2.0
+ idna==3.6
+ sniffio==1.3.0
"###);

Expand All @@ -674,8 +706,8 @@ fn run_with() -> Result<()> {
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Audited 4 packages in [TIME]
Resolved 2 packages in [TIME]
Audited 2 packages in [TIME]
× No solution found when resolving `--with` dependencies:
╰─▶ Because there are no versions of add and you require add, we can conclude that your requirements are unsatisfiable.
"###);
Expand Down

0 comments on commit ff066c8

Please sign in to comment.