-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[DTLTO][TEST] Fix Clang driver test failing on some build bots #148908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Not all builds name the compiler executable `clang`. For example, the Fuchsia build bots use `llvm` as the executable name, as they combine everything together in a busybot-style binary. Update the Clang driver test 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 test will verify that the path is valid later. Should fix llvm#147265.
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: bd1976bris (bd1976bris) ChangesNot all builds name the compiler executable Update the Clang driver test to simply check that a non-empty path is provided for the Should fix #147265. Full diff: https://github.com/llvm/llvm-project/pull/148908.diff 1 Files Affected:
diff --git a/clang/test/Driver/DTLTO/dtlto.c b/clang/test/Driver/DTLTO/dtlto.c
index d72e487e706b2..96795d9a4e6a4 100644
--- a/clang/test/Driver/DTLTO/dtlto.c
+++ b/clang/test/Driver/DTLTO/dtlto.c
@@ -10,7 +10,7 @@
// FORWARD: ld.lld
// FORWARD-SAME: "--thinlto-distributor=d.exe"
-// FORWARD-SAME: "--thinlto-remote-compiler={{.*}}clang{{[^\"]*}}"
+// FORWARD-SAME: "--thinlto-remote-compiler={{[^"]+}}"
// FORWARD-SAME: "--thinlto-distributor-arg=a1"
// FORWARD-SAME: "--thinlto-distributor-arg=a2"
// FORWARD-SAME: "--thinlto-distributor-arg=a3"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
…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.
Previously I masked issues with Multicall + DTLTO (see llvm#148908) due to an incomplete understanding of how the Multicall toolchain works. This patch reverts those incorrect changes and instead marks the affected tests XFAIL when running under Multicall. Issue llvm#159125 tracks fixing DTLTO with Multicall.
) Previously I masked issues with Multicall + DTLTO (see #148908) due to an incomplete understanding of how the Multicall toolchain works. This patch reverts those incorrect changes and instead marks the affected tests XFAIL when running under Multicall. Issue #159125 tracks fixing DTLTO with Multicall.
) Make the test regexes more permissive to fix buildbot failures caused by the merge of PR #159129. This mirrors the earlier fix in PR #148908. We retain cross-project-test coverage to verify that the path content is correct. A follow-up will update the tests to robustly check the Clang executable filename, likely along the lines of PR #159151. Short-term unbreak to keep the buildbots green without reverting a chain of commits.
…#159129) Previously I masked issues with Multicall + DTLTO (see llvm#148908) due to an incomplete understanding of how the Multicall toolchain works. This patch reverts those incorrect changes and instead marks the affected tests XFAIL when running under Multicall. Issue llvm#159125 tracks fixing DTLTO with Multicall.
…#159158) Make the test regexes more permissive to fix buildbot failures caused by the merge of PR llvm#159129. This mirrors the earlier fix in PR llvm#148908. We retain cross-project-test coverage to verify that the path content is correct. A follow-up will update the tests to robustly check the Clang executable filename, likely along the lines of PR llvm#159151. Short-term unbreak to keep the buildbots green without reverting a chain of commits.
…#159129) Previously I masked issues with Multicall + DTLTO (see llvm#148908) due to an incomplete understanding of how the Multicall toolchain works. This patch reverts those incorrect changes and instead marks the affected tests XFAIL when running under Multicall. Issue llvm#159125 tracks fixing DTLTO with Multicall.
…#159158) Make the test regexes more permissive to fix buildbot failures caused by the merge of PR llvm#159129. This mirrors the earlier fix in PR llvm#148908. We retain cross-project-test coverage to verify that the path content is correct. A follow-up will update the tests to robustly check the Clang executable filename, likely along the lines of PR llvm#159151. Short-term unbreak to keep the buildbots green without reverting a chain of commits.
…#159129) Previously I masked issues with Multicall + DTLTO (see llvm#148908) due to an incomplete understanding of how the Multicall toolchain works. This patch reverts those incorrect changes and instead marks the affected tests XFAIL when running under Multicall. Issue llvm#159125 tracks fixing DTLTO with Multicall.
…#159158) Make the test regexes more permissive to fix buildbot failures caused by the merge of PR llvm#159129. This mirrors the earlier fix in PR llvm#148908. We retain cross-project-test coverage to verify that the path content is correct. A follow-up will update the tests to robustly check the Clang executable filename, likely along the lines of PR llvm#159151. Short-term unbreak to keep the buildbots green without reverting a chain of commits.
Not all builds name the compiler executable
clang
. For example, the Fuchsia build bots usellvm
as their single toolchain executable name, as they combine everything together in a busybox-style binary.Update the Clang driver test 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 test will verify that the path is valid later.Should fix #147265.