-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Don't install headers headers which are not supposed to be published #16070
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
cmb69
wants to merge
7
commits into
php:master
Choose a base branch
from
cmb69:cmb/published-headers
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ac1ebfc
Publish header files explicitly
cmb69 6d0047c
Don't install arginfo.h files
cmb69 dfacc1e
Don't install Windows specific headers on non Windows systems
cmb69 3da50e7
Don't install internal headers
cmb69 a818d00
Update configure.ac
cmb69 a04adef
Update configure.ac
cmb69 df58859
Revert "Don't install Windows specific headers on non Windows systems"
cmb69 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have these lists generated by a small php script which can be added into https://github.com/php/php-src/blob/master/.github/actions/verify-generated-files/action.yml to assert all lists are always in sync and up-to-date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #15682 (comment); Windows only; I wouldn't know how to do this for other systems (i.e. via autoconf).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this PR proposes to hardcode the files, we do not need to generate them on each local build, but we need to assert that the hardcoded lists are up-to-date - for that purpose, there is https://github.com/php/php-src/blob/master/.github/actions/verify-generated-files/action.yml CI and we need a simple PHP script which will discover all *.h files.
In that script, published files can be hardcoded and the rest assumed as non-published. The script can then locate the lists in all files where the hardcoded lists are to be present and replace the lists will up-to-date lists.
The CI job would then check if the files in git are matching the generated outputs the same as we do for stubs and other generated files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR does not propose the list; that already happened via #15682. I agree that auto-generating and checking the list would be good, but I still have not been able to figure out how to do that on non Windows systems; and doing it only on Windows might miss headers from Windows incompatible stuff.