Add check_for_installed_private_headers_in_cmake_out #13485
Merged
Conversation
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
swolchok
added a commit
that referenced
this pull request
Aug 19, 2025
This adds a test that will run in CI to make sure that we don't install private headers. I fixed the existing places where we were installing private headers; reviewers, please confirm that we are OK with this technical break of source-level backward compatibility. ghstack-source-id: ca0dbcd ghstack-comment-id: 3198151825 Pull-Request: #13485
swolchok
added a commit
that referenced
this pull request
Aug 19, 2025
This adds a test that will run in CI to make sure that we don't install private headers. I fixed the existing places where we were installing private headers; reviewers, please confirm that we are OK with this technical break of source-level backward compatibility. ghstack-source-id: 733e1e5 ghstack-comment-id: 3198151825 Pull-Request: #13485
|
noting CI looks good |
swolchok
added a commit
that referenced
this pull request
Aug 20, 2025
…ripts/build_apple_frameworks.sh This removes the need to use Buck in this script. The previous PR (#13485) adds enforcement that we don't install non-public headers from CMake. I built the frameworks before and after. Here is a diff of all headers present in the frameworks: https://gist.github.com/swolchok/70647b551da7827a9cbdb083fad1e209 Generally, we now include more headers than before. The only header we used to include that we no longer include is schema/extended_header.h. It looks like we are not including the rest of the headers for the PTE schema anyway, so I propose that we fix installation of the schema headers separately. ghstack-source-id: 626397b ghstack-comment-id: 3207551649 Pull-Request: #13559
| ) | ||
| install( | ||
| DIRECTORY runtime/executor/ | ||
| DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/executorch/runtime/executor | ||
| FILES_MATCHING | ||
| PATTERN "*.h" | ||
| PATTERN "test" EXCLUDE | ||
| PATTERN "platform_memory_allocator.h" EXCLUDE |
There was a problem hiding this comment.
I need to follow up with Greg on what this even is used for.
JacobSzwejbka
approved these changes
Aug 24, 2025
agrima1304
pushed a commit
to agrima1304/executorch
that referenced
this pull request
Aug 26, 2025
This adds a test that will run in CI as part of unittest jobs to make sure that we don't install private headers. I fixed the existing places where we were installing private headers; reviewers, please confirm that we are OK with this technical break of source-level backward compatibility.
jackzhxng
added a commit
that referenced
this pull request
Aug 26, 2025
This reverts commit 90c16ef.
jackzhxng
added a commit
that referenced
this pull request
Aug 26, 2025
GregoryComer
added a commit
that referenced
this pull request
Aug 26, 2025
The Windows CI build job was disabled in #13669 due to a new Windows build failure in #13485. Since that diff was reverted in #13685, we can re-enable the job. I started fixing the issue in #13672, but dropped it due to the revert. It's a simple change, so it can likely be bundled when the affected PR is re-landed. Test Plan: The Build Preset / Windows job is passing on this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds a test that will run in CI as part of unittest jobs to make sure that we don't install private headers.
I fixed the existing places where we were installing private headers; reviewers, please confirm that we are OK with this technical break of source-level backward compatibility.