From e9839fe82f152adce8f8a5026df607deae0c2a67 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Wed, 14 Aug 2024 10:53:55 +0000 Subject: [PATCH 1/4] Add --build-tool-options to pass options to make/ninja/etc. https://github.com/llvm/llvm-lnt/commit/768874171529d31d12adb032df57b1c5f1c1fa77 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. --- lnt/tests/test_suite.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lnt/tests/test_suite.py b/lnt/tests/test_suite.py index d4e5290a..51e65257 100644 --- a/lnt/tests/test_suite.py +++ b/lnt/tests/test_suite.py @@ -580,7 +580,8 @@ def _build(self, path): '--build', '.', '-t', target, '-j', str(self._build_threads())] + - ([] if self.opts.succinct else ["-v"]), + ([] if self.opts.succinct else ["-v"]) + + ["--"] + shlex.split(self.opts.build_tool_options), cwd=subdir) except subprocess.CalledProcessError: # cmake is expected to exit with code 1 if there was any build @@ -1168,6 +1169,9 @@ def diagnose(self): @click.option("--use-make", "make", metavar="PATH", type=click.UNPROCESSED, help="Path to the build system tool [make/ninja/...]") +@click.option("--build-tool-options", + help="Options to pass to the build tool (ninja/make/etc.).", + type=click.UNPROCESSED) @click.option("--use-lit", "lit", metavar="PATH", type=click.UNPROCESSED, default="llvm-lit", help="Path to the LIT test runner [llvm-lit]") From 363a5c5ec88ccc62698767d18b1c9d32231c0f50 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Aug 2024 09:04:31 +0000 Subject: [PATCH 2/4] match wording --- lnt/tests/test_suite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnt/tests/test_suite.py b/lnt/tests/test_suite.py index 51e65257..eb7eeecd 100644 --- a/lnt/tests/test_suite.py +++ b/lnt/tests/test_suite.py @@ -1170,7 +1170,7 @@ def diagnose(self): type=click.UNPROCESSED, help="Path to the build system tool [make/ninja/...]") @click.option("--build-tool-options", - help="Options to pass to the build tool (ninja/make/etc.).", + help="Options to pass to the build system tool", type=click.UNPROCESSED) @click.option("--use-lit", "lit", metavar="PATH", type=click.UNPROCESSED, default="llvm-lit", From 7d2a57173533c9c18fb41d1e633d15c047a4032a Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Aug 2024 09:13:44 +0000 Subject: [PATCH 3/4] Add note to quickstart --- docs/quickstart.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/quickstart.rst b/docs/quickstart.rst index e2488f2a..de766415 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -72,6 +72,13 @@ command. The information below should be enough to get you started, but see the The ``SANDBOX`` value is a path to where the test suite build products and results will be stored (inside a timestamped directory, by default). + We recommend adding ``--build-tool-options="-k"`` (if you are using ``make``) + or ``--build-tool-options="-k 0"`` (if you are using ``ninja``). This ensures + that the build tool carries on building even if there is a compilation + failure in one of the tests. Without these options, every test after the + compilation failure will not be compiled and will be reported as a missing + executable. + #. On most systems, the execution time results will be a bit noisy. There are a range of things you can do to reduce noisiness (with LNT runtest test-suite command line options when available between brackets): From 63627ae155d805c0c8c9a79b55af4e3bf1b547d2 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Aug 2024 13:22:26 +0000 Subject: [PATCH 4/4] correct syntax in docs --- docs/quickstart.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/quickstart.rst b/docs/quickstart.rst index de766415..aae80707 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -72,8 +72,8 @@ command. The information below should be enough to get you started, but see the The ``SANDBOX`` value is a path to where the test suite build products and results will be stored (inside a timestamped directory, by default). - We recommend adding ``--build-tool-options="-k"`` (if you are using ``make``) - or ``--build-tool-options="-k 0"`` (if you are using ``ninja``). This ensures + We recommend adding ``--build-tool-options "-k"`` (if you are using ``make``) + or ``--build-tool-options "-k 0"`` (if you are using ``ninja``). This ensures that the build tool carries on building even if there is a compilation failure in one of the tests. Without these options, every test after the compilation failure will not be compiled and will be reported as a missing