Skip to content

Commit

Permalink
Make target Python version an optional field (astral-sh#4000)
Browse files Browse the repository at this point in the history
## Summary

Instead of checking if the target and installed version are the same, we
model the data such that the target version is only present if it was
specified by the user. This also means that we correctly say "requested
version" even if the two happen to be the same.
  • Loading branch information
charliermarsh committed Jun 3, 2024
1 parent 037e7e3 commit 77e9315
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 35 deletions.
2 changes: 1 addition & 1 deletion crates/bench/benches/uv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ mod resolver {
let index_locations = IndexLocations::default();
let installed_packages = EmptyInstalledPackages;
let interpreter = venv.interpreter();
let python_requirement = PythonRequirement::from_marker_environment(interpreter, &MARKERS);
let python_requirement = PythonRequirement::from_interpreter(interpreter);

let build_context = BuildDispatch::new(
client,
Expand Down
3 changes: 1 addition & 2 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,8 @@ impl<'a> BuildContext for BuildDispatch<'a> {
}

async fn resolve<'data>(&'data self, requirements: &'data [Requirement]) -> Result<Resolution> {
let python_requirement = PythonRequirement::from_interpreter(self.interpreter);
let markers = self.interpreter.markers();
let python_requirement =
PythonRequirement::from_marker_environment(self.interpreter, markers);
let tags = self.interpreter.tags()?;
let resolver = Resolver::new(
Manifest::simple(requirements.to_vec()),
Expand Down
16 changes: 11 additions & 5 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,17 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
&**package,
PubGrubPackageInner::Python(PubGrubPython::Target)
) {
return format!(
"the requested {package} version ({}) does not satisfy {}",
python.target(),
PackageRange::compatibility(package, set)
);
return if let Some(target) = python.target() {
format!(
"the requested {package} version ({target}) does not satisfy {}",
PackageRange::compatibility(package, set)
)
} else {
format!(
"the requested {package} version does not satisfy {}",
PackageRange::compatibility(package, set)
)
};
}
if matches!(
&**package,
Expand Down
24 changes: 16 additions & 8 deletions crates/uv-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use pep508_rs::{MarkerEnvironment, StringVersion};
use uv_interpreter::Interpreter;
use pep508_rs::StringVersion;
use uv_interpreter::{Interpreter, PythonVersion};

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct PythonRequirement {
Expand All @@ -8,21 +8,29 @@ pub struct PythonRequirement {
/// The target version of Python; that is, the version of Python for which we are resolving
/// dependencies. This is typically the same as the installed version, but may be different
/// when specifying an alternate Python version for the resolution.
target: StringVersion,
///
/// If `None`, the target version is the same as the installed version.
target: Option<StringVersion>,
}

impl PythonRequirement {
pub fn from_marker_environment(interpreter: &Interpreter, env: &MarkerEnvironment) -> Self {
/// Create a [`PythonRequirement`] to resolve against both an [`Interpreter`] and a
/// [`PythonVersion`].
pub fn from_python_version(interpreter: &Interpreter, python_version: &PythonVersion) -> Self {
Self {
installed: interpreter.python_full_version().clone(),
target: env.python_full_version().clone(),
target: Some(StringVersion {
string: python_version.to_string(),
version: python_version.python_full_version(),
}),
}
}

/// Create a [`PythonRequirement`] to resolve against an [`Interpreter`].
pub fn from_interpreter(interpreter: &Interpreter) -> Self {
Self {
installed: interpreter.python_full_version().clone(),
target: interpreter.python_full_version().clone(),
target: None,
}
}

Expand All @@ -32,7 +40,7 @@ impl PythonRequirement {
}

/// Return the target version of Python.
pub fn target(&self) -> &StringVersion {
&self.target
pub fn target(&self) -> Option<&StringVersion> {
self.target.as_ref()
}
}
10 changes: 6 additions & 4 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag

debug!(
"Solving with target Python version {}",
self.python_requirement.target()
self.python_requirement
.target()
.unwrap_or(self.python_requirement.installed())
);

'FORK: while let Some(mut state) = forked_states.pop() {
Expand Down Expand Up @@ -715,9 +717,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag

// The version is incompatible due to its Python requirement.
if let Some(requires_python) = metadata.requires_python.as_ref() {
let installed = self.python_requirement.installed();
let target = self.python_requirement.target();
if target != installed {
if let Some(target) = self.python_requirement.target() {
if !requires_python.contains(target) {
return Ok(Some(ResolverVersion::Unavailable(
version.clone(),
Expand All @@ -730,6 +730,8 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
)));
}
}

let installed = self.python_requirement.installed();
if !requires_python.contains(installed) {
return Ok(Some(ResolverVersion::Unavailable(
version.clone(),
Expand Down
23 changes: 16 additions & 7 deletions crates/uv-resolver/src/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,8 @@ impl VersionMapLazy {
// Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully)
if let Some(requires_python) = requires_python {
if self.python_requirement.target() != self.python_requirement.installed() {
if !requires_python.contains(self.python_requirement.target()) {
if let Some(target) = self.python_requirement.target() {
if !requires_python.contains(target) {
return SourceDistCompatibility::Incompatible(
IncompatibleSource::RequiresPython(
requires_python,
Expand Down Expand Up @@ -533,11 +533,20 @@ impl VersionMapLazy {

// Check for a Python version incompatibility`
if let Some(requires_python) = requires_python {
if !requires_python.contains(self.python_requirement.target()) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python,
PythonRequirementKind::Target,
));
if let Some(target) = self.python_requirement.target() {
if !requires_python.contains(target) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python,
PythonRequirementKind::Target,
));
}
} else {
if !requires_python.contains(self.python_requirement.installed()) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python,
PythonRequirementKind::Installed,
));
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/tests/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ async fn resolve(
.expect("Python should be installed")
.into_interpreter();
let interpreter = Interpreter::artificial(real_interpreter.platform().clone(), markers.clone());
let python_requirement = PythonRequirement::from_marker_environment(&interpreter, markers);
let python_requirement = PythonRequirement::from_interpreter(&interpreter);
let cache = Cache::temp().unwrap().init().unwrap();
let build_context = DummyContext::new(cache, interpreter.clone());
let hashes = HashStrategy::None;
Expand Down
9 changes: 7 additions & 2 deletions crates/uv/src/commands/pip/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,13 @@ pub(crate) async fn pip_compile(
InMemoryIndexRef::Borrowed(&source_index)
};

// Determine the Python requirement, based on the interpreter and the requested version.
let python_requirement = if let Some(python_version) = python_version.as_ref() {
PythonRequirement::from_python_version(&interpreter, python_version)
} else {
PythonRequirement::from_interpreter(&interpreter)
};

// Determine the tags, markers, and interpreter to use for resolution.
let tags = match (python_platform, python_version.as_ref()) {
(Some(python_platform), Some(python_version)) => Cow::Owned(Tags::from_env(
Expand Down Expand Up @@ -251,8 +258,6 @@ pub(crate) async fn pip_compile(
(None, None) => Cow::Borrowed(interpreter.markers()),
};

let python_requirement = PythonRequirement::from_marker_environment(&interpreter, &markers);

// Generate, but don't enforce hashes for the requirements.
let hasher = if generate_hashes {
HashStrategy::Generate
Expand Down
7 changes: 2 additions & 5 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,6 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
// Collect constraints and overrides.
let constraints = Constraints::from_requirements(constraints);
let overrides = Overrides::from_requirements(overrides);
let python_requirement = if let Some(markers) = markers {
PythonRequirement::from_marker_environment(interpreter, markers)
} else {
PythonRequirement::from_interpreter(interpreter)
};

// Determine any lookahead requirements.
let lookaheads = match options.dependency_mode {
Expand All @@ -206,6 +201,8 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
DependencyMode::Direct => Vec::new(),
};

let python_requirement = PythonRequirement::from_interpreter(interpreter);

// TODO(zanieb): Consider consuming these instead of cloning
let exclusions = Exclusions::new(reinstall.clone(), upgrade.clone());

Expand Down

0 comments on commit 77e9315

Please sign in to comment.