Skip to content

Commit

Permalink
Preserve symlinks when creating virtual environments
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 22, 2024
1 parent 1b77055 commit 552870d
Show file tree
Hide file tree
Showing 15 changed files with 73 additions and 9 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-build-frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ doctest = false
workspace = true

[dependencies]
uv-cache = { workspace = true }
uv-configuration = { workspace = true }
uv-distribution = { workspace = true }
uv-distribution-types = { workspace = true }
Expand Down
4 changes: 3 additions & 1 deletion crates/uv-build-frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use tokio::io::AsyncBufReadExt;
use tokio::process::Command;
use tokio::sync::{Mutex, Semaphore};
use tracing::{debug, info_span, instrument, Instrument};

use uv_cache::Cache;
use uv_configuration::{BuildKind, BuildOutput, ConfigSettings, LowerBound, SourceStrategy};
use uv_distribution::RequiresDist;
use uv_distribution_types::{IndexLocations, Resolution};
Expand Down Expand Up @@ -260,6 +260,7 @@ impl SourceBuild {
mut environment_variables: FxHashMap<OsString, OsString>,
level: BuildOutput,
concurrent_builds: usize,
cache: &Cache,
) -> Result<Self, Error> {
let temp_dir = build_context.cache().environment()?;

Expand Down Expand Up @@ -306,6 +307,7 @@ impl SourceBuild {
false,
false,
false,
cache,
)?
};

Expand Down
1 change: 1 addition & 0 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.build_extra_env_vars.clone(),
build_output,
self.concurrency.builds,
self.cache,
)
.boxed_local()
.await?;
Expand Down
21 changes: 20 additions & 1 deletion crates/uv-fs/src/path.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Cow;
use std::io;
use std::path::{Component, Path, PathBuf};
use std::sync::LazyLock;

Expand Down Expand Up @@ -236,7 +237,7 @@ pub fn normalize_path(path: &Path) -> PathBuf {
}

