Skip to content

Use CMake EXPORT #8954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 122 commits into
base: main
Choose a base branch
from
Open

Use CMake EXPORT #8954

wants to merge 122 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Mar 5, 2025

The CMake EXPORT feature makes target_compile_options/target_link_libraries etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. We are currently working around our lack of EXPORT by (incompletely and therefore incorrectly!) repeating our configuration in tools/cmake/executorch-config.cmake.

I was previously stalled here by XNNPACK's lack of use of EXPORT, but I discovered that I simply wasn't being thorough enough with installing/exporting the entire target graph.

Note that this diff raises the size test thresholds. This is because previously the size test wasn't including prim op registration, and this diff effectively fixes builds using find_package to always include that.

[ghstack-poisoned]
@swolchok
Copy link
Contributor Author

swolchok commented Mar 5, 2025

Copy link

pytorch-bot bot commented Mar 5, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit db7c3cd with merge base 8d57c95 (image):
💚 Looks good so far! There are no failures yet. 💚

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

swolchok added a commit that referenced this pull request Mar 5, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: e3e5b36
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@facebook-github-bot facebook-github-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 Mar 5, 2025
@swolchok swolchok marked this pull request as draft March 5, 2025 01:40
swolchok added 4 commits July 13, 2025 09:47
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@swolchok
Copy link
Contributor Author

have an update cooking, need to make sure check whether warnings I'm seeing about macOS version mismatch during install_executorch.sh are new and if so problematic

[ghstack-poisoned]
swolchok added 4 commits July 23, 2025 15:16
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 23, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: f1615a2
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
swolchok added 3 commits July 23, 2025 16:45
…t failure for the EXPORT PR

[ghstack-poisoned]
…t failure for the EXPORT PR

[ghstack-poisoned]
…t failure for the EXPORT PR

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jul 23, 2025
The CMake EXPORT feature seems like it's supposed to make target_compile_options etc. work nicely for both in-project builds and also projects that consume your project by installing it and finding it with find_package. Using EXPORT would make it easy to have similar behavior to Buck's exported_preprocessor_flags, for example.

Unfortunately, I am currently stalled here by XNNPACK's lack of use of EXPORT. I tried working around dependencies that don't use EXPORT by re-installing them with EXPORT ourselves, but XNNPACK has rather a lot so I'm sending this out to highlight the problem and ask if there's another way.

I found some similar issues in TFLite: tensorflow/tensorflow#57658 tensorflow/tensorflow#87172

ghstack-source-id: efb6f61
ghstack-comment-id: 2699501745
Pull Request resolved: #8954
@swolchok swolchok requested review from jathu and removed request for shoumikhin, dbort, iseeyuan, jackzhxng and mcr229 July 24, 2025 00:05
@swolchok swolchok marked this pull request as ready for review July 24, 2025 16:16
@swolchok
Copy link
Contributor Author

This is getting very close to green CI if it's not there already, so marking ready for review.

Base automatically changed from gh/swolchok/528/head to main July 24, 2025 20:41
@swolchok
Copy link
Contributor Author

looks like ARM size tests need their bump increased and otherwise this looks very promising

@swolchok
Copy link
Contributor Author

finally 100% green CI!

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: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants