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 -p to use complex Python version requests in uv pip compile #11486

Merged
merged 6 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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
13 changes: 12 additions & 1 deletion crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,9 +1170,20 @@ pub struct PipCompileArgs {
///
/// If a patch version is omitted, the minimum patch version is assumed. For
/// example, `3.8` is mapped to `3.8.0`.
#[arg(long, short, help_heading = "Python options")]
#[arg(long, help_heading = "Python options")]
pub python_version: Option<PythonVersion>,

/// The Python interpreter or version to use for resolution.
///
/// In previous versions of uv, `-p` was an alias for `--python-version` in `uv pip compile` but
/// an alias for `--python` in all other commands. This option is provided as a backwards
/// compatible shim, allowing `-p` to behave as `--python` without introducing a breaking
/// change.
///
/// `UV_PYTHON` is respected, but overridden by `--python-version` or `--python`.
#[arg(short, hide = true, help_heading = "Python options")]
pub python_legacy: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this just be done via -p? So --python 3.7 and -p 3.7 now behave differently?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should --python 3.7 error if 3.7 isn't installed or available?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's complicated.

--python 3.7 continues to work as it does today — it errors if it's not found. I don't know if I want to break that behavior because it does matter when you're not just doing version requests, e.g., --python pypy needs to fail if we can't find that implementation or the tags will be wrong. Similarly, --python <path> needs to fail.

We could special case --python <version> in the future if we want, which (as you said) would let us drop this legacy flag. However, then there would be no way for users to enforce that we actually find that interpreter.


/// The platform for which requirements should be resolved.
///
/// Represented as a "target triple", a string that describes the target platform in terms of
Expand Down
43 changes: 39 additions & 4 deletions crates/uv/src/commands/pip/compile.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::BTreeSet;
use std::env;
use std::path::Path;
use std::str::FromStr;
use std::sync::Arc;

use anyhow::{anyhow, Result};
Expand Down Expand Up @@ -38,6 +39,7 @@ use uv_resolver::{
InMemoryIndex, OptionsBuilder, PrereleaseMode, PythonRequirement, RequiresPython,
ResolutionMode, ResolverEnvironment,
};
use uv_static::EnvVars;
use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy};
use uv_warnings::warn_user;

Expand Down Expand Up @@ -86,14 +88,15 @@ pub(crate) async fn pip_compile(
no_build_isolation: bool,
no_build_isolation_package: Vec<PackageName>,
build_options: BuildOptions,
python_version: Option<PythonVersion>,
mut python_version: Option<PythonVersion>,
python_platform: Option<TargetTriple>,
universal: bool,
exclude_newer: Option<ExcludeNewer>,
sources: SourceStrategy,
annotation_style: AnnotationStyle,
link_mode: LinkMode,
python: Option<String>,
mut python: Option<String>,
mut python_legacy: Option<String>,
system: bool,
python_preference: PythonPreference,
concurrency: Concurrency,
Expand All @@ -103,6 +106,38 @@ pub(crate) async fn pip_compile(
printer: Printer,
preview: PreviewMode,
) -> Result<ExitStatus> {
// If the user requests both `-p` and `--python` or `--python-version`, error
if let Some(python_legacy) = python_legacy.as_ref() {
if let Some(python) = python.as_ref() {
return Err(anyhow!(
"Cannot specify both `-p` ({python_legacy}) and `--python` ({python}).",
));
}
if let Some(python_version) = python_version.as_ref() {
return Err(anyhow!(
"Cannot specify both `-p` ({python_legacy}) and `--python-version` ({python_version}).",
));
}
}

// Respect `UV_PYTHON` with legacy behavior
if python_legacy.is_none() && python_version.is_none() && python.is_none() {
if let Ok(python) = std::env::var(EnvVars::UV_PYTHON) {
if !python.is_empty() {
python_legacy = Some(python);
}
}
}

// Resolve `-p` into `--python-version` or `--python`
if let Some(python_legacy) = python_legacy {
if let Ok(version) = PythonVersion::from_str(&python_legacy) {
python_version = Some(version);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the net effect here is that we don't error if the version doesn't exist, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

} else {
python = Some(python_legacy);
}
}

// If the user requests `extras` but does not provide a valid source (e.g., a `pyproject.toml`),
// return an error.
if !extras.is_empty() && !requirements.iter().any(RequirementsSource::allows_extras) {
Expand Down Expand Up @@ -189,8 +224,8 @@ pub(crate) async fn pip_compile(
let request = PythonRequest::parse(python);
PythonInstallation::find(&request, environment_preference, python_preference, &cache)
} else {
// TODO(zanieb): The split here hints at a problem with the abstraction; we should be able to use
// `PythonInstallation::find(...)` here.
// TODO(zanieb): The split here hints at a problem with the request abstraction; we should
// be able to use `PythonInstallation::find(...)` here.
let request = if let Some(version) = python_version.as_ref() {
// TODO(zanieb): We should consolidate `VersionRequest` and `PythonVersion`
PythonRequest::Version(VersionRequest::from(version))
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
args.settings.annotation_style,
args.settings.link_mode,
args.settings.python,
args.python_legacy,
args.settings.system,
globals.python_preference,
globals.concurrency,
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,7 @@ pub(crate) struct PipCompileSettings {
pub(crate) overrides_from_workspace: Vec<Requirement>,
pub(crate) environments: SupportedEnvironments,
pub(crate) refresh: Refresh,
pub(crate) python_legacy: Option<String>,
pub(crate) settings: PipSettings,
}

Expand Down Expand Up @@ -1581,6 +1582,7 @@ impl PipCompileSettings {
no_binary,
only_binary,
python_version,
python_legacy,
python_platform,
universal,
no_universal,
Expand Down Expand Up @@ -1650,6 +1652,7 @@ impl PipCompileSettings {
overrides_from_workspace,
environments,
refresh: Refresh::from(refresh),
python_legacy,
settings: PipSettings::combine(
PipOptions {
python: python.and_then(Maybe::into_option),
Expand Down
Loading
Loading