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

Patch embedded install path for Python dylib on macOS during python install #10629

Merged
merged 5 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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: 13 additions & 0 deletions crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use tracing::{debug, info};

use uv_cache::Cache;
use uv_client::BaseClientBuilder;
use uv_fs::Simplified as _;
use uv_pep440::{Prerelease, Version};
use uv_warnings::warn_user;

use crate::discovery::{
find_best_python_installation, find_python_installation, EnvironmentPreference, PythonRequest,
Expand Down Expand Up @@ -165,6 +167,17 @@ impl PythonInstallation {
installed.ensure_externally_managed()?;
installed.ensure_sysconfig_patched()?;
installed.ensure_canonical_executables()?;
if let Err(e) = installed.ensure_dylib_patched() {
warn_user!(
"Failed to patch the install name of the dynamic library for {}. This may cause issues when building Python native extensions.",
installed.executable().simplified_display(),
);
debug!(
"Failed to patch the install name of the dynamic library for {}. This may cause issues when building Python native extensions.\n{}",
installed.executable().simplified_display(),
e
);
zanieb marked this conversation as resolved.
Show resolved Hide resolved
}

Ok(Self {
source: PythonSource::Managed,
Expand Down
1 change: 1 addition & 0 deletions crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ mod implementation;
mod installation;
mod interpreter;
mod libc;
pub mod macos_dylib;
pub mod managed;
#[cfg(windows)]
mod microsoft_store;
Expand Down
44 changes: 44 additions & 0 deletions crates/uv-python/src/macos_dylib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use std::{io::ErrorKind, path::PathBuf};

use uv_fs::Simplified as _;

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,
zanieb marked this conversation as resolved.
Show resolved Hide resolved
#[error("Failed to update the install name of the Python dynamic library located at `{}`", dylib.user_display())]
RenameError { dylib: PathBuf, stderr: String },
}
27 changes: 26 additions & 1 deletion crates/uv-python/src/managed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 <https://github.com/astral-sh/uv/issues/10598> for more information.
pub fn ensure_dylib_patched(&self) -> Result<(), 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.
Expand Down
11 changes: 11 additions & 0 deletions crates/uv/src/commands/python/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,17 @@ 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() {
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(),
);
debug!(
"Failed to patch the install name of the dynamic library for {}. This may cause issues when building Python native extensions.\n{}",
installation.executable().simplified_display(),
e
);
}

if preview.is_disabled() {
debug!("Skipping installation of Python executables, use `--preview` to enable.");
Expand Down
40 changes: 40 additions & 0 deletions crates/uv/tests/it/python_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,3 +876,43 @@ 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;

let context: TestContext = TestContext::new_with_versions(&[]).with_filtered_python_keys();

// Install the latest version
context
.python_install()
.arg("--preview")
.arg("3.13")
.assert()
.success();

let dylib = context
.temp_dir
.child("managed")
.child("cpython-3.13.1-macos-aarch64-none")
Copy link
Contributor Author

@LukeMathWalker LukeMathWalker Jan 15, 2025

Choose a reason for hiding this comment

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

This bit is not going to fly, since it may run on multiple platforms, not just "aarch64-none".
I haven't found a way to get the "current" platform tag though. Any pointers? @zanieb

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps platform_key_from_env

Copy link
Member

Choose a reason for hiding this comment

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

I did this in 4f2dfba quick so we can release it today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

.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 -----
"###);
}