Corrections to CMake changes to enable library sub-builds #571
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
Two scripts arguments were renamed during review, but the cmake steps calling the script weren't updated. Additionally the our downstream test targets such as check-package-llvm-toolchain target require the path to an external llvm-lit in order to run the tests.
The variables to enable exceptions and RTTI are now cached and the names are capitalized, but CMake arguments needed updating to reflect this.
dcandler
changed the title
|
Found one more issue: the parameter to enable exceptions and RTTI was not getting passed down correctly so the exceptions sample failed. |
The PREBUILT_TARGET_LIBRARIES option is meant to disable the building of all libraries, on the assumption that pre-built libraries will be copied from another location. The functionality was accidentally removed, but has now been returned.
CMake automatically expands a string containing semi-colons to a list, but ExternalProject argument must also be a semi-colon separated list. To get around this, convert the string to a comma separated list, since the LIST_SEPARATOR option will reinterpret this as a semi-colon list.
Previously, FVP testing was skipped if a FVP install was not available. However, this now throws a configuration error if testing relies on FVPs. But if you specifically want to test with FVPs, it is desirable to have the configuration error warn you with an if the install cannot be found. To resolve this, an additional option has been added to explicitly enable FVP testing, defaulting to OFF. This will override any settings from the variant JSON, so that the default case remains skipping FVP testing.
vhscampos
approved these changes
Nov 20, 2024
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.
Two scripts arguments were renamed during review, but the cmake steps calling the script weren't updated.
Additionally the our downstream test targets such as check-package-llvm-toolchain target require the path to an external llvm-lit in order to run the tests.