compiler_wrapper: add --rewrapper-path support

This CL adds --rewrapper-path support to the compiler, as requested in
the linked bug. The use of this is mutually exclusive with that of
--gomacc-path.

We currently look for either a `--gomacc-path` flag or a `GOMACC_PATH`
env var to enable goma. If any rewrapper flag is specified, this
disables our checking of `GOMACC_PATH` for the purposes of mutual
exclusivity checking.

BUG=b:190741226
TEST=go test, a few manual invocations

Change-Id: I03dd2835a313806a9700c49c47c82fecf19a4b7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2956696
Reviewed-by: Ryan Beltran <ryanbeltran@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>

4 files changed, 92 insertions, 8 deletions

diff --git a/compiler_wrapper/command.go b/compiler_wrapper/command.go
index e6ea6690..8b9fa235 100644
--- a/compiler_wrapper/command.go
+++ b/compiler_wrapper/command.go
@@ -200,8 +200,10 @@ func (builder *commandBuilder) clone() *commandBuilder {
 	}
 }
 
-func (builder *commandBuilder) wrapPath(path string) {
-	builder.args = append([]builderArg{{value: builder.path, fromUser: false}}, builder.args...)
+func (builder *commandBuilder) wrapPath(path string, extraFlags ...string) {
+	newArgs := createBuilderArgs( /*fromUser=*/ false, extraFlags)
+	newArgs = append(newArgs, builderArg{value: builder.path, fromUser: false})
+	builder.args = append(newArgs, builder.args...)
 	builder.path = path
 }
 
diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go
index ebc1c42a..e772484d 100644
--- a/compiler_wrapper/compiler_wrapper.go
+++ b/compiler_wrapper/compiler_wrapper.go
@@ -91,8 +91,9 @@ func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int
 			mainBuilder.addPreUserArgs(mainBuilder.cfg.clangFlags...)
 			mainBuilder.addPreUserArgs(mainBuilder.cfg.commonFlags...)
 			mainBuilder.addPostUserArgs(mainBuilder.cfg.clangPostFlags...)
+			inheritGomaFromEnv := true
 			// Android doesn't support rewrapper; don't try to use it.
-			if remoteBuildUsed, err = processGomaCccFlags(mainBuilder); err != nil {
+			if remoteBuildUsed, err = processGomaCccFlags(mainBuilder, inheritGomaFromEnv); err != nil {
 				return 0, err
 			}
 			compilerCmd = mainBuilder.build()
diff --git a/compiler_wrapper/remote_build_flag_test.go b/compiler_wrapper/remote_build_flag_test.go
index a4362274..4a894179 100644
--- a/compiler_wrapper/remote_build_flag_test.go
+++ b/compiler_wrapper/remote_build_flag_test.go
@@ -192,3 +192,60 @@ func withGomaccTestContext(t *testing.T, f func(*testContext, string)) {
 		f(ctx, gomaPath)
 	})
 }
