fix: include libfoundation_models.dylib in wheels and build for multiple python versions #3
Conversation
…ple Python versions The dylib was only included in source distributions via MANIFEST.in but not in wheels. Wheels require files to be part of package data. Now copies the dylib to the package directory during build and includes it via package_data in pyproject.toml. Also improved CI workflow to build wheels for Python 3.9-3.13 with explicit dylib verification before publishing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughBuild and packaging now compile and bundle libfoundation_models.dylib into the package and wheels; CI builds macOS wheels across a Python 3.9–3.14 matrix with per-wheel dylib verification; package metadata bumped to 0.1.4 and setup.py updated to build and copy the dylib into the package. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI (publish-to-pypi)
participant Runner as Matrix Runner
participant Job as build-wheels
participant Setup as setup.py (build steps)
participant Swift as Swift compiler
participant Wheel as Wheel builder
participant Storage as Artifact storage
participant Publish as publish job
CI->>Runner: start macOS Python matrix (3.9..3.14)
Runner->>Job: run build-wheels for matrix.python-version
Job->>Setup: invoke build (BuildPyWithDylib / BuildSwiftThenExt)
Setup->>Swift: build_swift_dylib() — compile libfoundation_models.dylib
Swift-->>Setup: compiled libfoundation_models.dylib
Setup->>Setup: copy dylib into package dir and build output
Setup->>Wheel: build wheel (includes dylib)
Wheel-->>Storage: upload wheel artifact (wheel-${matrix.python-version})
Note right of Storage: CI verifies each wheel contains libfoundation_models.dylib
CI->>Publish: download merged artifacts
Publish->>Storage: verify every downloaded wheel contains dylib
Publish->>Publish: publish to PyPI if verifications pass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
27-27: Update classifiers and requires-python to match CI coverage.The project declares Python 3.8 and 3.9 support in classifiers and
requires-python = ">=3.8", but the CI workflows don't cover them:test.ymltests only 3.10-3.12, andpublish-to-pypi.ymlbuilds wheels only for 3.9-3.13. Python 3.8 reached end-of-life in October 2024. While the codebase uses Python 3.8+ syntax (walrus operator), neither 3.8 nor 3.9 are actively tested.Update
pyproject.tomllines 27 and 36 to:
- Remove the Python 3.8 classifier
- Update
requires-pythonto">=3.9"to match actual CI/CD coverageAlternatively, add Python 3.8 and 3.9 to
test.ymlif active support is required (not recommended for EOL versions).
🧹 Nitpick comments (1)
setup.py (1)
46-54: Consider checking package dylib in rebuild logic.The rebuild logic checks if the lib dylib needs updating but doesn't verify that
PKG_DYLIB_PATHexists. If someone manually deletesapplefoundationmodels/libfoundation_models.dylibbut the lib dylib is current, the rebuild will be skipped and the package dylib won't be restored, potentially causing wheel builds to fail.Apply this diff to handle the edge case:
# Check if we need to rebuild needs_rebuild = ( not DYLIB_PATH.exists() or + not PKG_DYLIB_PATH.exists() or not SWIFT_SRC.exists() or SWIFT_SRC.stat().st_mtime > DYLIB_PATH.stat().st_mtime ) if not needs_rebuild: - print(f"Swift dylib is up to date: {DYLIB_PATH}") + print(f"Swift dylib and package copy are up to date: {DYLIB_PATH}") return
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/publish-to-pypi.yml(1 hunks)pyproject.toml(2 hunks)setup.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
setup.py
109-109: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (8)
pyproject.toml (2)
7-7: Version bump looks appropriate.The version increment to 0.1.4 is suitable for a bugfix release that addresses the missing dylib in wheels.
54-54: Essential change to include dylib in wheels.Adding
"*.dylib"to package-data ensures the shared library is included when building wheels. This works correctly with the setup.py changes that copy the dylib into the package directory.setup.py (2)
11-11: Good path organization for package-aware builds.The addition of
shutil,PKG_DIR, andPKG_DYLIB_PATHclearly establishes where the dylib should be copied for wheel distribution. DerivingSWIFT_DIRfromPKG_DIRis cleaner than the previous hardcoded path.Also applies to: 21-22, 26-26
134-137: RPATH configuration correctly handles both development and wheel installations.The three RPATH entries ensure the extension can find the dylib in different scenarios:
@loader_pathfinds the dylib in the same directory as the .so (works for wheels where both are inapplefoundationmodels/)@loader_path/../libhandles traditional package structures- Absolute
LIB_DIRpath supports development buildsThis configuration correctly supports the new wheel packaging approach.
.github/workflows/publish-to-pypi.yml (4)
13-21: Excellent addition of multi-version wheel building.The matrix strategy with Python 3.9-3.13 ensures broad compatibility. Setting
fail-fast: falseallows all versions to build even if one encounters issues, which is important for comprehensive release coverage.
46-48: Explicit wheel-only build is clearer.Using
python -m build --wheelexplicitly builds only the wheel distribution, making the intent clear and avoiding unnecessary sdist generation in the wheel-building job.
50-64: Essential verification step prevents broken wheels from being published.The dylib verification immediately after building ensures packaging problems are caught early. Failing the build when the dylib is missing prevents broken wheels from reaching the publish step.
69-70: Robust artifact handling with defense-in-depth verification.The workflow improvements are well-designed:
- Per-version artifact names prevent collisions when building for multiple Python versions
merge-multiple: trueefficiently collects all wheels into a single directory for publishing- The verification loop before publishing provides a final safety check, ensuring no broken wheels slip through
This defense-in-depth approach (verify after build + verify before publish) significantly reduces the risk of publishing wheels without the required dylib.
Also applies to: 74-74, 82-82, 88-98
The dylib wasn't being included in wheels because build_ext runs after build_py, so the dylib wasn't copied to the build directory when package data files were collected. Added BuildPyWithDylib class that builds the Swift dylib before copying package files and explicitly copies it to the build directory. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Set MACOSX_DEPLOYMENT_TARGET=26.0 to properly tag wheels for macOS 26+ - Set ARCHFLAGS="-arch arm64" to build only for Apple Silicon - Add Swift source files to package-data to ensure they're included This silences the "wheel needs a higher macOS version" warning and ensures we only build for arm64 since Apple Intelligence requires Apple Silicon. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
I had to upgrade my Xcode in the App Store app first, but this branch worked when I did this: git clone https://github.com/btucker/apple-foundation-models-py
cd apple-foundation-models-py
uv run --with-editable '.[dev]' --python 3.12 --with build python -m buildThis gave me And that wheel worked! uvx --python 3.12 --with llm-apple --with dist/apple_foundation_models-0.1.4-cp312-cp312-macosx_26_0_arm64.whl llm -m apple hi
|
|
Also built me a working Python 3.14 wheel: uv run --with-editable '.[dev]' --python 3.14 --with build python -m build
uvx --python 3.14 --with llm-apple --with dist/apple_foundation_models-0.1.4-cp314-cp314-macosx_26_0_arm64.whl llm -m apple hi |
|
@simonw nice! Thank you for trying! I think I finally kind of understand how wheels work... new territory for me. The warnings were actually pretty useful in the end. About to publish a new version which hopefully should work out of the box. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
setup.py (1)
36-113: Clean up f-strings without placeholders.The dylib build logic is well-structured with proper error handling and the critical copy to the package directory (line 101). However, several print statements use f-strings without any placeholders.
Apply this diff to remove unnecessary f-prefixes:
if os_version < 26: - print(f"Warning: macOS 26.0+ required for Apple Intelligence") - print(f"Current version: {platform.mac_ver()[0]}") - print("Continuing anyway (library will be built but may not function)") + print("Warning: macOS 26.0+ required for Apple Intelligence") + print(f"Current version: {platform.mac_ver()[0]}") + print("Continuing anyway (library will be built but may not function)") # Create lib directory LIB_DIR.mkdir(parents=True, exist_ok=True) # Compile Swift to dylib cmd = [ "swiftc", str(SWIFT_SRC), "-O", "-whole-module-optimization", - f"-target", f"{ARCH}-apple-macos26.0", + "-target", f"{ARCH}-apple-macos26.0", "-framework", "Foundation", "-framework", "FoundationModels", "-emit-library", "-o", str(DYLIB_PATH), "-emit-module", "-emit-module-path", str(SWIFTMODULE_PATH), "-Xlinker", "-install_name", - "-Xlinker", f"@rpath/libfoundation_models.dylib", + "-Xlinker", "@rpath/libfoundation_models.dylib", ] try: result = subprocess.run(cmd, check=True, capture_output=True, text=True) if result.stdout: print(result.stdout) if result.stderr: print(result.stderr, file=sys.stderr) print(f"✓ Successfully built: {DYLIB_PATH}") print(f" Size: {DYLIB_PATH.stat().st_size / 1024:.1f} KB") # Copy dylib to package directory so it's included in wheels print(f"Copying dylib to package directory: {PKG_DYLIB_PATH}") shutil.copy2(DYLIB_PATH, PKG_DYLIB_PATH) - print(f"✓ Dylib copied to package directory") + print("✓ Dylib copied to package directory") except subprocess.CalledProcessError as e: - print(f"✗ Swift compilation failed") + print("✗ Swift compilation failed") print(e.stderr) sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/publish-to-pypi.yml(1 hunks)pyproject.toml(2 hunks)setup.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
setup.py
66-66: f-string without any placeholders
Remove extraneous f prefix
(F541)
78-78: f-string without any placeholders
Remove extraneous f prefix
(F541)
86-86: f-string without any placeholders
Remove extraneous f prefix
(F541)
90-90: subprocess call: check for execution of untrusted input
(S603)
102-102: f-string without any placeholders
Remove extraneous f prefix
(F541)
105-105: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build wheels on macOS for Python 3.11
- GitHub Check: Build wheels on macOS for Python 3.13
- GitHub Check: Build wheels on macOS for Python 3.12
- GitHub Check: Build wheels on macOS for Python 3.9
- GitHub Check: Build wheels on macOS for Python 3.10
🔇 Additional comments (11)
pyproject.toml (2)
7-7: LGTM! Version bump aligns with the PR objectives.The version increment from 0.1.3 to 0.1.4 is appropriate for this bug fix release that includes the dylib in wheels.
54-54: LGTM! Package data patterns correctly include the dylib.The addition of
"*.dylib"ensureslibfoundation_models.dylibis included in wheels. The Swift source and header patterns provide completeness for development scenarios..github/workflows/publish-to-pypi.yml (5)
53-67: LGTM! Dylib verification prevents broken wheels.The verification step correctly checks for
libfoundation_models.dylibin the wheel and fails fast with clear error messages if missing. This addresses the root cause of issue #2.
69-73: LGTM! Artifact naming prevents collisions.Using
wheel-${{ matrix.python-version }}ensures each Python version's wheel is uploaded with a unique artifact name, preventing overwrites in the matrix build.
77-101: LGTM! Multi-artifact handling and verification are correct.The publish job correctly:
- Depends on the renamed
build-wheelsjob- Uses
merge-multiple: trueto flatten all matrix artifacts intodist/- Verifies every wheel contains the dylib before publishing
This ensures only valid wheels reach PyPI.
15-15: No action required — macos-26 is a valid GitHub-hosted runner.The
macos-26runner label refers to macOS 26 (Tahoe) runner images that were added as a public beta in 2025, so the workflow specification is correct.
46-51: Confirm whether Intel Mac support is intentionally excluded and document the wheel-only publishing strategy.The workflow builds wheels only (
--wheel, no sdist) and restricts to ARM64 architecture, despitesetup.pysupporting botharm64andx86_64(line 30-33). No other workflows build sdist or x86_64 wheels.If Intel Mac support is intentional, document this clearly. If not, consider:
- Building universal2 wheels for broader compatibility, or
- Explicitly adding x86_64 to the matrix, or
- Explaining why arm64-only is acceptable
Consider also whether an sdist should be published for this package (currently none is built).
setup.py (4)
11-11: LGTM! Import and path additions support the dylib copy workflow.The new imports (
shutil,build_py) and path constants (PKG_DIR, updatedSWIFT_DIR,PKG_DYLIB_PATH) correctly support the dylib build and packaging logic.Also applies to: 15-15, 22-23, 27-27
115-132: LGTM! BuildPyWithDylib ensures dylib is included in wheels.The custom
build_pyclass correctly:
- Builds the Swift dylib before packaging
- Executes standard Python build steps
- Copies the dylib to the build directory, ensuring it's included in wheels
This addresses the core issue from #2.
134-140: LGTM! Refactoring to shared build function improves maintainability.The
BuildSwiftThenExtclass now calls the sharedbuild_swift_dylib()function, eliminating code duplication and centralizing the build logic.
177-184: LGTM! Custom build class registration completes the packaging fix.Adding
BuildPyWithDylibto the cmdclass ensures the dylib is built and copied during package builds, completing the fix for issue #2.
|
Whelp, I'm still doing something wrong. When building on github it's building as a universal2. |
The GitHub Actions runner Python is built as universal2, so even with ARCHFLAGS set, wheels were still tagged as universal2. Setting _PYTHON_HOST_PLATFORM overrides the platform tag to force arm64. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
The Need to dig into why. |
PyPI doesn't recognize macOS 26.0 as a valid platform tag yet. Use macosx_11_0_arm64 instead so wheels can be uploaded. The dylib is still built for macOS 26.0, and runtime availability checks ensure it only works on macOS 26.0+. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Make the macOS version requirement prominent in the PyPI package description so users know before installing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
49-54: Consider consolidating package inclusion to package-data only (optional refactor).The config uses both
include-package-data = true(line 51, which enables MANIFEST.in) and explicitpackage-data(line 54). While both can coexist without conflict, a cleaner approach would be to move all file patterns topackage-dataand setinclude-package-data = false, or remove the redundantinclude-package-datadirective if MANIFEST.in is no longer needed.Per the PR, MANIFEST.in was previously used to include the dylib, but now
setup.pyhandles dylib copying andpackage-datadeclares it explicitly. Verify that MANIFEST.in is still necessary for other files or can be deprecated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish-to-pypi.yml(1 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish-to-pypi.yml
🔇 Additional comments (4)
pyproject.toml (4)
27-36: Verify Python 3.8 support is active.The classifiers list Python 3.8 support (line 27) and
requires-python = ">=3.8", but the CI workflow mentioned in PR comments only builds wheels for Python 3.9–3.13. Confirm whether 3.8 is actually tested and maintained, or update the classifiers andrequires-pythonconstraint to match the CI scope.
8-8: Clarify macOS version enforcement in metadata.The description now states "requires macOS 26.0+", but this requirement is not enforced via project metadata (e.g., platform constraints or environment markers). Users on older macOS versions may successfully install the package via pip but encounter runtime failures when loading the dylib. Consider whether the macOS platform tag in the wheel filename (e.g.,
macosx_26_0_arm64) is sufficient, or if additional guardrails (e.g.,python_requiresplatform markers or runtime checks) are needed.Per PR comments, there have been platform tag issues with PyPI rejecting
macosx_26_0_arm64as unsupported; ensure the final wheels have valid platform tags before publishing.
54-54: Verify Swift source files are intended for distribution.The package-data now includes
swift/*.swiftandswift/*.h. Confirm whether these Swift source files are runtime dependencies needed in the wheel, or if they are build-time artifacts that should not be distributed. If they are build-time only, remove them from package-data to reduce wheel size.Additionally, verify that the
*.dylibglob pattern correctly matcheslibfoundation_models.dylibaftersetup.pycopies it into theapplefoundationmodels/directory during the build phase (before setuptools processes package-data).
7-8: Version and description updates look good.The version bump to 0.1.4 and enhanced description clearly communicate the package functionality and platform requirements. No issues here.
- Replace python -m build with uv build for faster, simpler builds - Add smoke test to verify wheel installs and imports correctly - Use astral-sh/setup-uv action with caching enabled 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
It turns out, PyPI doesn't support wheels targeting macOS 26 yet. Opened a PR to add that: pypi/warehouse#19018 |
Set MACOSX_DEPLOYMENT_TARGET=26.0 and force arm64 platform tag to properly build wheels tagged as macosx_26_0_arm64. This ensures the wheel metadata accurately reflects the macOS 26.0+ requirement. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove redundant include-package-data directive and rely solely on explicit package-data configuration. This is cleaner since we're only building wheels (no sdist) and setup.py handles dylib copying. Added *.pyx to package-data for completeness. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add uv-based development workflow to README while keeping pip as an alternative. This reflects the project's adoption of uv for CI builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove Python 3.8 from classifiers and update requires-python to match CI workflow which builds wheels for Python 3.9-3.13. Python 3.8 may not be available on macos-26 runners. Updated both pyproject.toml and README.md for consistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add Python 3.14 classifier and build wheels for it in CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/publish-to-pypi.yml (1)
98-112: Enhance error messaging in multi-wheel verification loop.The verification loop (lines 104–112) silently exits on the first missing dylib without clearly indicating which wheel failed or its size/contents. This makes debugging harder when publishing a batch of wheels.
Consider logging the wheel filename and size before the check, and dumping contents on failure:
echo "Verifying each wheel contains dylib:" for wheel in dist/*.whl; do echo "Checking $wheel:" + ls -lh "$wheel" if unzip -l "$wheel" | grep -q "libfoundation_models.dylib"; then echo " ✓ dylib present" else echo " ✗ ERROR: dylib missing in $wheel!" + echo "Wheel contents:" + unzip -l "$wheel" exit 1 fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-to-pypi.yml(1 hunks)
🔇 Additional comments (5)
.github/workflows/publish-to-pypi.yml (5)
13-21: Matrix strategy is well-configured for multi-version builds.The matrix correctly targets Python 3.9–3.13 with
fail-fast: falseto ensure all versions build even if one fails. This addresses the secondary objective (expand platform/version coverage).
54-62: Wheel installation test provides good functional verification.Testing the wheel in an isolated venv confirms that the dylib is correctly bundled and loadable at runtime. The cleanup is also appropriate.
64-78: Dylib verification in build job is a critical safety check.The
grepcheck forlibfoundation_models.dylibin the wheel and the fallback to show full contents on failure is effective. This directly addresses the primary objective (ensure dylib is in wheels) and catches packaging regressions early.
86-96: Publish job correctly depends on matrix build and merges artifacts.The
needs: [build-wheels]dependency andmerge-multiple: trueensure all wheels are downloaded before verification and publishing. This allows publishing a complete set across Python versions in one step.
46-52: ****The platform tag concern in this review is based on incorrect information. PyPI Warehouse accepts macOS platform tags in the format
macosx_{major}_0_arm64per PEP 425, and there is no evidence that wheels taggedmacosx_26_0_arm64are rejected. The workflow correctly sets_PYTHON_HOST_PLATFORM: "macosx-26.0-arm64"(which normalizes to the standard format) and runs on the matchingmacos-26runner. Additionally, the workflow includes proper verification safeguards: dylib presence checks, installation tests, and artifact validation. Suggesting a downgrade to older macOS versions would be a regression and would misrepresent the actual target platform.Likely an incorrect or invalid review comment.
Swift source files (.swift, .h) are only needed at build time, not runtime. Remove them from package-data and MANIFEST.in to reduce wheel size from 181KB to 177KB. Runtime dependencies (dylib and compiled .so) are still correctly included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Improve debugging for multi-wheel builds by: - Show wheel filename and size before checking - Include full wheel contents in error output when dylib is missing - Clarify which specific wheel failed verification 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Reduce from 185 to 110 lines (40% reduction) - Remove redundant error checks and variables - Consolidate Swift build command formatting - Only support arm64 architecture (Apple Silicon) - Simplify class definitions and remove verbose comments - Use sys.exit() for cleaner error handling
Run full test suite against installed wheel to verify: - Module imports correctly - Dylib is present and loadable - All tests pass from installed package - Tests run from /tmp to ensure using wheel, not source
- Test workflow now builds wheels and tests from installed packages - Ensures wheels work correctly before publishing - Test all Python versions (3.9-3.14) from wheels - Publish workflow focuses on building and publishing - Tests run from /tmp to guarantee using installed wheel, not source
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/publish-to-pypi.yml(1 hunks).github/workflows/test.yml(3 hunks)MANIFEST.in(1 hunks)README.md(3 hunks)pyproject.toml(3 hunks)setup.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 Ruff (0.14.3)
setup.py
55-55: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (16)
.github/workflows/test.yml (4)
17-17: Verify Python 3.14 inclusion against PR scope.The matrix includes Python 3.14, but the PR description mentions support for Python 3.9–3.13. Confirm whether 3.14 support is intentional or if the matrix should be capped at 3.13.
41-49: Wheel-based testing approach is well-suited for packaging validation.The workflow now builds a wheel, installs it into an isolated venv, and runs tests against the installed package. This is the correct pattern for validating that
libfoundation_models.dylib(and other package data) is properly included in the wheel. The approach catches packaging issues that would only surface after installation from a wheel, not from source tree testing.
66-68: Verify that tests are location-independent when run from/tmp.Tests are executed from
/tmpwith an activated venv, which ensures they test the installed wheel rather than the source tree. However, this requires that all test fixtures, data files, and relative imports are either:
- Bundled with the wheel as package data, or
- Generated/created at runtime, or
- Resolved via absolute paths or module-relative imports.
Confirm that your test suite does not rely on relative paths to find test artifacts (e.g.,
../fixtures/data.json).
51-62: Ensure Apple Intelligence check error handling remains appropriate.The step now runs within the activated venv (line 53) and includes
continue-on-error: true, which is sensible for a feature-availability check. Verify that transient import or availability-check failures do not mask real packaging issues (e.g., missing dylib causing ImportError)..github/workflows/publish-to-pypi.yml (2)
53-67: LGTM! Strong verification before upload.The per-wheel dylib verification ensures the core issue (dylib missing from wheels) is caught before artifacts are uploaded. The failure handling with full contents dump is helpful for debugging.
87-104: LGTM! Defense-in-depth verification before PyPI publish.The second verification layer in the publish job ensures no wheels slip through without the dylib, even if the build-wheels verification was somehow bypassed. This defense-in-depth approach is appropriate for critical packaging infrastructure.
MANIFEST.in (1)
1-16: LGTM! Clear sdist/wheel packaging separation.The restructured MANIFEST.in correctly focuses on sdist packaging while relying on
pyproject.toml[tool.setuptools.package-data]for wheel contents. The global-exclude patterns prevent source artifacts (Swift files, headers, static libraries) from leaking into wheels, which is the correct approach.This complements the
pyproject.tomlchanges that explicitly include*.dylibin package-data for wheels.pyproject.toml (3)
7-8: LGTM! Version bump and description update.The version bump to 0.1.4 and the macOS 26.0+ requirement in the description accurately reflect the changes in this PR.
37-37: LGTM! Dropping EOL Python 3.8.Raising the minimum to Python 3.9 is appropriate since Python 3.8 reached end-of-life in October 2024.
54-59: Including*.pyxin wheels is unconventional.The
*.dylibaddition is essential and correct (core fix for issue #2). However, including*.pyx(Cython source files) in binary wheels is unconventional. Wheels typically contain only compiled extensions (.so/.pyd) and runtime assets.Are you intentionally distributing Cython sources for debugging or rebuilding? If not, consider removing
*.pyxfrom package-data.If Cython sources are intentional, document the rationale; otherwise apply this diff:
applefoundationmodels = [ "py.typed", "*.pxd", - "*.pyx", "*.dylib", ]Note:
*.pxd(Cython definition files) are appropriate for type information and are typically kept for library consumers.setup.py (6)
13-21: LGTM! Clear path organization and Apple Silicon focus.The path constants are well-organized, and the explicit arm64-only support aligns with the package's macOS 26.0+ requirement (Apple Silicon Macs only).
24-63: LGTM! Robust dylib build with proper error handling.The
build_swift_dylib()function is well-structured:
- Up-to-date checks prevent unnecessary rebuilds
- Comprehensive validation and error messages
- Copies dylib to
PKG_DYLIB(key for wheel inclusion via package-data)- Proper
@rpathinstall name for relocatable dylibNote: The static analysis warning (S603) on line 55 is a false positive. The command is constructed from trusted sources (
SWIFT_SRCis derived from__file__, not user input).
65-73: LGTM! Ensures dylib is copied to build directory.
BuildPyWithDylibcorrectly integrates the dylib into the package build:
- Builds dylib via
build_swift_dylib()- Runs standard package copy via
super().run()- Explicitly copies
PKG_DYLIBtobuild_lib/applefoundationmodels/This ensures the dylib is present in the package build directory, making it available for wheel packaging via the
package-dataconfiguration.
76-80: LGTM! Build order ensures dylib exists before extension.
BuildSwiftThenExtcorrectly ensures the Swift dylib is built before the Cython extension that links against it. This prevents link-time errors.
83-104: LGTM! Extension configuration with proper rpath setup.The Cython extension configuration is correct:
include_dirspoints to Swift headers inapplefoundationmodels/swift/library_dirspoints tolib/for build-time linkingextra_link_argssets multiple rpath entries to locate the dylib at runtime:
{LIB_DIR}for development builds@loader_pathfor wheels (dylib and .so are in the same package directory)This ensures the extension can find
libfoundation_models.dylibin both development and installed wheel scenarios.
109-109: LGTM! Custom build classes properly registered.The
cmdclassregistration wires both custom build commands into the setuptools workflow, ensuring:
BuildPyWithDylibcopies the dylib into the package for wheel inclusionBuildSwiftThenExtbuilds the dylib before the Cython extension
The dylib was only included in source distributions via MANIFEST.in but not in wheels. Wheels require files to be part of package data. Now copies the dylib to the package directory during build and includes it via package_data in pyproject.toml.
Also improved CI workflow to build wheels for Python 3.9-3.13 with explicit dylib verification before publishing.
fixes #2
Summary by CodeRabbit
New Features
Chores
Documentation
Tests