/// Like `fs_err::canonicalize`, but avoids attempting to resolve symlinks on Windows.
pub fn canonicalize_executable(path: impl AsRef<Path>) -> std::io::Result<PathBuf> {
pub fn canonicalize_executable(path: impl AsRef<Path>) -> io::Result<PathBuf> {
let path = path.as_ref();
debug_assert!(
path.is_absolute(),
Expand All @@ -250,6 +251,24 @@ pub fn canonicalize_executable(path: impl AsRef<Path>) -> std::io::Result<PathBu
}
}

/// Resolve a symlink for an executable. Unlike `fs_err::canonicalize`, this does not resolve
/// symlinks recursively.
pub fn read_executable_link(executable: &Path) -> Result<Option<PathBuf>, io::Error> {
let Ok(link) = executable.read_link() else {
return Ok(None);
};
if link.is_absolute() {
Ok(Some(link))
} else {
executable
.parent()
.map(|parent| parent.join(link))
.as_deref()
.map(normalize_absolute_path)
.transpose()
}
}

/// Compute a path describing `path` relative to `base`.
///
/// `lib/python/site-packages/foo/__init__.py` and `lib/python/site-packages` -> `foo/__init__.py`
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ impl InstalledTools {
&self,
name: &PackageName,
interpreter: Interpreter,
cache: &Cache,
) -> Result<PythonEnvironment, Error> {
let environment_path = self.tool_dir(name);

Expand Down Expand Up @@ -270,6 +271,7 @@ impl InstalledTools {
false,
false,
false,
cache,
)?;

Ok(venv)
Expand Down
1 change: 1 addition & 0 deletions crates/uv-virtualenv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ doctest = false
workspace = true

[dependencies]
uv-cache = { workspace = true }
uv-fs = { workspace = true }
uv-platform-tags = { workspace = true }
uv-pypi-types = { workspace = true }
Expand Down
6 changes: 5 additions & 1 deletion crates/uv-virtualenv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::io;
use std::path::Path;

use thiserror::Error;

use uv_cache::Cache;
use uv_platform_tags::PlatformError;
use uv_python::{Interpreter, PythonEnvironment};

Expand All @@ -12,6 +12,8 @@ mod virtualenv;
pub enum Error {
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
Interpreter(#[from] uv_python::InterpreterError),
#[error("Failed to determine Python interpreter to use")]
Discovery(#[from] uv_python::DiscoveryError),
#[error("Failed to determine Python interpreter to use")]
Expand Down Expand Up @@ -55,6 +57,7 @@ pub fn create_venv(
allow_existing: bool,
relocatable: bool,
seed: bool,
cache: &Cache,
) -> Result<PythonEnvironment, Error> {
// Create the virtualenv at the given location.
let virtualenv = virtualenv::create(
Expand All @@ -65,6 +68,7 @@ pub fn create_venv(
allow_existing,
relocatable,
seed,
cache,
)?;

// Create the corresponding `PythonEnvironment`.
Expand Down
33 changes: 29 additions & 4 deletions crates/uv-virtualenv/src/virtualenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use fs_err as fs;
use fs_err::File;
use itertools::Itertools;
use tracing::debug;

use uv_cache::Cache;
use uv_fs::{cachedir, Simplified, CWD};
use uv_pypi_types::Scheme;
use uv_python::{Interpreter, VirtualEnvironment};
Expand Down Expand Up @@ -52,15 +52,40 @@ pub(crate) fn create(
allow_existing: bool,
relocatable: bool,
seed: bool,
cache: &Cache,
) -> Result<VirtualEnvironment, Error> {
// Determine the base Python executable; that is, the Python executable that should be
// considered the "base" for the virtual environment. This is typically the Python executable
// from the [`Interpreter`]; however, if the interpreter is a virtual environment itself, then
// the base Python executable is the Python executable of the interpreter's base interpreter.
let base_python = if cfg!(unix) {
// On Unix, follow symlinks to resolve the base interpreter, since the Python executable in
// a virtual environment is a symlink to the base interpreter.
uv_fs::canonicalize_executable(interpreter.sys_executable())?
// If we're in virtual environment, resolve symlinks until we find a non-virtual interpreter.
if interpreter.is_virtualenv() {
if let Some(base_executable) =
uv_fs::read_executable_link(interpreter.sys_executable())?
{
let mut base_interpreter = Interpreter::query(base_executable, cache)?;
while base_interpreter.is_virtualenv() {
let Some(base_executable) =
uv_fs::read_executable_link(base_interpreter.sys_executable())?
else {
break;
};
base_interpreter = Interpreter::query(base_executable, cache)?;
}
base_interpreter.sys_executable().to_path_buf()
} else {
// If the interpreter isn't a symlink, use `sys._base_executable` or, as a last
// resort, `sys.executable`.
if let Some(base_executable) = interpreter.sys_base_executable() {
base_executable.to_path_buf()
} else {
interpreter.sys_executable().to_path_buf()
}
}
} else {
interpreter.sys_executable().to_path_buf()
}
} else if cfg!(windows) {
// On Windows, follow `virtualenv`. If we're in a virtual environment, use
// `sys._base_executable` if it exists; if not, use `sys.base_prefix`. For example, with
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl CachedEnvironment {
false,
true,
false,
cache,
)?;

sync_environment(
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ pub(crate) async fn get_or_init_environment(
false,
false,
false,
cache,
)?)
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ pub(crate) async fn run(
false,
false,
false,
cache,
)?;

Some(environment.into_interpreter())
Expand Down Expand Up @@ -511,6 +512,7 @@ pub(crate) async fn run(
false,
false,
false,
cache,
)?
} else {
// If we're not isolating the environment, reuse the base environment for the
Expand Down Expand Up @@ -658,6 +660,7 @@ pub(crate) async fn run(
false,
false,
false,
cache,
)?;
venv.into_interpreter()
} else {
Expand Down Expand Up @@ -707,6 +710,7 @@ pub(crate) async fn run(
false,
false,
false,
cache,
)?
}
Some(spec) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ pub(crate) async fn install(
)
.await?;

let environment = installed_tools.create_environment(&from.name, interpreter)?;
let environment = installed_tools.create_environment(&from.name, interpreter, &cache)?;

// At this point, we removed any existing environment, so we should remove any of its
// executables.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/tool/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ async fn upgrade_tool(
)
.await?;

let environment = installed_tools.create_environment(name, interpreter.clone())?;
let environment = installed_tools.create_environment(name, interpreter.clone(), cache)?;

let environment = sync_environment(
environment,
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ async fn venv_impl(
allow_existing,
relocatable,
seed,
cache,
)
.map_err(VenvError::Creation)?;

Expand Down

0 comments on commit 552870d

Please sign in to comment.