Dedupe addition of crash-diagnostics-dir

For chromeos-chrome builds, we need to remove any unnecessary
{PVR} like info from the build cmd in order for cache hits across
builds.

chromeos-chrome already defines "crash_diagnostics-dir" in such a
manner, but compiler-wrapper adds a duplicate definition of this same
flag but with {PVR} like info.

The fix here is to update compiler-wrapper so that it does not add
"crash-diagnostics-dir" if it is already defined.

BUG=b:326847828
TEST=go test -vet=all (and added more tests)

Change-Id: I81c4dbb29241e85cf78659f35ac9dd93999b6b01
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/5321366
Tested-by: Eric Lok <lokeric@google.com>
Commit-Queue: Eric Lok <lokeric@google.com>
Auto-Submit: Eric Lok <lokeric@google.com>
Reviewed-by: Jordan Abrahams-Whitehead <ajordanr@google.com>
Commit-Queue: Ryan Beltran <ryanbeltran@chromium.org>
Reviewed-by: Ryan Beltran <ryanbeltran@chromium.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>

2 files changed, 60 insertions, 2 deletions

diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go
index e940c656..22109e3c 100644
--- a/compiler_wrapper/compiler_wrapper.go
+++ b/compiler_wrapper/compiler_wrapper.go
@@ -350,14 +350,28 @@ func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int
 	}
 }
 
+func hasFlag(flag string, flagList ...string) bool {
+	for _, flagVal := range flagList {
+		if strings.Contains(flagVal, flag) {
+			return true
+		}
+	}
+	return false
+}
+
 func prepareClangCommand(crashArtifactsDir string, builder *commandBuilder) (err error) {
 	if !builder.cfg.isHostWrapper {
 		processSysrootFlag(builder)
 	}
 	builder.addPreUserArgs(builder.cfg.clangFlags...)
-	if crashArtifactsDir != "" {
-		builder.addPreUserArgs("-fcrash-diagnostics-dir=" + crashArtifactsDir)
+
+	var crashDiagFlagName = "-fcrash-diagnostics-dir"
+	if crashArtifactsDir != "" &&
+		!hasFlag(crashDiagFlagName, builder.cfg.clangFlags...) &&
+		!hasFlag(crashDiagFlagName, builder.cfg.clangPostFlags...) {
+		builder.addPreUserArgs(crashDiagFlagName + "=" + crashArtifactsDir)
 	}
+
 	builder.addPostUserArgs(builder.cfg.clangPostFlags...)
 	calcCommonPreUserArgs(builder)
 	return processClangFlags(builder)
diff --git a/compiler_wrapper/compiler_wrapper_test.go b/compiler_wrapper/compiler_wrapper_test.go
index 52356f83..2cace6e9 100644
--- a/compiler_wrapper/compiler_wrapper_test.go
+++ b/compiler_wrapper/compiler_wrapper_test.go
@@ -252,3 +252,47 @@ func TestCalculateAndroidWrapperPath(t *testing.T) {
 		}
 	}
 }
+
+// If "crash-diagnostics-dir" flag is already provided, only use that flag and don't add a dupe
+func TestCrashDiagPreFlag(t *testing.T) {
+	withTestContext(t, func(ctx *testContext) {
+		ctx.cfg.clangFlags = []string{"-fcrash-diagnostics-dir=/build/something/foo"}
+		ctx.env = []string{
+			"CROS_ARTIFACTS_TMP_DIR=/tmp/foo",
+		}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(clangX86_64, mainCc)))
+		if err := verifyArgCount(cmd, 1, "-fcrash-diagnostics-dir=/build/something/foo"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+// If "crash-diagnostics-dir" flag is already provided, only use that flag and don't add a dupe
+func TestCrashDiagPostFlag(t *testing.T) {
+	withTestContext(t, func(ctx *testContext) {
+		ctx.cfg.clangPostFlags = []string{"-fcrash-diagnostics-dir=/build/something/foo"}
+		ctx.env = []string{
+			"CROS_ARTIFACTS_TMP_DIR=/tmp/foo",
+		}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(clangX86_64, mainCc)))
+		if err := verifyArgCount(cmd, 1, "-fcrash-diagnostics-dir=/build/something/foo"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+// If "crash-diagnostics-dir" flag is not provided, add one in
+func TestCrashDiagDefault(t *testing.T) {
+	withTestContext(t, func(ctx *testContext) {
+		ctx.env = []string{
+			"CROS_ARTIFACTS_TMP_DIR=/tmp/foo",
+		}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(clangX86_64, mainCc)))
+		if err := verifyArgCount(cmd, 1, "-fcrash-diagnostics-dir=/tmp/foo/toolchain/clang_crash_diagnostics"); err != nil {
+			t.Error(err)
+		}
+	})
+}