From 1af02ce8f27f4349e552a634c83b68302660e25e Mon Sep 17 00:00:00 2001 From: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com> Date: Wed, 15 Jan 2025 21:11:54 +0100 Subject: [PATCH] Patch embedded install path for Python dylib on macOS during `python install` (#10629) ## Summary Fixes #10598 ## Test Plan Looking for input here @zanieb. How/where would you include tests for this? More broadly: do we want a failure to perform the rename to be a hard error? Or should it start out as a warning? --------- Co-authored-by: Zanie Blue --- crates/uv-python/src/installation.rs | 3 ++ crates/uv-python/src/lib.rs | 1 + crates/uv-python/src/macos_dylib.rs | 63 ++++++++++++++++++++++++ crates/uv-python/src/managed.rs | 29 ++++++++++- crates/uv/src/commands/python/install.rs | 3 ++ crates/uv/tests/it/python_install.rs | 44 +++++++++++++++++ 6 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 crates/uv-python/src/macos_dylib.rs diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs index e7fc39940..12a32b0c6 100644 --- a/crates/uv-python/src/installation.rs +++ b/crates/uv-python/src/installation.rs @@ -165,6 +165,9 @@ impl PythonInstallation { installed.ensure_externally_managed()?; installed.ensure_sysconfig_patched()?; installed.ensure_canonical_executables()?; + if let Err(e) = installed.ensure_dylib_patched() { + e.warn_user(&installed); + } Ok(Self { source: PythonSource::Managed, diff --git a/crates/uv-python/src/lib.rs b/crates/uv-python/src/lib.rs index ea5b3e572..0eb3cb36f 100644 --- a/crates/uv-python/src/lib.rs +++ b/crates/uv-python/src/lib.rs @@ -30,6 +30,7 @@ mod implementation; mod installation; mod interpreter; mod libc; +pub mod macos_dylib; pub mod managed; #[cfg(windows)] mod microsoft_store; diff --git a/crates/uv-python/src/macos_dylib.rs b/crates/uv-python/src/macos_dylib.rs new file mode 100644 index 000000000..7294497b3 --- /dev/null +++ b/crates/uv-python/src/macos_dylib.rs @@ -0,0 +1,63 @@ +use std::{io::ErrorKind, path::PathBuf}; + +use uv_fs::Simplified as _; +use uv_warnings::warn_user; + +use crate::managed::ManagedPythonInstallation; + +pub fn patch_dylib_install_name(dylib: PathBuf) -> Result<(), Error> { + let output = match std::process::Command::new("install_name_tool") + .arg("-id") + .arg(&dylib) + .arg(&dylib) + .output() + { + Ok(output) => output, + Err(e) => { + let e = if e.kind() == ErrorKind::NotFound { + Error::MissingInstallNameTool + } else { + e.into() + }; + return Err(e); + } + }; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + return Err(Error::RenameError { dylib, stderr }); + } + + Ok(()) +} + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error("`install_name_tool` is not available on this system. +This utility is part of macOS Developer Tools. Please ensure that the Xcode Command Line Tools are installed by running: + + xcode-select --install + +For more information, see: https://developer.apple.com/xcode/")] + MissingInstallNameTool, + #[error("Failed to update the install name of the Python dynamic library located at `{}`", dylib.user_display())] + RenameError { dylib: PathBuf, stderr: String }, +} + +impl Error { + /// Emit a user-friendly warning about the patching failure. + pub fn warn_user(&self, installation: &ManagedPythonInstallation) { + let error = if tracing::enabled!(tracing::Level::DEBUG) { + format!("\nUnderlying error: {self}") + } else { + String::new() + }; + warn_user!( + "Failed to patch the install name of the dynamic library for {}. This may cause issues when building Python native extensions.{}", + installation.executable().simplified_display(), + error + ); + } +} diff --git a/crates/uv-python/src/managed.rs b/crates/uv-python/src/managed.rs index 1b35e8c2c..90f2e65de 100644 --- a/crates/uv-python/src/managed.rs +++ b/crates/uv-python/src/managed.rs @@ -25,7 +25,8 @@ use crate::libc::LibcDetectionError; use crate::platform::Error as PlatformError; use crate::platform::{Arch, Libc, Os}; use crate::python_version::PythonVersion; -use crate::{sysconfig, PythonRequest, PythonVariant}; +use crate::{macos_dylib, sysconfig, PythonRequest, PythonVariant}; + #[derive(Error, Debug)] pub enum Error { #[error(transparent)] @@ -88,6 +89,8 @@ pub enum Error { NameParseError(#[from] installation::PythonInstallationKeyError), #[error(transparent)] LibcDetection(#[from] LibcDetectionError), + #[error(transparent)] + MacOsDylib(#[from] macos_dylib::Error), } /// A collection of uv-managed Python installations installed on the current system. #[derive(Debug, Clone, Eq, PartialEq)] @@ -508,6 +511,28 @@ impl ManagedPythonInstallation { Ok(()) } + /// On macOS, ensure that the `install_name` for the Python dylib is set + /// correctly, rather than pointing at `/install/lib/libpython{version}.dylib`. + /// This is necessary to ensure that native extensions written in Rust + /// link to the correct location for the Python library. + /// + /// See for more information. + pub fn ensure_dylib_patched(&self) -> Result<(), macos_dylib::Error> { + if cfg!(target_os = "macos") { + if *self.implementation() == ImplementationName::CPython { + let dylib_path = self.python_dir().join("lib").join(format!( + "{}python{}{}{}", + std::env::consts::DLL_PREFIX, + self.key.version().python_version(), + self.key.variant().suffix(), + std::env::consts::DLL_SUFFIX + )); + macos_dylib::patch_dylib_install_name(dylib_path)?; + } + } + Ok(()) + } + /// Create a link to the managed Python executable. /// /// If the file already exists at the target path, an error will be returned. @@ -603,7 +628,7 @@ impl ManagedPythonInstallation { } /// Generate a platform portion of a key from the environment. -fn platform_key_from_env() -> Result { +pub fn platform_key_from_env() -> Result { let os = Os::from_env(); let arch = Arch::from_env(); let libc = Libc::from_env()?; diff --git a/crates/uv/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs index 5f482fea5..23ab9fd46 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -312,6 +312,9 @@ pub(crate) async fn install( installation.ensure_externally_managed()?; installation.ensure_sysconfig_patched()?; installation.ensure_canonical_executables()?; + if let Err(e) = installation.ensure_dylib_patched() { + e.warn_user(installation); + } if preview.is_disabled() { debug!("Skipping installation of Python executables, use `--preview` to enable."); diff --git a/crates/uv/tests/it/python_install.rs b/crates/uv/tests/it/python_install.rs index f521ee3f5..1e132cc06 100644 --- a/crates/uv/tests/it/python_install.rs +++ b/crates/uv/tests/it/python_install.rs @@ -876,3 +876,47 @@ fn python_install_preview_broken_link() { ); }); } + +#[cfg(target_os = "macos")] +#[test] +fn python_dylib_install_name_is_patched_on_install() { + use assert_cmd::assert::OutputAssertExt; + use uv_python::managed::platform_key_from_env; + + let context: TestContext = TestContext::new_with_versions(&[]).with_filtered_python_keys(); + + // Install the latest version + context + .python_install() + .arg("--preview") + .arg("3.13.1") + .assert() + .success(); + + let dylib = context + .temp_dir + .child("managed") + .child(format!( + "cpython-3.13.1-{}", + platform_key_from_env().unwrap() + )) + .child("lib") + .child(format!( + "{}python3.13{}", + std::env::consts::DLL_PREFIX, + std::env::consts::DLL_SUFFIX + )); + + let mut cmd = std::process::Command::new("otool"); + cmd.arg("-D").arg(dylib.as_ref()); + + uv_snapshot!(context.filters(), cmd, @r###" + success: true + exit_code: 0 + ----- stdout ----- + [TEMP_DIR]/managed/cpython-3.13.1-[PLATFORM]/lib/libpython3.13.dylib: + [TEMP_DIR]/managed/cpython-3.13.1-[PLATFORM]/lib/libpython3.13.dylib + + ----- stderr ----- + "###); +}