[DTLTO][TEST] Get the DTLTO Clang driver tests failing on some buildbots to pass #159158
Conversation
…bots Not all builds name the compiler executable clang. For example, the Fuchsia buildbots use llvm as their single toolchain executable name, as they combine everything together in a busybox-style binary. This is currently causing the new ps5-dtlto.c to fail on such build bots. Update the Clang driver tests to simply check that a non-empty path is provided for the --thinlto-remote-compiler argument, rather than hardcoding the executable name. The cross-project tests will verify that the path is valid later. This is the same fix as applied earlier in llvm#148908. However, that fix left a case in the dtlto.c test that was subsequently reflected into the new ps5-dtlto.c test where it caused a failure. Why it doesn't cause a failure in the existing dtlto.c test is a mystery to me - perhaps the substring "clang" is now included in the path to the busybox-style binary, or perhaps that test was disabled for affected buildbots and then not re-enabled? It's clearly a latent issue though so I have also fixed the dtlto.c test in this patch. Should fix the buildbot failures caused by: llvm#158041.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: bd1976bris (bd1976bris) ChangesMake the test regexes more permissive to fix buildbot failures caused by the merge of PR #159129. This is the same fix as applied in: #148908. We retain cross-platform-test coverage to check that the path content is correct. In a subsequent commit I will make the tests more robust, perhaps along the lines of: PR #159151. This is a short term fix to get the build bot green without having to revert a trail of commits. Full diff: https://github.com/llvm/llvm-project/pull/159158.diff 2 Files Affected:
diff --git a/clang/test/Driver/DTLTO/dtlto.c b/clang/test/Driver/DTLTO/dtlto.c
index 96795d9a4e6a4..bb1bd4ace0487 100644
--- a/clang/test/Driver/DTLTO/dtlto.c
+++ b/clang/test/Driver/DTLTO/dtlto.c
@@ -35,7 +35,7 @@
// DEFAULT: ld.lld
// DEFAULT-SAME: "--thinlto-distributor=d.exe"
-// DEFAULT-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// DEFAULT-SAME: "--thinlto-remote-compiler={{[^"]+}}"
/// Check that nothing is forwarded when the compiler is not in LTO mode, and that
/// appropriate unused option warnings are issued.
diff --git a/clang/test/Driver/DTLTO/ps5-dtlto.c b/clang/test/Driver/DTLTO/ps5-dtlto.c
index 9b70c88257a85..a37073b85aa32 100644
--- a/clang/test/Driver/DTLTO/ps5-dtlto.c
+++ b/clang/test/Driver/DTLTO/ps5-dtlto.c
@@ -35,7 +35,7 @@
// DEFAULT: prospero-lld
// DEFAULT-SAME: "--thinlto-distributor=d.exe"
-// DEFAULT-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// DEFAULT-SAME: "--thinlto-remote-compiler={{[^"]+}}"
/// Check that the arguments are forwarded unconditionally even when the
/// compiler is not in LTO mode.
|
|
Hi @petrhosek. This is the same fix as you approved in #159050. It's not ideal but there's not loss of coverage given the DTLTO testing in the |
|
This one did not fix the buildbot: https://lab.llvm.org/buildbot/#/builders/10/builds/13615 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/22713 Here is the relevant piece of the build log for the reference |
We see the same issue on our Windows builders. |
|
Hi, can we revert your original PR if possible while you are investigating the issue? It blocks bots. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/8933 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/16191 Here is the relevant piece of the build log for the reference |
|
@jplehr , @petrhosek and @Kewen12 really sorry that I didn't check back on this last night. I'm unsure why this didn't fix the bots as it's basically #148908 which has been running for some weeks on the bots with no reports of problems. Anyway, I'll get this resolved (somehow!) first thing today. |
If there is something that I can do to assist, please let me know. |
|
On our bot, I cleaned both source and build tree and gave that a go. |
|
@bd1976bris it came into this build https://lab.llvm.org/buildbot/#/builders/10/builds/13650 which did not break. :) |
6 participants
Make the test regexes more permissive to fix buildbot failures caused by the merge of PR #159129. This is the same fix as applied in: #148908. We retain cross-platform-test coverage to check that the path content is correct.
In a subsequent commit I will make the tests more robust, perhaps along the lines of: PR #159151.
This is a short term fix to get the build bot green without having to revert a trail of commits.