You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ld64 on MacOS lacks --exclude-libs,ALL, which leads to symbols from statically linked libraries"leaking" in our wheel.
This PR makes sure that exports are restricted to the module init symbol to stop dyld weak-coalescing of statically-linked deps (e.g. pyarrow's bundled AWS SDK in libarrow) from aborting in aws_fatal_assert.
Any other comments?
Checklist
Checklist for code changes...
Have you updated the relevant docstrings, documentation and copyright notice?
Build-only change restricting the macOS extension's exported symbols to _PyInit_arcticdb_ext, with packaging and runtime regression tests. The change is sound: the symbol name correctly accounts for the macOS leading-underscore ABI, the export restriction only fires on the non-conda (wheel) AppleClang path where HIDE_LINKED_SYMBOLS is ON, which exactly matches the MACOS_WHEEL_BUILD test skip guard, and pybind's default-visibility PyInit survives the restriction. Both a nm-based packaging check and a pyarrow-before-arcticdb runtime regression are included.
The latest commits add unrelated test/CI cleanups: pinning the conda Windows runner to windows-2022, introducing an unparametrized latest_venv session fixture (used by test_comp_venv_loading_path), and removing stray trailing commas in two @pytest.mark.parametrize argname strings. All are benign — no correctness, safety, or behavioral impact.
PR Title and Description
Description is empty (template only). Per the review guidelines the description should explain what changed and why — here, that ld64 lacks --exclude-libs,ALL, so exports are restricted to the module init symbol to stop dyld weak-coalescing of statically-linked deps (e.g. pyarrow's bundled AWS SDK in libarrow) from aborting in aws_fatal_assert. The context already lives in the code comments and test docstrings; please lift a sentence or two into the description.
No correctness, memory-safety, API, on-disk-format, or build-configuration issues found. Labels (no-release-notes, patch) are appropriate for a build/test-only change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
no-release-notesThis PR shouldn't be added to release notes.patchSmall change, should increase patch version
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
What does this implement or fix?
ld64 on MacOS lacks
--exclude-libs,ALL, which leads to symbols from statically linked libraries"leaking" in our wheel.This PR makes sure that exports are restricted to the module init symbol to stop dyld weak-coalescing of statically-linked deps (e.g. pyarrow's bundled AWS SDK in libarrow) from aborting in aws_fatal_assert.
Any other comments?
Checklist
Checklist for code changes...