Skip to content

Correct comments on the Core ML wheel library - #22242

Open
shoumikhin wants to merge 1 commit into
mainfrom
coreml-wheel-comment-fixes
Open

Correct comments on the Core ML wheel library#22242
shoumikhin wants to merge 1 commit into
mainfrom
coreml-wheel-comment-fixes

Conversation

@shoumikhin

Copy link
Copy Markdown
Contributor

Summary

Fix three comments on the standalone Core ML delegate library in the macOS wheel. Comment changes only, no behavior change.

  • The shipped dylib is retained because it is named directly on the link line, not by anything specific to the Mach-O format, and only -dead_strip_dylibs would drop it (nothing passes that).
  • Linking the helper archives privately also keeps their include directories out of the library's export interface, which is intended, since a wheel consumer links the bundled delegate and never names the helpers.
  • Shipping the delegate as a shared library moves a duplicate-symbol conflict from static-link time to load time; the single shipped library and the dynamic link keep exactly one copy in the process.

Test plan

Comment-only. Confirmed both CMake files still pass cmake-format.

Fix three comments on the standalone Core ML delegate library. Comment changes
only, no behavior change.

Explain that the shipped dylib is retained because it is named directly on the
link line, not by anything specific to the Mach-O format, and note that only
-dead_strip_dylibs would drop it, which nothing passes. Note that linking the
helper archives privately also keeps their include directories out of the
library's export interface, which is intended, since a wheel consumer links the
bundled delegate and never names the helpers. Note that shipping the delegate as
a shared library moves a duplicate-symbol conflict from static-link time to load
time, and that the single shipped library and the dynamic link keep exactly one
copy in the process.

Test Plan:
Comment-only. Confirmed both CMake files still format-parse.
Copilot AI lite review requested due to automatic review settings August 27, 2026 23:04
@shoumikhin shoumikhin added the release notes: none Do not include this in the release notes label Aug 27, 2026
@pytorch-bot

pytorch-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22242

Note: Links to docs will display an error until the docs builds have been completed.

❌ You can merge normally! (1 Unrelated Failure), 1 Unclassified Failure

As of commit 24df64f with merge base d0c2da3 (image):

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants