Skip to content

Commit

Permalink
Refactor dependency discovery to handle multiple architectures (#230)
Browse files Browse the repository at this point in the history 
* Add `-arch all` to all `otool` calls

* Switch to _get_install_ids

Deprecate get_install_id

Update lower bound for typing_extensions

* Fix get_install_names for multiple architectures

Refactor unique_everseen-like function

* Update mock test results

Differing install names are handled, bad install ids are detected

* Update MacOS runners

Remove older deprecated runners and add newer ones

* Refactor dependency collection to account for multiple architectures

Now tracks install names and rpaths per-architecture.

Deprecated several older functions.

Refactored related functions to support pathlib.

* Avoid pathlib for library path data

Library paths can be sensitive and pathlib will mess with the trailing slash

* Update changelog

* Note deprecated public functions
  • Loading branch information
@HexDecimal
HexDecimal authored Dec 17, 2024
1 parent dfb21c8 commit 9169a66
Show file tree
Hide file tree
Showing 8 changed files with 309 additions and 141 deletions.
13 changes: 8 additions & 5 deletions .github/workflows/python-package.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ jobs:

tests:
needs: [pre-commit, mypy]
runs-on: macos-12
runs-on: macos-13
steps:
- uses: actions/checkout@v4
with:
submodules: true
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: "14.0"
xcode-version: "14.2"
- uses: actions/setup-python@v5
with:
python-version: "3.x"
Expand Down Expand Up @@ -134,12 +134,15 @@ jobs:
fail-fast: true
matrix:
include:
- os: "macos-12"
xcode: "14.0"
python: "3.x"
- os: "macos-13"
xcode: "14.2"
python: "3.x"
- os: "macos-14"
xcode: "15.4"
python: "3.x"
- os: "macos-15"
xcode: "16.0"
python: "3.x"
- os: "ubuntu-latest"
python: "3.9"
- os: "ubuntu-latest"
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ The `wheel_makers` directory holds scripts used to make test data. GitHub Action

Use [pathlib](https://docs.python.org/3/library/pathlib.html) for any new code using paths.
Refactor any touched functions to use pathlib when it does not break backwards compatibility.
Prefer using `PurePosixPath` to handle relative library paths returned from MacOS tools such as `otool`.
Prefer using `str` to handle paths returned from MacOS tools such as `otool`.

All new functions must have [type hints](https://mypy.readthedocs.io/en/stable/getting_started.html).
All touched functions must be refactored to use type hints, including test functions.
Expand Down
15 changes: 15 additions & 0 deletions Changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ rules on making a good Changelog.
- `patch_wheel` function raises `FileNotFoundError` instead of `ValueError` on
missing patch files.

### Deprecated

- `get_rpaths` and `get_install_id` are deprecated due to not supporting
architectures.
- `unique_by_index` is deprecated. Use more-itertools unique_everseen instead.

### Fixed

- Fixed `NotImplementedError` when libraries depend on differing binaries
per-architecture.
[#230](https://github.com/matthew-brett/delocate/pull/230)
- Now checks all architectures instead of an arbitrary default.
This was causing inconsistent behavior across MacOS versions.
[#230](https://github.com/matthew-brett/delocate/pull/230)

### Removed

- Dropped support for Python 3.7 and Python 3.8.
Expand Down
193 changes: 109 additions & 84 deletions delocate/libsana.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,21 @@
import sys
import warnings
from collections.abc import Iterable, Iterator
from os.path import basename, dirname, realpath
from os import PathLike
from os.path import join as pjoin
from os.path import realpath
from pathlib import Path
from typing import (
Callable,
)

from typing_extensions import deprecated

from .tmpdirs import TemporaryDirectory
from .tools import (
_get_install_names,
_get_rpaths,
get_environment_variable_paths,
get_install_names,
get_rpaths,
zip2dir,
)

Expand All @@ -40,8 +44,8 @@ def _filter_system_libs(libname: str) -> bool:


def get_dependencies(
lib_fname: str,
executable_path: str | None = None,
lib_fname: str | PathLike[str],
executable_path: str | PathLike[str] | None = None,
filt_func: Callable[[str], bool] = lambda filepath: True,
) -> Iterator[tuple[str | None, str]]:
"""Find and yield the real paths of dependencies of the library `lib_fname`.
Expand All @@ -54,9 +58,9 @@ def get_dependencies(
Parameters
----------
lib_fname : str
lib_fname : str or PathLike
The library to fetch dependencies from. Must be an existing file.
executable_path : str, optional
executable_path : str or PathLike, optional
An alternative path to use for resolving `@executable_path`.
filt_func : callable, optional
A callable which accepts filename as argument and returns True if we
Expand All @@ -82,50 +86,66 @@ def get_dependencies(
DependencyNotFound
When `lib_fname` does not exist.
"""
if not filt_func(lib_fname):
lib_fname = Path(lib_fname)
if not filt_func(str(lib_fname)):
logger.debug(f"Ignoring dependencies of {lib_fname}")
return
if not os.path.isfile(lib_fname):
if not _filter_system_libs(lib_fname):
if not lib_fname.is_file():
if not _filter_system_libs(str(lib_fname)):
logger.debug(
"Ignoring missing library %s because it is a system library.",
lib_fname,
)
return
raise DependencyNotFound(lib_fname)
rpaths = get_rpaths(lib_fname) + get_environment_variable_paths()
for install_name in get_install_names(lib_fname):
try:
if install_name.startswith("@"):
dependency_path = resolve_dynamic_paths(
install_name,
rpaths,
loader_path=dirname(lib_fname),
executable_path=executable_path,
)
else:
dependency_path = search_environment_for_lib(install_name)
if not os.path.isfile(dependency_path):
if not _filter_system_libs(dependency_path):
logger.debug(
"Skipped missing dependency %s"
" because it is a system library.",
dependency_path,

environment_paths = get_environment_variable_paths()
rpaths = {
arch: [*paths, *environment_paths]
for arch, paths in _get_rpaths(lib_fname).items()
}

install_name_seen = set()
for arch, install_names in _get_install_names(lib_fname).items():
for install_name in install_names:
if install_name in install_name_seen:
# The same dependency listed by multiple architectures should
# only be counted once.
continue
install_name_seen.add(install_name)
try:
if install_name.startswith("@"):
dependency_path = resolve_dynamic_paths(
install_name,
rpaths[arch],
loader_path=lib_fname.parent,
executable_path=executable_path,
)
else:
raise DependencyNotFound(dependency_path)
if dependency_path != install_name:
logger.debug(
"%s resolved to: %s", install_name, dependency_path
dependency_path = search_environment_for_lib(install_name)
if not Path(dependency_path).is_file():
if not _filter_system_libs(dependency_path):
logger.debug(
"Skipped missing dependency %s"
" because it is a system library.",
dependency_path,
)
else:
raise DependencyNotFound(dependency_path)
if dependency_path != install_name:
logger.debug(
"%s resolved to: %s", install_name, dependency_path
)
yield dependency_path, str(install_name)
except DependencyNotFound:
message = (
f"\n{install_name} not found:\n Needed by: {lib_fname}"
)
yield dependency_path, install_name
except DependencyNotFound:
message = f"\n{install_name} not found:\n Needed by: {lib_fname}"
if install_name.startswith("@rpath"):
message += "\n Search path:\n " + "\n ".join(rpaths)
logger.error(message)
# At this point install_name is known to be a bad path.
yield None, install_name
if Path(install_name).parts[0] == "@rpath":
message += "\n Search path:\n " + "\n ".join(rpaths)
logger.error(message)
# At this point install_name is known to be a bad path.
yield None, str(install_name)


def walk_library(
Expand Down Expand Up @@ -459,10 +479,10 @@ def tree_libs(


def resolve_dynamic_paths(
lib_path: str,
rpaths: Iterable[str],
loader_path: str,
executable_path: str | None = None,
lib_path: str | PathLike[str],
rpaths: Iterable[str | PathLike[str]],
loader_path: str | PathLike[str],
executable_path: str | PathLike[str] | None = None,
) -> str:
"""Return `lib_path` with any special runtime linking names resolved.
Expand All @@ -475,15 +495,15 @@ def resolve_dynamic_paths(
Parameters
----------
lib_path : str
lib_path : str or PathLike
The path to a library file, which may or may not be a relative path
starting with `@rpath`, `@loader_path`, or `@executable_path`.
rpaths : sequence of str
rpaths : sequence of str or PathLike
A sequence of search paths, usually gotten from a call to `get_rpaths`.
loader_path : str
loader_path : str or PathLike
The path to be used for `@loader_path`.
This should be the directory of the library which is loading `lib_path`.
executable_path : None or str, optional
executable_path : None or str or PathLike, optional
The path to be used for `@executable_path`.
If None is given then the path of the Python executable will be used.
Expand All @@ -498,39 +518,48 @@ def resolve_dynamic_paths(
When `lib_path` has `@rpath` in it but no library can be found on any
of the provided `rpaths`.
"""
lib_path = Path(lib_path)

if executable_path is None:
executable_path = dirname(sys.executable)
executable_path = Path(sys.executable).parent

if not lib_path.startswith(
("@rpath/", "@loader_path/", "@executable_path/")
if lib_path.parts[0] not in (
("@rpath", "@loader_path", "@executable_path")
):
return realpath(lib_path)
return str(Path(lib_path).resolve())

if lib_path.startswith("@loader_path/"):
paths_to_search = []
if lib_path.parts[0] == "@loader_path":
paths_to_search = [loader_path]
elif lib_path.startswith("@executable_path/"):
elif lib_path.parts[0] == "@executable_path":
paths_to_search = [executable_path]
elif lib_path.startswith("@rpath/"):
elif lib_path.parts[0] == "@rpath":
paths_to_search = list(rpaths)

# these paths are searched by the macos loader in order if the
# library is not in the previous paths.
paths_to_search.extend(_default_paths_to_search)

rel_path = lib_path.split("/", 1)[1]
rel_path = Path(*lib_path.parts[1:])
for prefix_path in paths_to_search:
try:
abs_path = resolve_dynamic_paths(
pjoin(prefix_path, rel_path), (), loader_path, executable_path
)
abs_path = Path(
resolve_dynamic_paths(
Path(prefix_path, rel_path),
(),
loader_path,
executable_path,
)
).resolve()
except DependencyNotFound:
continue
if os.path.exists(abs_path):
return realpath(abs_path)
if abs_path.exists():
return str(abs_path)

raise DependencyNotFound(lib_path)


@deprecated("This function was replaced by resolve_dynamic_paths")
def resolve_rpath(lib_path: str, rpaths: Iterable[str]) -> str:
"""Return `lib_path` with its `@rpath` resolved.
Expand Down Expand Up @@ -579,7 +608,7 @@ def resolve_rpath(lib_path: str, rpaths: Iterable[str]) -> str:
return lib_path


def search_environment_for_lib(lib_path: str) -> str:
def search_environment_for_lib(lib_path: str | PathLike[str]) -> str:
"""Search common environment variables for `lib_path`.
We'll use a single approach here:
Expand All @@ -601,7 +630,7 @@ def search_environment_for_lib(lib_path: str) -> str:
Parameters
----------
lib_path : str
lib_path : str or PathLike
Name of the library to search for
Returns
Expand All @@ -610,27 +639,21 @@ def search_environment_for_lib(lib_path: str) -> str:
Real path of the first found location, if it can be found, or
``realpath(lib_path)`` if it cannot.
"""
lib_basename = basename(lib_path)
potential_library_locations = []

# 1. Search on DYLD_LIBRARY_PATH
potential_library_locations += _paths_from_var(
"DYLD_LIBRARY_PATH", lib_basename
)

# 2. Search for realpath(lib_path)
potential_library_locations.append(realpath(lib_path))

# 3. Search on DYLD_FALLBACK_LIBRARY_PATH
potential_library_locations += _paths_from_var(
"DYLD_FALLBACK_LIBRARY_PATH", lib_basename
)
lib_path = Path(lib_path)
potential_library_locations: list[Path] = [
# 1. Search on DYLD_LIBRARY_PATH
*_paths_from_var("DYLD_LIBRARY_PATH", lib_path.name),
# 2. Search for realpath(lib_path)
lib_path.resolve(),
# 3. Search on DYLD_FALLBACK_LIBRARY_PATH
*_paths_from_var("DYLD_FALLBACK_LIBRARY_PATH", lib_path.name),
]

for location in potential_library_locations:
if os.path.exists(location):
if location.exists():
# See GH#133 for why we return the realpath here if it can be found
return realpath(location)
return realpath(lib_path)
return str(location.resolve())
return str(lib_path.resolve())


def get_prefix_stripper(strip_prefix: str) -> Callable[[str], str]:
Expand Down Expand Up @@ -754,8 +777,10 @@ def wheel_libs(
return stripped_lib_dict(lib_dict, realpath(tmpdir) + os.path.sep)


def _paths_from_var(varname: str, lib_basename: str) -> list[str]:
def _paths_from_var(
varname: str, lib_basename: str | PathLike[str]
) -> Iterator[Path]:
var = os.environ.get(varname)
if var is None:
return []
return [pjoin(path, lib_basename) for path in var.split(os.pathsep)]
return
yield from (Path(path, lib_basename) for path in var.split(os.pathsep))
Loading

0 comments on commit 9169a66

Please sign in to comment.