Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 98 additions & 50 deletions crates/uv-toolchain/src/discovery.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::borrow::Cow;
use std::collections::HashSet;
use std::fmt::{self, Formatter};
use std::num::ParseIntError;
use std::{env, io};
use std::{path::Path, path::PathBuf, str::FromStr};

use itertools::Itertools;
use pep440_rs::{Version, VersionSpecifiers};
use same_file::is_same_file;
use thiserror::Error;
use tracing::{debug, instrument, trace};
Expand Down Expand Up @@ -35,7 +35,7 @@ pub enum ToolchainRequest {
/// Use any discovered Python toolchain
#[default]
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),
/// A path to a directory containing a Python installation, e.g. `.venv`
Directory(PathBuf),
Expand Down Expand Up @@ -63,13 +63,14 @@ pub enum ToolchainSources {
}

/// A Python toolchain version request.
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum VersionRequest {
#[default]
Any,
Major(u8),
MajorMinor(u8, u8),
MajorMinorPatch(u8, u8, u8),
Range(VersionSpecifiers),
}

/// The policy for discovery of "system" Python interpreters.
Expand Down Expand Up @@ -159,6 +160,10 @@ pub enum Error {
#[error(transparent)]
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}")]
SourceNotSelected(ToolchainRequest, ToolchainSource, ToolchainSources),
}
Expand Down Expand Up @@ -567,7 +572,7 @@ pub(crate) fn find_toolchain(
ToolchainNotFound::NoMatchingImplementationVersion(
sources.clone(),
*implementation,
*version,
version.clone(),
),
));
};
Expand Down Expand Up @@ -612,9 +617,9 @@ pub(crate) fn find_toolchain(
.transpose()?
else {
let err = if matches!(version, VersionRequest::Any) {
ToolchainNotFound::NoPythonInstallation(sources.clone(), Some(*version))
ToolchainNotFound::NoPythonInstallation(sources.clone(), Some(version.clone()))
} else {
ToolchainNotFound::NoMatchingVersion(sources.clone(), *version)
ToolchainNotFound::NoMatchingVersion(sources.clone(), version.clone())
};
return Ok(ToolchainResult::Err(err));
};
Expand Down Expand Up @@ -683,14 +688,17 @@ pub fn find_best_toolchain(
if let Some(request) = match request {
ToolchainRequest::Version(version) => {
if version.has_patch() {
Some(ToolchainRequest::Version((*version).without_patch()))
Some(ToolchainRequest::Version(version.clone().without_patch()))
} else {
None
}
}
ToolchainRequest::ImplementationVersion(implementation, version) => Some(
ToolchainRequest::ImplementationVersion(*implementation, (*version).without_patch()),
),
ToolchainRequest::ImplementationVersion(implementation, version) => {
Some(ToolchainRequest::ImplementationVersion(
*implementation,
version.clone().without_patch(),
))
}
_ => None,
} {
debug!("Looking for relaxed patch version {request}");
Expand Down Expand Up @@ -1030,7 +1038,7 @@ impl ToolchainRequest {
}

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) {
(
Cow::Borrowed("python.exe"),
Expand All @@ -1042,7 +1050,7 @@ impl VersionRequest {
};

match self {
Self::Any => [Some(python3), Some(python), None, None],
Self::Any | Self::Range(_) => [Some(python3), Some(python), None, None],
Self::Major(major) => [
Some(Cow::Owned(format!("python{major}{extension}"))),
Some(python),
Expand Down Expand Up @@ -1085,7 +1093,7 @@ impl VersionRequest {
};

match self {
Self::Any => [Some(python3), Some(python), None, None],
Self::Any | Self::Range(_) => [Some(python3), Some(python), None, None],
Self::Major(major) => [
Some(Cow::Owned(format!("{name}{major}{extension}"))),
Some(python),
Expand Down Expand Up @@ -1113,98 +1121,126 @@ impl VersionRequest {
}

/// 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 {
Self::Any => true,
Self::Major(major) => interpreter.python_major() == major,
Self::Major(major) => interpreter.python_major() == *major,
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) => {
(
interpreter.python_major(),
interpreter.python_minor(),
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 {
Self::Any => true,
Self::Major(major) => version.major() == major,
Self::MajorMinor(major, minor) => (version.major(), version.minor()) == (major, minor),
Self::Major(major) => version.major() == *major,
Self::MajorMinor(major, minor) => {
(version.major(), version.minor()) == (*major, *minor)
}
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 {
Self::Any => true,
Self::Major(self_major) => self_major == major,
Self::MajorMinor(self_major, self_minor) => (self_major, self_minor) == (major, minor),
Self::Major(self_major) => *self_major == major,
Self::MajorMinor(self_major, self_minor) => {
(*self_major, *self_minor) == (major, 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 {
Self::Any => true,
Self::Major(self_major) => self_major == major,
Self::MajorMinor(self_major, self_minor) => (self_major, self_minor) == (major, minor),
Self::Major(self_major) => *self_major == major,
Self::MajorMinor(self_major, self_minor) => {
(*self_major, *self_minor) == (major, minor)
}
Self::MajorMinorPatch(self_major, self_minor, self_patch) => {
(self_major, self_minor, self_patch) == (major, minor, 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.
fn has_patch(self) -> bool {
fn has_patch(&self) -> bool {
match self {
Self::Any => false,
Self::Major(..) => false,
Self::MajorMinor(..) => false,
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]
fn without_patch(self) -> Self {
match self {
Self::Any => Self::Any,
Self::Major(major) => Self::Major(major),
Self::MajorMinor(major, minor) => Self::MajorMinor(major, minor),
Self::MajorMinorPatch(major, minor, _) => Self::MajorMinor(major, minor),
Self::Range(_) => self,
}
}
}

impl FromStr for VersionRequest {
type Err = ParseIntError;
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let versions = s
// e.g. `3.12.1`
if let Ok(versions) = s
.splitn(3, '.')
.map(str::parse::<u8>)
.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!(),
};
.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!(),
};

Ok(selector)
Ok(selector)
// e.g. `>=3.12.1,<3.12`
} else if let Ok(specifiers) = VersionSpecifiers::from_str(s) {
Ok(Self::Range(specifiers))
} else {
Err(Error::InvalidVersionRequest(s.to_string()))
}
}
}

Expand All @@ -1224,6 +1260,7 @@ impl fmt::Display for VersionRequest {
Self::MajorMinorPatch(major, minor, patch) => {
write!(f, "{major}.{minor}.{patch}")
}
Self::Range(specifiers) => write!(f, "{specifiers}"),
}
}
}
Expand Down Expand Up @@ -1462,6 +1499,14 @@ mod tests {
ToolchainRequest::parse("3.12"),
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!(
ToolchainRequest::parse("foo"),
ToolchainRequest::ExecutableName("foo".to_string())
Expand Down Expand Up @@ -1526,14 +1571,17 @@ mod tests {

#[test]
fn version_request_from_str() {
assert_eq!(VersionRequest::from_str("3"), Ok(VersionRequest::Major(3)));
assert_eq!(
VersionRequest::from_str("3.12"),
Ok(VersionRequest::MajorMinor(3, 12))
VersionRequest::from_str("3").unwrap(),
VersionRequest::Major(3)
);
assert_eq!(
VersionRequest::from_str("3.12").unwrap(),
VersionRequest::MajorMinor(3, 12)
);
assert_eq!(
VersionRequest::from_str("3.12.1"),
Ok(VersionRequest::MajorMinorPatch(3, 12, 1))
VersionRequest::from_str("3.12.1").unwrap(),
VersionRequest::MajorMinorPatch(3, 12, 1)
);
assert!(VersionRequest::from_str("1.foo.1").is_err());
}
Expand Down
8 changes: 4 additions & 4 deletions crates/uv-toolchain/src/downloads.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::fmt::Display;
use std::io;
use std::num::ParseIntError;
use std::path::{Path, PathBuf};
use std::str::FromStr;

Expand All @@ -26,7 +25,7 @@ pub enum Error {
#[error(transparent)]
ImplementationError(#[from] ImplementationError),
#[error("Invalid python version: {0}")]
InvalidPythonVersion(ParseIntError),
InvalidPythonVersion(String),
#[error("Download failed")]
NetworkError(#[from] BetterReqwestError),
#[error("Download failed")]
Expand Down Expand Up @@ -215,7 +214,7 @@ impl PythonDownloadRequest {
impl Display for PythonDownloadRequest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut parts = Vec::new();
if let Some(version) = self.version {
if let Some(version) = &self.version {
parts.push(version.to_string());
}
if let Some(implementation) = self.implementation {
Expand All @@ -239,7 +238,8 @@ impl FromStr for PythonDownloadRequest {

fn from_str(s: &str) -> Result<Self, Self::Err> {
// 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))
}
}
Expand Down
6 changes: 5 additions & 1 deletion crates/uv/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ impl TestContext {
.arg("--cache-dir")
.arg(self.cache_dir.path())
.env("VIRTUAL_ENV", self.venv.as_os_str())
.env("UV_TEST_PYTHON_PATH", "/dev/null")
.env("UV_NO_WRAP", "1")
.env("UV_TEST_PYTHON_PATH", "/dev/null")
.current_dir(&self.temp_dir);
Expand All @@ -250,7 +251,10 @@ impl TestContext {
/// Create a `uv lock` command with options shared across scenarios.
pub fn lock(&self) -> std::process::Command {
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
}

Expand Down
Loading
Loading