Skip to content

Commit

Permalink
Replace tool environments on updated Python request (#4746)
Browse files Browse the repository at this point in the history
## Summary

Closes #4741.
  • Loading branch information
charliermarsh committed Jul 2, 2024
1 parent e88e137 commit 7573145
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 32 deletions.
40 changes: 33 additions & 7 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ impl InstalledTools {
/// Remove the environment for a tool.
///
/// Does not remove the tool's entrypoints.
///
/// # Errors
///
/// If no such environment exists for the tool.
pub fn remove_environment(&self, name: &PackageName) -> Result<(), Error> {
let _lock = self.acquire_lock();
let environment_path = self.root.join(name.to_string());
Expand All @@ -161,23 +165,45 @@ impl InstalledTools {
Ok(())
}

/// Return the [`PythonEnvironment`] for a given tool.
pub fn environment(
/// Return the [`PythonEnvironment`] for a given tool, if it exists.
pub fn get_environment(
&self,
name: &PackageName,
remove_existing: bool,
interpreter: Interpreter,
cache: &Cache,
) -> Result<PythonEnvironment, Error> {
) -> Result<Option<PythonEnvironment>, Error> {
let _lock = self.acquire_lock();
let environment_path = self.root.join(name.to_string());

if !remove_existing && environment_path.exists() {
if environment_path.is_dir() {
debug!(
"Using existing environment for tool `{name}` at `{}`.",
environment_path.user_display()
);
return Ok(PythonEnvironment::from_root(environment_path, cache)?);
Ok(Some(PythonEnvironment::from_root(environment_path, cache)?))
} else {
Ok(None)
}
}

/// Create the [`PythonEnvironment`] for a given tool, removing any existing environments.
pub fn create_environment(
&self,
name: &PackageName,
interpreter: Interpreter,
) -> Result<PythonEnvironment, Error> {
let _lock = self.acquire_lock();
let environment_path = self.root.join(name.to_string());

// Remove any existing environment.
match fs_err::remove_dir_all(&environment_path) {
Ok(()) => {
debug!(
"Removed existing environment for tool `{name}` at `{}`.",
environment_path.user_display()
);
}
Err(err) if err.kind() == io::ErrorKind::NotFound => (),
Err(err) => return Err(err.into()),
}

debug!(
Expand Down
65 changes: 48 additions & 17 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ pub(crate) async fn install(
.connectivity(connectivity)
.native_tls(native_tls);

let python_request = python.as_deref().map(ToolchainRequest::parse);

// Pre-emptively identify a Python interpreter. We need an interpreter to resolve any unnamed
// requirements, even if we end up using a different interpreter for the tool install itself.
let interpreter = Toolchain::find_or_fetch(
python.as_deref().map(ToolchainRequest::parse),
python_request.clone(),
EnvironmentPreference::OnlySystem,
toolchain_preference,
toolchain_fetch,
Expand Down Expand Up @@ -147,21 +151,42 @@ pub(crate) async fn install(

let installed_tools = InstalledTools::from_settings()?;
let existing_tool_receipt = installed_tools.get_tool_receipt(&from.name)?;
let existing_environment =
installed_tools
.get_environment(&from.name, cache)?
.filter(|environment| {
python_request.as_ref().map_or(true, |python_request| {
if python_request.satisfied(environment.interpreter(), cache) {
debug!("Found existing environment for tool `{}`", from.name);
true
} else {
let _ = writeln!(
printer.stderr(),
"Existing environment for `{}` does not satisfy the requested Python interpreter: `{}`",
from.name,
python_request
);
false
}
})
});

// If the requested and receipt requirements are the same...
if let Some(tool_receipt) = existing_tool_receipt.as_ref() {
let receipt = tool_receipt
.requirements()
.iter()
.cloned()
.map(Requirement::from)
.collect::<Vec<_>>();
if requirements == receipt {
// And the user didn't request a reinstall or upgrade...
if !force && settings.reinstall.is_none() && settings.upgrade.is_none() {
// We're done.
writeln!(printer.stderr(), "Tool `{from}` is already installed")?;
return Ok(ExitStatus::Failure);
if existing_environment.is_some() {
if let Some(tool_receipt) = existing_tool_receipt.as_ref() {
let receipt = tool_receipt
.requirements()
.iter()
.cloned()
.map(Requirement::from)
.collect::<Vec<_>>();
if requirements == receipt {
// And the user didn't request a reinstall or upgrade...
if !force && settings.reinstall.is_none() && settings.upgrade.is_none() {
// We're done.
writeln!(printer.stderr(), "Tool `{from}` is already installed")?;
return Ok(ExitStatus::Failure);
}
}
}
}
Expand All @@ -171,9 +196,15 @@ pub(crate) async fn install(
// entrypoints (without `--force`).
let reinstall_entry_points = existing_tool_receipt.is_some();

// TODO(zanieb): Build the environment in the cache directory then copy into the tool directory
// This lets us confirm the environment is valid before removing an existing install
let environment = installed_tools.environment(&from.name, force, interpreter, cache)?;
// TODO(zanieb): Build the environment in the cache directory then copy into the tool directory.
// This lets us confirm the environment is valid before removing an existing install. However,
// entrypoints always contain an absolute path to the relevant Python interpreter, which would
// be invalidated by moving the environment.
let environment = if let Some(environment) = existing_environment {
environment
} else {
installed_tools.create_environment(&from.name, interpreter)?
};

// Install the ephemeral requirements.
let spec = RequirementsSpecification::from_requirements(requirements.clone());
Expand Down
85 changes: 77 additions & 8 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,14 +648,6 @@ fn tool_install_entry_point_exists() {
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Resolved [N] packages in [TIME]
Installed [N] packages in [TIME]
+ black==24.3.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
Installed: black, blackd
"###);

Expand Down Expand Up @@ -1329,3 +1321,80 @@ fn tool_install_upgrade() {
"###);
});
}

/// Test reinstalling tools with varying `--python` requests.
#[test]
fn tool_install_python_request() {
let context = TestContext::new_with_versions(&["3.11", "3.12"])
.with_filtered_counts()
.with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

// Install `black`.
uv_snapshot!(context.filters(), context.tool_install()
.arg("-p")
.arg("3.12")
.arg("black")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ black==24.3.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
Installed: black, blackd
"###);

// Install with Python 3.12 (compatible).
uv_snapshot!(context.filters(), context.tool_install()
.arg("-p")
.arg("3.12")
.arg("black")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Tool `black` is already installed
"###);

// Install with Python 3.11 (incompatible).
uv_snapshot!(context.filters(), context.tool_install()
.arg("-p")
.arg("3.11")
.arg("black")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Existing environment for `black` does not satisfy the requested Python interpreter: `Python 3.11`
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ black==24.3.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
Installed: black, blackd
"###);
}

0 comments on commit 7573145

Please sign in to comment.