Update/fix CheckSizeType cmake check #18309
Merged
Conversation
This file contains 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
|
@thewtex, I think you originally contributed the forked version of CheckTypeSize... a very long time ago now. The main concern I have is not knowing which version of cmake we should fork CheckTypeSize from. I chose v3.10.2 mostly arbitrarily and seems to work nicely.. at least of me locally. |
|
I think #18084 should be excluded from the Fixes list, as we don't want it to auto-close and the issue has more to do with Node.js upgrade. |
|
Although it looks like Github only marks the first issue in the list as auto-close anyway... |
|
Done. |
|
Good to land? |
RReverser
reviewed
Dec 5, 2022
sbc100
commented
Dec 5, 2022
Update the CheckTypeSize files to v3.10.2 (mostly arbitrary version, but this is what we use in CI). Local patch to CheckTypeSize is not much smaller. Instead of executing the program and using the return value (doesn't work for return values greater than 255), we simply inject `-oformat=wasm` onto the link command, and then we can use `strings` on the binary just like the upstream check. This is basically a single line patch against upstream now. Fixes: #18278 #18238 #17268 #18084 #17811
RReverser
approved these changes
Dec 5, 2022
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.
Update the CheckTypeSize files to v3.10.2 (mostly arbitrary version).
Local patch to CheckTypeSize is not much smaller. Instead of executing the program and using the return value (doesn't work for return values greater than 255), we simply inject
-oformat=wasmonto the link command, and then we can usestringson the binary just like the upstream check. This is basically a single line patch against upstream now.Fixes: #18278 #18238 #17268 #17811