+
+func TestRewrapperDefersToTheWrapperProperly(t *testing.T) {
+	withTestContext(t, func(ctx *testContext) {
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(gccX86_64, mainCc, "--rewrapper-path", "/rewrapper", "--rewrapper-cfg", "/some-cfg", "some", "other", "args")))
+		if err := verifyPath(cmd, "/rewrapper"); err != nil {
+			t.Error(err)
+		}
+		if err := verifyArgOrder(cmd, "-cfg", "/some-cfg", gccX86_64+".real", mainCc, "some", "other", "args"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestRewrapperCfgMustBePrsentIfRewrapperPathIs(t *testing.T) {
+	withGomaccTestContext(t, func(ctx *testContext, gomaPath string) {
+		stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(gccX86_64, mainCc, "--rewrapper-path", "/rewrapper")))
+		if err := verifyNonInternalError(stderr, "--rewrapper-cfg must be specified if --rewrapper-path is"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestRewrapperPathMustBePrsentIfRewrapperCfgIs(t *testing.T) {
+	withGomaccTestContext(t, func(ctx *testContext, gomaPath string) {
+		stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(gccX86_64, mainCc, "--rewrapper-cfg", "/some-cfg")))
+		if err := verifyNonInternalError(stderr, "--rewrapper-path must be specified if --rewrapper-cfg is"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestRewrapperAndGomaAreMutuallyExclusive(t *testing.T) {
+	withGomaccTestContext(t, func(ctx *testContext, gomaPath string) {
+		stderr := ctx.mustFail(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(gccX86_64, mainCc, "--rewrapper-path", "/rewrapper", "--rewrapper-cfg", "/some-cfg", "--gomacc-path", gomaPath)))
+		if err := verifyNonInternalError(stderr, "rewrapper and gomacc are mutually exclusive"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestRewrapperBlocksGomaInheritanceFromEnv(t *testing.T) {
+	withGomaccTestContext(t, func(ctx *testContext, gomaPath string) {
+		ctx.env = []string{"GOMACC_PATH=" + gomaPath}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(gccX86_64, mainCc, "--rewrapper-path", "/rewrapper", "--rewrapper-cfg", "/some-cfg")))
+		if err := verifyPath(cmd, "/rewrapper"); err != nil {
+			t.Error(err)
+		}
+		if err := verifyArgOrder(cmd, "-cfg", "/some-cfg", gccX86_64+".real", mainCc); err != nil {
+			t.Error(err)
+		}
+	})
+}
diff --git a/compiler_wrapper/remote_build_flags.go b/compiler_wrapper/remote_build_flags.go
index fa70cbc3..fc26c93f 100644
--- a/compiler_wrapper/remote_build_flags.go
+++ b/compiler_wrapper/remote_build_flags.go
@@ -93,13 +93,13 @@ func removeOneUserCmdlineFlagWithValue(builder *commandBuilder, flagName string)
 	}
 }
 
-func processGomaCccFlags(builder *commandBuilder) (gomaUsed bool, err error) {
+func processGomaCccFlags(builder *commandBuilder, inheritFromEnv bool) (gomaUsed bool, err error) {
 	gomaPath, err := removeOneUserCmdlineFlagWithValue(builder, "--gomacc-path")
 	if err != nil && err != errNoSuchCmdlineArg {
 		return false, err
 	}
 
-	if err == errNoSuchCmdlineArg || gomaPath == "" {
+	if inheritFromEnv && (err == errNoSuchCmdlineArg || gomaPath == "") {
 		gomaPath, _ = builder.env.getenv("GOMACC_PATH")
 	}
 
@@ -113,8 +113,31 @@ func processGomaCccFlags(builder *commandBuilder) (gomaUsed bool, err error) {
 }
 
 func processRewrapperCcFlags(builder *commandBuilder) (rewrapperUsed bool, err error) {
-	// FIXME(gbiv): Add parsing and such for this.
-	return false, nil
+	rewrapperPath, pathErr := removeOneUserCmdlineFlagWithValue(builder, "--rewrapper-path")
+	if pathErr != nil && pathErr != errNoSuchCmdlineArg {
+		return false, err
+	}
+
+	rewrapperCfg, cfgErr := removeOneUserCmdlineFlagWithValue(builder, "--rewrapper-cfg")
+	if cfgErr != nil && cfgErr != errNoSuchCmdlineArg {
+		return false, err
+	}
+
+	if pathErr == errNoSuchCmdlineArg {
+		if cfgErr != errNoSuchCmdlineArg {
+			return false, newUserErrorf("--rewrapper-path must be specified if --rewrapper-cfg is")
+		}
+		return false, nil
+	}
+
+	if cfgErr == errNoSuchCmdlineArg {
+		return false, newUserErrorf("--rewrapper-cfg must be specified if --rewrapper-path is")
+	}
+
+	// It's unclear that we should have a similar fallback to gomacc if --rewrapper-path doesn't
+	// exist, so don't until it's obviously necessary.
+	builder.wrapPath(rewrapperPath, "-cfg", rewrapperCfg)
+	return true, nil
 }
 
 func processRemoteBuildFlags(builder *commandBuilder) (remoteBuildUsed bool, err error) {
@@ -123,7 +146,8 @@ func processRemoteBuildFlags(builder *commandBuilder) (remoteBuildUsed bool, err
 		return rewrapperUsed, err
 	}
 
-	gomaUsed, err := processGomaCccFlags(builder)
+	inheritGomaFromEnv := !rewrapperUsed
+	gomaUsed, err := processGomaCccFlags(builder, inheritGomaFromEnv)
 	remoteBuildUsed = gomaUsed || rewrapperUsed
 	if err != nil {
 		return remoteBuildUsed, err