Add --build-tool-options to pass options to make/ninja/etc. #25
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
llvm@7688741 stopped us always using make but in the process removed the "-k" flag. This flag causes make to carry on even when some targets fail. This is very important for running the test suite in CI. Without it, a single build failure marks thousands of subsequent programs as missing, as if they too had failed to compile. If I deliberately break a single source test the results currently are: ``` Total Discovered Tests: 3424 Passed : 378 (11.04%) Executable Missing: 3046 (88.96%) Import succeeded. ``` This is what they should be: ``` Executable Missing Tests (1): test-suite :: SingleSource/UnitTests/2003-04-22-Switch.test Total Discovered Tests: 3424 Passed : 3423 (99.97%) Executable Missing: 1 (0.03%) Import succeeded. ``` cmake --build does not have its own -k option, so we have to pass it after -- to the native tool. Unfortunately ninja's -k takes a number, so ninja -k 0 is equivalent to make -k. Therefore we can't just always add "-- -k" here. Instead I've added a new option --build-tool-options where you can pass any arguments you want. The build bots will use this to pass "-k" to make, as we know for sure they use make. Anything using ninja will also want to pass "-k 0" to get the same effect.
|
First spotted by @antmox a few weeks ago. And while I have your attention @lukel97 , thanks for getting LNT working on Python 3.12. This has unblocked some work Linaro wanted to do. Also we'd been thinking about switching to ninja for test suites but never got around to it, so we're on board with the changes overall. Just need to fix this for build bot purposes. |
lukel97
approved these changes
Aug 14, 2024
|
We could call it out in the documentation, is there a good place for that? I'm not familiar with LNT's docs. |
|
I think https://llvm.org/docs/lnt/quickstart.html#running-tests, I'll update the PR tomorrow. |
DavidSpickett
added a commit
to DavidSpickett/llvm-zorg
that referenced
this pull request
Aug 15, 2024
When llvm/llvm-lnt@7688741 enabled using Ninja, it removed the -k flag from make. -k causes make to carry on if there's a build failure. Without this flag, a single compilation failure turns into thousands of missing executables in the final results. As subsequent targets are not built. llvm/llvm-lnt#25 is adding an option to the lnt runner to pass options to the build tool. This PR uses that to pass -k to make to restore the old behaviour on the build bots.
|
This looks good. I have also updated llvm/llvm-zorg#245 |
omjavaid
approved these changes
Aug 26, 2024
DavidSpickett
added a commit
to llvm/llvm-zorg
that referenced
this pull request
Aug 29, 2024
When llvm/llvm-lnt@7688741 enabled using Ninja, it removed the -k flag from make. -k causes make to carry on if there's a build failure. Without this flag, a single compilation failure turns into thousands of missing executables in the final results. As subsequent targets are not built. llvm/llvm-lnt#25 is adding an option to the lnt runner to pass options to the build tool. This PR uses that to pass -k to make to restore the old behaviour on the build bots.
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.
7688741 stopped us always using make but in the process removed the "-k" flag.
This flag causes make to carry on even when some targets fail. This is very important for running the test suite in CI. Without it, a single build failure marks thousands of subsequent programs as missing, as if they too had failed to compile.
If I deliberately break a single source test the results currently are:
This is what they should be:
cmake --build does not have its own -k option, so we have to pass it after -- to the native tool. Unfortunately ninja's -k takes a number, so ninja -k 0 is equivalent to make -k. Therefore we can't just always add "-- -k" here.
Instead I've added a new option --build-tool-options where you can pass any arguments you want. The build bots will use this to pass "-k" to make, as we know for sure they use make.
Anything using ninja will also want to pass "-k 0" to get the same effect.