[compiler-wrapper] Handle double build for clang-tidy
For clang-tidy, the equivalent of -Werror is --warnings-as-errors (or
-warnings-as-errors). The error message will also have
"warnings-as-errors". So,
1. Trigger double-build if "warnings-as-errors" is in *stdout*.
Clang-tidy will also fail if clang issues a Werror diagnostic. In this
case, we'd get "clang-diagnostic" in stdout. Do a re-build in both
these cases.
2. Remove flags containing "-warnings-as-errors" in the rerun. Unlike
-Werror/Wno-error, clang-tidy doesn't have a -no-warnings-as-errors to
override a prior -warnings-as-errors. Existing double-build processing
will disable -Werror flags to Clang.
Originally reviewed at
https://android-review.googlesource.com/c/platform/external/toolchain-utils/+/1322200.
BUG=None
TEST=go test
Change-Id: I18de245972da81e0ac3600f9098b71cec82e9e96
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2239370
Commit-Queue: Pirama Arumuga Nainar <pirama@google.com>
Tested-by: Pirama Arumuga Nainar <pirama@google.com>
Reviewed-by: George Burgess <gbiv@chromium.org>
diff --git a/compiler_wrapper/disable_werror_flag.go b/compiler_wrapper/disable_werror_flag.go
index f68786d..75f71a0 100644
--- a/compiler_wrapper/disable_werror_flag.go
+++ b/compiler_wrapper/disable_werror_flag.go
@@ -14,6 +14,8 @@
"syscall"
)
+const numWErrorEstimate = 30
+
func getNewWarningsDir(env env, cfg *config) (dirName string, ok bool) {
if cfg.isAndroidWrapper {
if cfg.useLlvmNext {
@@ -32,13 +34,17 @@
}
func disableWerrorFlags(originalArgs []string) []string {
- noErrors := []string{"-Wno-error"}
+ extraArgs := []string{"-Wno-error"}
+ newArgs := make([]string, 0, len(originalArgs)+numWErrorEstimate)
for _, flag := range originalArgs {
if strings.HasPrefix(flag, "-Werror=") {
- noErrors = append(noErrors, strings.Replace(flag, "-Werror", "-Wno-error", 1))
+ extraArgs = append(extraArgs, strings.Replace(flag, "-Werror", "-Wno-error", 1))
+ }
+ if !strings.Contains(flag, "-warnings-as-errors") {
+ newArgs = append(newArgs, flag)
}
}
- return noErrors
+ return append(newArgs, extraArgs...)
}
func isLikelyAConfTest(cfg *config, cmd *command) bool {
@@ -75,9 +81,16 @@
if err != nil {
return 0, err
}
+
// The only way we can do anything useful is if it looks like the failure
// was -Werror-related.
- if originalExitCode == 0 || !bytes.Contains(originalStderrBuffer.Bytes(), []byte("-Werror")) || isLikelyAConfTest(cfg, originalCmd) {
+ originalStdoutBufferBytes := originalStdoutBuffer.Bytes()
+ shouldRetry := originalExitCode != 0 &&
+ !isLikelyAConfTest(cfg, originalCmd) &&
+ (bytes.Contains(originalStderrBuffer.Bytes(), []byte("-Werror")) ||
+ bytes.Contains(originalStdoutBufferBytes, []byte("warnings-as-errors")) ||
+ bytes.Contains(originalStdoutBufferBytes, []byte("clang-diagnostic-")))
+ if !shouldRetry {
originalStdoutBuffer.WriteTo(env.stdout())
originalStderrBuffer.WriteTo(env.stderr())
return originalExitCode, nil
@@ -87,7 +100,7 @@
retryStderrBuffer := &bytes.Buffer{}
retryCommand := &command{
Path: originalCmd.Path,
- Args: append(originalCmd.Args, disableWerrorFlags(originalCmd.Args)...),
+ Args: disableWerrorFlags(originalCmd.Args),
EnvUpdates: originalCmd.EnvUpdates,
}
retryExitCode, err := wrapSubprocessErrorWithSourceLoc(retryCommand,
diff --git a/compiler_wrapper/disable_werror_flag_test.go b/compiler_wrapper/disable_werror_flag_test.go
index 53b0fb5..28e4837 100644
--- a/compiler_wrapper/disable_werror_flag_test.go
+++ b/compiler_wrapper/disable_werror_flag_test.go
@@ -449,3 +449,72 @@
}
})
}
+
+func TestClangTidyNoDoubleBuild(t *testing.T) {
+ withTestContext(t, func(ctx *testContext) {
+ ctx.cfg.isAndroidWrapper = true
+ ctx.cfg.useLlvmNext = true
+ ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangTidyAndroid, "--", mainCc)))
+ if ctx.cmdCount != 1 {
+ t.Errorf("expected 1 call. Got: %d", ctx.cmdCount)
+ }
+ })
+}
+
+func withAndroidClangTidyTestContext(t *testing.T, work func(ctx *testContext)) {
+ withTestContext(t, func(ctx *testContext) {
+ ctx.cfg.isAndroidWrapper = true
+ ctx.cfg.useLlvmNext = true
+ ctx.env = []string{"OUT_DIR=/tmp"}
+
+ ctx.cmdMock = func(cmd *command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
+ hasArg := func(s string) bool {
+ for _, e := range cmd.Args {
+ if strings.Contains(e, s) {
+ return true
+ }
+ }
+ return false
+ }
+ switch ctx.cmdCount {
+ case 1:
+ if hasArg("-Werror") {
+ fmt.Fprint(stdout, "clang-diagnostic-")
+ return newExitCodeError(1)
+ }
+ if hasArg("-warnings-as-errors") {
+ fmt.Fprint(stdout, "warnings-as-errors")
+ return newExitCodeError(1)
+ }
+ return nil
+ case 2:
+ if hasArg("warnings-as-errors") {
+ return fmt.Errorf("Unexpected arg warnings-as-errors found. All args: %s", cmd.Args)
+ }
+ return nil
+ default:
+ t.Fatalf("unexpected command: %#v", cmd)
+ return nil
+ }
+ }
+ work(ctx)
+ })
+}
+
+func TestClangTidyDoubleBuildClangTidyError(t *testing.T) {
+ withAndroidClangTidyTestContext(t, func(ctx *testContext) {
+ ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangTidyAndroid, "-warnings-as-errors=*", "--", mainCc)))
+ if ctx.cmdCount != 2 {
+ t.Errorf("expected 2 calls. Got: %d", ctx.cmdCount)
+ }
+ })
+}
+
+func TestClangTidyDoubleBuildClangError(t *testing.T) {
+ withAndroidClangTidyTestContext(t, func(ctx *testContext) {
+ ctx.must(callCompiler(ctx, ctx.cfg, ctx.newCommand(clangTidyAndroid, "-Werrors=*", "--", mainCc)))
+ if ctx.cmdCount != 2 {
+ t.Errorf("expected 2 calls. Got: %d", ctx.cmdCount)
+ }
+ })
+}
diff --git a/compiler_wrapper/testutil_test.go b/compiler_wrapper/testutil_test.go
index 57a68df..5d8ef92 100644
--- a/compiler_wrapper/testutil_test.go
+++ b/compiler_wrapper/testutil_test.go
@@ -17,15 +17,18 @@
"testing"
)
-const mainCc = "main.cc"
-const clangAndroid = "./clang"
-const clangX86_64 = "./x86_64-cros-linux-gnu-clang"
-const gccX86_64 = "./x86_64-cros-linux-gnu-gcc"
-const gccX86_64Eabi = "./x86_64-cros-eabi-gcc"
-const gccArmV7 = "./armv7m-cros-linux-gnu-gcc"
-const gccArmV7Eabi = "./armv7m-cros-eabi-gcc"
-const gccArmV8 = "./armv8m-cros-linux-gnu-gcc"
-const gccArmV8Eabi = "./armv8m-cros-eabi-gcc"
+const (
+ mainCc = "main.cc"
+ clangAndroid = "./clang"
+ clangTidyAndroid = "./clang-tidy"
+ clangX86_64 = "./x86_64-cros-linux-gnu-clang"
+ gccX86_64 = "./x86_64-cros-linux-gnu-gcc"
+ gccX86_64Eabi = "./x86_64-cros-eabi-gcc"
+ gccArmV7 = "./armv7m-cros-linux-gnu-gcc"
+ gccArmV7Eabi = "./armv7m-cros-eabi-gcc"
+ gccArmV8 = "./armv8m-cros-linux-gnu-gcc"
+ gccArmV8Eabi = "./armv8m-cros-eabi-gcc"
+)
type testContext struct {
t *testing.T
