Skip to content

Comments

Add support for delocating macOS wheels#17336

Open
charliermarsh wants to merge 18 commits intomainfrom
charlie/delocate
Open

Add support for delocating macOS wheels#17336
charliermarsh wants to merge 18 commits intomainfrom
charlie/delocate

Conversation

@charliermarsh
Copy link
Member

Summary

This PR adds support for delocating macOS wheels. The behavior is not yet exposed on the CLI; it's only intended for internal-to-uv use. The implementation is based on https://github.com/matthew-brett/delocate, specifically delocate-listdeps {wheel} to list library dependencies (exposed as list_wheel_dependencies()) and delocate-wheel --require-archs {delocate_archs} -w {dest_dir} {wheel} to copy external libraries into binaries (exposed as delocate_wheel()).

@charliermarsh charliermarsh marked this pull request as ready for review January 6, 2026 17:38
@charliermarsh charliermarsh added the internal A refactor or improvement that is not user-facing label Jan 6, 2026
@charliermarsh charliermarsh force-pushed the charlie/delocate branch 2 times, most recently from 49bf890 to fb8f82c Compare January 6, 2026 18:09
/// Search path for dynamic libraries on macOS (checked before system paths).
///
/// Used during wheel delocating to find library dependencies.
#[attr_added_in("0.9.22")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[attr_added_in("0.9.22")]
#[attr_added_in("next version")]

@charliermarsh charliermarsh marked this pull request as draft January 15, 2026 18:50
@charliermarsh charliermarsh marked this pull request as ready for review January 15, 2026 20:25
// @loader_path and @executable_path are relative to the binary containing the reference.
let parent = binary_path.parent()?;
let resolved = parent.join(relative);
if let Ok(path) = resolved.canonicalize() {
Copy link
Member

Choose a reason for hiding this comment

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

What happens at runtime with a library like this, would this create a wheel with a shared library that fails to load?


// Check macOS version compatibility against target version.
if let Some(version) = target_version {
check_macos_version_compatible(lib_path, version)?;
Copy link
Member

Choose a reason for hiding this comment

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

This and the find_max_macos_version below look off: In the Python delocate: there's a function that runs over the wheel directory after the libs have been copied and computes the minimum of all libs in the new wheel, I think that's libraries + binaries in our terminology (https://github.com/matthew-brett/delocate/blob/a97bc907ea5fab9256e2c92b995b332605fcd98b/delocate/delocating.py#L660-L691). It has the option to check for the required macos version support, but that check only runs if MACOSX_DEPLOYMENT_TARGET is set, otherwise it uses the minimum macos version possible with all binaries/libraries in the wheel. Do we have a test input wheel to check this?

new_name
);

let output = Command::new("install_name_tool")
Copy link
Member

@konstin konstin Jan 21, 2026

Choose a reason for hiding this comment

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

More something for a follow-up PR, but where did we end up on using arwen to avoid the problems with subprocess calls? It's not tested enough rn, but were there any architectural concerns? mid-term, I rather fix bugs in a rust library than working around install_name_tool.

Copy link
Member

@konstin konstin Jan 23, 2026

Choose a reason for hiding this comment

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

CC @geofft who has a way doing it without full arwen, avoiding rewriting the binary for just patching the relevant sections.

PathNotInWheel { path: PathBuf, wheel_dir: PathBuf },

#[error("Unsupported Mach-O format: {0}")]
UnsupportedFormat(String),
Copy link
Member

Choose a reason for hiding this comment

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

This variant is unused

MachOParse(String),

#[error("Dependency not found: {name} (required by {})", required_by.user_display())]
DependencyNotFound { name: String, required_by: PathBuf },
Copy link
Member

Choose a reason for hiding this comment

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

This variant is unused

pub use delocate::{DelocateOptions, delocate_wheel, list_wheel_dependencies};
pub use error::DelocateError;
pub use macho::{Arch, MachOFile};
pub use uv_platform::MacOSVersion;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this external symbol pub-use?

}

#[test]
fn test_macos_version_ordering() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we need that test, it tests a #[derive(Ord)] in combination with the field order.

}

#[test]
fn test_find_dist_info() {
Copy link
Member

Choose a reason for hiding this comment

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

This now covered by the existing test suite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants