Prevent tools/bazel from breaking flags-as-proto parsing (#9350)

Author: bduffany

cli/bazelisk/bazelisk.go

@@ -21,6 +21,12 @@ var (
 	setVersionErr  error
 )
 
+const (
+	// bazelisk environment variable name that skips tools/bazel if set to a
+	// non-empty string.
+	skipWrapperEnvVar = "BAZELISK_SKIP_WRAPPER"
+)
+
 type RunOpts struct {
 	// Stdout is the Writer where bazelisk should write its stdout.
 	// Defaults to os.Stdout if nil.
@@ -29,9 +35,26 @@ type RunOpts struct {
 	// Stderr is the Writer where bazelisk should write its stderr.
 	// Defaults to os.Stderr if nil.
 	Stderr io.Writer
+
+	// SkipWrapper skips the tools/bazel wrapper if it exists.
+	SkipWrapper bool
 }
 
 func Run(args []string, opts *RunOpts) (exitCode int, err error) {
+	if opts.SkipWrapper {
+		prev, ok := os.LookupEnv(skipWrapperEnvVar)
+		if err := os.Setenv(skipWrapperEnvVar, "true"); err != nil {
+			return -1, fmt.Errorf("failed to set %s: %s", skipWrapperEnvVar, err)
+		}
+		// Reset BAZELISK_SKIP_WRAPPER to its previous state after running
+		// bazelisk.
+		if ok {
+			defer os.Setenv(skipWrapperEnvVar, prev)
+		} else {
+			defer os.Unsetenv(skipWrapperEnvVar)
+		}
+	}
+
 	// If we were already invoked via bazelisk, then set the bazel version to
 	// the next version appearing in the .bazelversion file so that bazelisk
 	// doesn't just invoke us again (resulting in an infinite loop).
@@ -80,7 +103,7 @@ func Run(args []string, opts *RunOpts) (exitCode int, err error) {
 // This will return true when referencing a CLI release in .bazelversion such as
 // "buildbuddy-io/0.0.13" and then running `bazelisk`.
 func IsInvokedByBazelisk() bool {
-	return filepath.Base(os.Args[0]) == "bazelisk" || os.Getenv("BAZELISK_SKIP_WRAPPER") == "true"
+	return filepath.Base(os.Args[0]) == "bazelisk" || os.Getenv(skipWrapperEnvVar) == "true"
 }
 
 // makePipeWriter adapts a writer to an *os.File by using an os.Pipe().

cli/parser/parser.go

@@ -503,7 +503,11 @@ func runBazelHelpWithCache() (string, error) {
 	buf := &bytes.Buffer{}
 	errBuf := &bytes.Buffer{}
 	log.Debugf("\x1b[90mGathering metadata for bazel %s...\x1b[m", topic)
-	opts := &bazelisk.RunOpts{Stdout: io.MultiWriter(tmp, buf), Stderr: errBuf}
+	opts := &bazelisk.RunOpts{
+		Stdout:      io.MultiWriter(tmp, buf),
+		Stderr:      errBuf,
+		SkipWrapper: true,
+	}
 	exitCode, err := bazelisk.Run([]string{
 		"--ignore_all_rc_files",
 		// Run in a temp output base to avoid messing with any running bazel

cli/test/integration/cli/cli_test.go

@@ -384,6 +384,21 @@ startup --host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1
 	require.NotContains(t, string(b), "Running Bazel server needs to be killed")
 }
 
+func TestHelpWithBazelWrapper(t *testing.T) {
+	ws := testcli.NewWorkspace(t)
+	testfs.WriteAllFileContents(t, ws, map[string]string{
+		"BUILD": "",
+		"tools/bazel": `#!/usr/bin/env bash
+echo "A message from ./tools/bazel that tries to break bb's parsing logic!"
+"$BAZEL_REAL" "$@"
+`,
+	})
+	testfs.MakeExecutable(t, ws, "tools/bazel")
+	cmd := testcli.BazelCommand(t, ws, "build", "//...")
+	b, err := testcli.CombinedOutput(cmd)
+	require.NoErrorf(t, err, "output: %s", string(b))
+}
+
 func retryUntilSuccess(t *testing.T, f func() error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()