[release/5.0] Fix bad configure tests #43031
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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was an extra -c in the CMAKE_REQUIRED_FLAGS set for testing HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC that was breaking build of coreclr under homebrew. The option was somehow making these checks behave on ARM Linux, eveb though it is not clear to me why, as it was just causing this option to be passed to the compiler twice at different positions of the command line of the cmake tests. This change fixes it by using check_symbol_exists instead of check_c_source_compiles, since just removing the duplicite -c was resulting in the check failing on ARM / ARM64 Linux due to a missing symbol from libunwind during linking.
jeffschwMSFT
approved these changes
Oct 5, 2020
jeffschwMSFT
removed
the
Servicing-consider
jkotas
approved these changes
Oct 5, 2020
|
The libraries test failure is unrelated, I have created an issue #43166 to track it. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backport of #42756 to release/5.0
/cc @dagood @janvorli
There was an extra -c in the CMAKE_REQUIRED_FLAGS set for testing
HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC that was breaking
build of coreclr under homebrew. The option was somehow making
these checks behave on ARM Linux, even though it is not clear to
me why, as it was just causing this option to be passed to the
compiler twice at different positions of the command line of
the cmake tests.
This change fixes it by using check_symbol_exists instead of
check_c_source_compiles, since just removing the duplicite -c
was resulting in the check failing on ARM / ARM64 Linux due
to a missing symbol from libunwind during linking.
Customer Impact
We've got this issue reported by a customer who was trying to add brew recipe for packaging
.NET Core 3.1 on OSX.
Testing
I've manually verified that the HAVE_UNW_GET_ACCESSORS and HAVE_UNW_GET_SAVE_LOC (and UNWIND_CONTEXT_IS_UCONTEXT_T that was not broken, but it is in the block of code influenced
by the change) are generated correctly for all the support target platforms / architectures.
Risk
Very low, the change influences only a compile time detection of a presence of two libunwind
functions.