Allow version specifiers to be used in Python version requests (#4214)

In service of https://github.com/astral-sh/uv/issues/4212 but this is
user-facing e.g. Python discovery will support version specifiers
everywhere now.

Closes https://github.com/astral-sh/uv/issues/4212
This commit is contained in:
Zanie Blue 2024-06-10 19:20:09 -04:00 committed by GitHub
parent 10e0abc9b1
commit 5f37395f45
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 119 additions and 61 deletions

View file

@ -1,11 +1,11 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::HashSet; use std::collections::HashSet;
use std::fmt::{self, Formatter}; use std::fmt::{self, Formatter};
use std::num::ParseIntError;
use std::{env, io}; use std::{env, io};
use std::{path::Path, path::PathBuf, str::FromStr}; use std::{path::Path, path::PathBuf, str::FromStr};
use itertools::Itertools; use itertools::Itertools;
use pep440_rs::{Version, VersionSpecifiers};
use same_file::is_same_file; use same_file::is_same_file;
use thiserror::Error; use thiserror::Error;
use tracing::{debug, instrument, trace}; use tracing::{debug, instrument, trace};
@ -35,7 +35,7 @@ pub enum ToolchainRequest {
/// Use any discovered Python toolchain /// Use any discovered Python toolchain
#[default] #[default]
Any, Any,
/// A Python version without an implementation name e.g. `3.10` /// A Python version without an implementation name e.g. `3.10` or `>=3.12,<3.13`
Version(VersionRequest), Version(VersionRequest),
/// A path to a directory containing a Python installation, e.g. `.venv` /// A path to a directory containing a Python installation, e.g. `.venv`
Directory(PathBuf), Directory(PathBuf),
@ -63,13 +63,14 @@ pub enum ToolchainSources {
} }
/// A Python toolchain version request. /// A Python toolchain version request.
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] #[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum VersionRequest { pub enum VersionRequest {
#[default] #[default]
Any, Any,
Major(u8), Major(u8),
MajorMinor(u8, u8), MajorMinor(u8, u8),
MajorMinorPatch(u8, u8, u8), MajorMinorPatch(u8, u8, u8),
Range(VersionSpecifiers),
} }
/// The policy for discovery of "system" Python interpreters. /// The policy for discovery of "system" Python interpreters.
@ -159,6 +160,10 @@ pub enum Error {
#[error(transparent)] #[error(transparent)]
PyLauncher(#[from] crate::py_launcher::Error), PyLauncher(#[from] crate::py_launcher::Error),
/// An invalid version request was given
#[error("Invalid version request: {0}")]
InvalidVersionRequest(String),
#[error("Interpreter discovery for `{0}` requires `{1}` but it is not selected; the following are selected: {2}")] #[error("Interpreter discovery for `{0}` requires `{1}` but it is not selected; the following are selected: {2}")]
SourceNotSelected(ToolchainRequest, ToolchainSource, ToolchainSources), SourceNotSelected(ToolchainRequest, ToolchainSource, ToolchainSources),
} }
@ -567,7 +572,7 @@ pub(crate) fn find_toolchain(
ToolchainNotFound::NoMatchingImplementationVersion( ToolchainNotFound::NoMatchingImplementationVersion(
sources.clone(), sources.clone(),
*implementation, *implementation,
*version, version.clone(),
), ),
)); ));
}; };
@ -612,9 +617,9 @@ pub(crate) fn find_toolchain(
.transpose()? .transpose()?
else { else {
let err = if matches!(version, VersionRequest::Any) { let err = if matches!(version, VersionRequest::Any) {
ToolchainNotFound::NoPythonInstallation(sources.clone(), Some(*version)) ToolchainNotFound::NoPythonInstallation(sources.clone(), Some(version.clone()))
} else { } else {
ToolchainNotFound::NoMatchingVersion(sources.clone(), *version) ToolchainNotFound::NoMatchingVersion(sources.clone(), version.clone())
}; };
return Ok(ToolchainResult::Err(err)); return Ok(ToolchainResult::Err(err));
}; };
@ -683,14 +688,17 @@ pub fn find_best_toolchain(
if let Some(request) = match request { if let Some(request) = match request {
ToolchainRequest::Version(version) => { ToolchainRequest::Version(version) => {
if version.has_patch() { if version.has_patch() {
Some(ToolchainRequest::Version((*version).without_patch())) Some(ToolchainRequest::Version(version.clone().without_patch()))
} else { } else {
None None
} }
} }
ToolchainRequest::ImplementationVersion(implementation, version) => Some( ToolchainRequest::ImplementationVersion(implementation, version) => {
ToolchainRequest::ImplementationVersion(*implementation, (*version).without_patch()), Some(ToolchainRequest::ImplementationVersion(
), *implementation,
version.clone().without_patch(),
))
}
_ => None, _ => None,
} { } {
debug!("Looking for relaxed patch version {request}"); debug!("Looking for relaxed patch version {request}");
@ -1030,7 +1038,7 @@ impl ToolchainRequest {
} }
impl VersionRequest { impl VersionRequest {
pub(crate) fn default_names(self) -> [Option<Cow<'static, str>>; 4] { pub(crate) fn default_names(&self) -> [Option<Cow<'static, str>>; 4] {
let (python, python3, extension) = if cfg!(windows) { let (python, python3, extension) = if cfg!(windows) {
( (
Cow::Borrowed("python.exe"), Cow::Borrowed("python.exe"),
@ -1042,7 +1050,7 @@ impl VersionRequest {
}; };
match self { match self {
Self::Any => [Some(python3), Some(python), None, None], Self::Any | Self::Range(_) => [Some(python3), Some(python), None, None],
Self::Major(major) => [ Self::Major(major) => [
Some(Cow::Owned(format!("python{major}{extension}"))), Some(Cow::Owned(format!("python{major}{extension}"))),
Some(python), Some(python),
@ -1085,7 +1093,7 @@ impl VersionRequest {
}; };
match self { match self {
Self::Any => [Some(python3), Some(python), None, None], Self::Any | Self::Range(_) => [Some(python3), Some(python), None, None],
Self::Major(major) => [ Self::Major(major) => [
Some(Cow::Owned(format!("{name}{major}{extension}"))), Some(Cow::Owned(format!("{name}{major}{extension}"))),
Some(python), Some(python),
@ -1113,67 +1121,87 @@ impl VersionRequest {
} }
/// Check if a interpreter matches the requested Python version. /// Check if a interpreter matches the requested Python version.
fn matches_interpreter(self, interpreter: &Interpreter) -> bool { fn matches_interpreter(&self, interpreter: &Interpreter) -> bool {
match self { match self {
Self::Any => true, Self::Any => true,
Self::Major(major) => interpreter.python_major() == major, Self::Major(major) => interpreter.python_major() == *major,
Self::MajorMinor(major, minor) => { Self::MajorMinor(major, minor) => {
(interpreter.python_major(), interpreter.python_minor()) == (major, minor) (interpreter.python_major(), interpreter.python_minor()) == (*major, *minor)
} }
Self::MajorMinorPatch(major, minor, patch) => { Self::MajorMinorPatch(major, minor, patch) => {
( (
interpreter.python_major(), interpreter.python_major(),
interpreter.python_minor(), interpreter.python_minor(),
interpreter.python_patch(), interpreter.python_patch(),
) == (major, minor, patch) ) == (*major, *minor, *patch)
} }
Self::Range(specifiers) => specifiers.contains(interpreter.python_version()),
} }
} }
pub(crate) fn matches_version(self, version: &PythonVersion) -> bool { pub(crate) fn matches_version(&self, version: &PythonVersion) -> bool {
match self { match self {
Self::Any => true, Self::Any => true,
Self::Major(major) => version.major() == major, Self::Major(major) => version.major() == *major,
Self::MajorMinor(major, minor) => (version.major(), version.minor()) == (major, minor), Self::MajorMinor(major, minor) => {
(version.major(), version.minor()) == (*major, *minor)
}
Self::MajorMinorPatch(major, minor, patch) => { Self::MajorMinorPatch(major, minor, patch) => {
(version.major(), version.minor(), version.patch()) == (major, minor, Some(patch)) (version.major(), version.minor(), version.patch())
== (*major, *minor, Some(*patch))
} }
Self::Range(specifiers) => specifiers.contains(&version.version),
} }
} }
fn matches_major_minor(self, major: u8, minor: u8) -> bool { fn matches_major_minor(&self, major: u8, minor: u8) -> bool {
match self { match self {
Self::Any => true, Self::Any => true,
Self::Major(self_major) => self_major == major, Self::Major(self_major) => *self_major == major,
Self::MajorMinor(self_major, self_minor) => (self_major, self_minor) == (major, minor), Self::MajorMinor(self_major, self_minor) => {
(*self_major, *self_minor) == (major, minor)
}
Self::MajorMinorPatch(self_major, self_minor, _) => { Self::MajorMinorPatch(self_major, self_minor, _) => {
(self_major, self_minor) == (major, minor) (*self_major, *self_minor) == (major, minor)
}
Self::Range(specifiers) => {
specifiers.contains(&Version::new([u64::from(major), u64::from(minor)]))
} }
} }
} }
pub(crate) fn matches_major_minor_patch(self, major: u8, minor: u8, patch: u8) -> bool { pub(crate) fn matches_major_minor_patch(&self, major: u8, minor: u8, patch: u8) -> bool {
match self { match self {
Self::Any => true, Self::Any => true,
Self::Major(self_major) => self_major == major, Self::Major(self_major) => *self_major == major,
Self::MajorMinor(self_major, self_minor) => (self_major, self_minor) == (major, minor), Self::MajorMinor(self_major, self_minor) => {
Self::MajorMinorPatch(self_major, self_minor, self_patch) => { (*self_major, *self_minor) == (major, minor)
(self_major, self_minor, self_patch) == (major, minor, patch)
} }
Self::MajorMinorPatch(self_major, self_minor, self_patch) => {
(*self_major, *self_minor, *self_patch) == (major, minor, patch)
}
Self::Range(specifiers) => specifiers.contains(&Version::new([
u64::from(major),
u64::from(minor),
u64::from(patch),
])),
} }
} }
/// Return true if a patch version is present in the request. /// Return true if a patch version is present in the request.
fn has_patch(self) -> bool { fn has_patch(&self) -> bool {
match self { match self {
Self::Any => false, Self::Any => false,
Self::Major(..) => false, Self::Major(..) => false,
Self::MajorMinor(..) => false, Self::MajorMinor(..) => false,
Self::MajorMinorPatch(..) => true, Self::MajorMinorPatch(..) => true,
Self::Range(_) => false,
} }
} }
/// Return a new `VersionRequest` without the patch version. /// Return a new [`VersionRequest`] without the patch version if possible.
///
/// If the patch version is not present, it is returned unchanged.
#[must_use] #[must_use]
fn without_patch(self) -> Self { fn without_patch(self) -> Self {
match self { match self {
@ -1181,30 +1209,38 @@ impl VersionRequest {
Self::Major(major) => Self::Major(major), Self::Major(major) => Self::Major(major),
Self::MajorMinor(major, minor) => Self::MajorMinor(major, minor), Self::MajorMinor(major, minor) => Self::MajorMinor(major, minor),
Self::MajorMinorPatch(major, minor, _) => Self::MajorMinor(major, minor), Self::MajorMinorPatch(major, minor, _) => Self::MajorMinor(major, minor),
Self::Range(_) => self,
} }
} }
} }
impl FromStr for VersionRequest { impl FromStr for VersionRequest {
type Err = ParseIntError; type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> { fn from_str(s: &str) -> Result<Self, Self::Err> {
let versions = s // e.g. `3.12.1`
if let Ok(versions) = s
.splitn(3, '.') .splitn(3, '.')
.map(str::parse::<u8>) .map(str::parse::<u8>)
.collect::<Result<Vec<_>, _>>()?; .collect::<Result<Vec<_>, _>>()
{
let selector = match versions.as_slice() {
// e.g. `3`
[major] => VersionRequest::Major(*major),
// e.g. `3.10`
[major, minor] => VersionRequest::MajorMinor(*major, *minor),
// e.g. `3.10.4`
[major, minor, patch] => VersionRequest::MajorMinorPatch(*major, *minor, *patch),
_ => unreachable!(),
};
let selector = match versions.as_slice() { Ok(selector)
// e.g. `3` // e.g. `>=3.12.1,<3.12`
[major] => VersionRequest::Major(*major), } else if let Ok(specifiers) = VersionSpecifiers::from_str(s) {
// e.g. `3.10` Ok(Self::Range(specifiers))
[major, minor] => VersionRequest::MajorMinor(*major, *minor), } else {
// e.g. `3.10.4` Err(Error::InvalidVersionRequest(s.to_string()))
[major, minor, patch] => VersionRequest::MajorMinorPatch(*major, *minor, *patch), }
_ => unreachable!(),
};
Ok(selector)
} }
} }
@ -1224,6 +1260,7 @@ impl fmt::Display for VersionRequest {
Self::MajorMinorPatch(major, minor, patch) => { Self::MajorMinorPatch(major, minor, patch) => {
write!(f, "{major}.{minor}.{patch}") write!(f, "{major}.{minor}.{patch}")
} }
Self::Range(specifiers) => write!(f, "{specifiers}"),
} }
} }
} }
@ -1462,6 +1499,14 @@ mod tests {
ToolchainRequest::parse("3.12"), ToolchainRequest::parse("3.12"),
ToolchainRequest::Version(VersionRequest::from_str("3.12").unwrap()) ToolchainRequest::Version(VersionRequest::from_str("3.12").unwrap())
); );
assert_eq!(
ToolchainRequest::parse(">=3.12"),
ToolchainRequest::Version(VersionRequest::from_str(">=3.12").unwrap())
);
assert_eq!(
ToolchainRequest::parse(">=3.12,<3.13"),
ToolchainRequest::Version(VersionRequest::from_str(">=3.12,<3.13").unwrap())
);
assert_eq!( assert_eq!(
ToolchainRequest::parse("foo"), ToolchainRequest::parse("foo"),
ToolchainRequest::ExecutableName("foo".to_string()) ToolchainRequest::ExecutableName("foo".to_string())
@ -1526,14 +1571,17 @@ mod tests {
#[test] #[test]
fn version_request_from_str() { fn version_request_from_str() {
assert_eq!(VersionRequest::from_str("3"), Ok(VersionRequest::Major(3)));
assert_eq!( assert_eq!(
VersionRequest::from_str("3.12"), VersionRequest::from_str("3").unwrap(),
Ok(VersionRequest::MajorMinor(3, 12)) VersionRequest::Major(3)
); );
assert_eq!( assert_eq!(
VersionRequest::from_str("3.12.1"), VersionRequest::from_str("3.12").unwrap(),
Ok(VersionRequest::MajorMinorPatch(3, 12, 1)) VersionRequest::MajorMinor(3, 12)
);
assert_eq!(
VersionRequest::from_str("3.12.1").unwrap(),
VersionRequest::MajorMinorPatch(3, 12, 1)
); );
assert!(VersionRequest::from_str("1.foo.1").is_err()); assert!(VersionRequest::from_str("1.foo.1").is_err());
} }

View file

@ -1,6 +1,5 @@
use std::fmt::Display; use std::fmt::Display;
use std::io; use std::io;
use std::num::ParseIntError;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::str::FromStr; use std::str::FromStr;
@ -26,7 +25,7 @@ pub enum Error {
#[error(transparent)] #[error(transparent)]
ImplementationError(#[from] ImplementationError), ImplementationError(#[from] ImplementationError),
#[error("Invalid python version: {0}")] #[error("Invalid python version: {0}")]
InvalidPythonVersion(ParseIntError), InvalidPythonVersion(String),
#[error("Download failed")] #[error("Download failed")]
NetworkError(#[from] BetterReqwestError), NetworkError(#[from] BetterReqwestError),
#[error("Download failed")] #[error("Download failed")]
@ -215,7 +214,7 @@ impl PythonDownloadRequest {
impl Display for PythonDownloadRequest { impl Display for PythonDownloadRequest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut parts = Vec::new(); let mut parts = Vec::new();
if let Some(version) = self.version { if let Some(version) = &self.version {
parts.push(version.to_string()); parts.push(version.to_string());
} }
if let Some(implementation) = self.implementation { if let Some(implementation) = self.implementation {
@ -239,7 +238,8 @@ impl FromStr for PythonDownloadRequest {
fn from_str(s: &str) -> Result<Self, Self::Err> { fn from_str(s: &str) -> Result<Self, Self::Err> {
// TODO(zanieb): Implement parsing of additional request parts // TODO(zanieb): Implement parsing of additional request parts
let version = VersionRequest::from_str(s).map_err(Error::InvalidPythonVersion)?; let version =
VersionRequest::from_str(s).map_err(|_| Error::InvalidPythonVersion(s.to_string()))?;
Ok(Self::new(Some(version), None, None, None, None)) Ok(Self::new(Some(version), None, None, None, None))
} }
} }

View file

@ -234,6 +234,7 @@ impl TestContext {
.arg("--cache-dir") .arg("--cache-dir")
.arg(self.cache_dir.path()) .arg(self.cache_dir.path())
.env("VIRTUAL_ENV", self.venv.as_os_str()) .env("VIRTUAL_ENV", self.venv.as_os_str())
.env("UV_TEST_PYTHON_PATH", "/dev/null")
.env("UV_NO_WRAP", "1") .env("UV_NO_WRAP", "1")
.env("UV_TEST_PYTHON_PATH", "/dev/null") .env("UV_TEST_PYTHON_PATH", "/dev/null")
.current_dir(&self.temp_dir); .current_dir(&self.temp_dir);
@ -250,7 +251,10 @@ impl TestContext {
/// Create a `uv lock` command with options shared across scenarios. /// Create a `uv lock` command with options shared across scenarios.
pub fn lock(&self) -> std::process::Command { pub fn lock(&self) -> std::process::Command {
let mut command = self.lock_without_exclude_newer(); let mut command = self.lock_without_exclude_newer();
command.arg("--exclude-newer").arg(EXCLUDE_NEWER); command
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("UV_TEST_PYTHON_PATH", "/dev/null");
command command
} }

View file

@ -2065,13 +2065,19 @@ fn lock_requires_python() -> Result<()> {
}); });
// Validate that attempting to install with an unsupported Python version raises an error. // Validate that attempting to install with an unsupported Python version raises an error.
let context = TestContext::new("3.8"); let context38 = TestContext::new("3.8");
fs_err::copy(pyproject_toml, context.temp_dir.join("pyproject.toml"))?; fs_err::copy(pyproject_toml, context38.temp_dir.join("pyproject.toml"))?;
fs_err::copy(&lockfile, context.temp_dir.join("uv.lock"))?; fs_err::copy(&lockfile, context38.temp_dir.join("uv.lock"))?;
let filters: Vec<_> = context38
.filters()
.into_iter()
.chain(context.filters())
.collect();
// Install from the lockfile. // Install from the lockfile.
uv_snapshot!(context.filters(), context.sync(), @r###" uv_snapshot!(filters, context38.sync(), @r###"
success: false success: false
exit_code: 2 exit_code: 2
----- stdout ----- ----- stdout -----
@ -2079,7 +2085,7 @@ fn lock_requires_python() -> Result<()> {
----- stderr ----- ----- stderr -----
warning: `uv sync` is experimental and may change without warning. warning: `uv sync` is experimental and may change without warning.
Removing virtual environment at: [VENV]/ Removing virtual environment at: [VENV]/
error: Requested Python executable `>=3.12` not found in PATH error: No interpreter found for Python >=3.12 in provided path, active virtual environment, or search path
"###); "###);
Ok(()) Ok(())