Report a proper exit code when Bazel is terminated by signal and use exec on Unix in the case of bazelisk run (#757)

* Better handle Bazel process termination caused by signals on POSIX-compatible systems

* Use `exec` for running bazel on Unix instead of starting it as a child process in the case of `run` command

Fixes #512 and #556

Author: valco1994

core/core.go

@@ -180,7 +180,7 @@ func RunBazeliskWithArgsFuncAndConfigAndOut(argsFunc ArgsFunc, repos *Repositori
 		return 0, nil
 	}
 
-	exitCode, err := runBazel(bazelInstallation.Path, args, out, config)
+	exitCode, err := runBazel(bazelInstallation.Path, args, out, config, isRunCommand(args))
 	if err != nil {
 		return -1, fmt.Errorf("could not run Bazel: %v", err)
 	}
@@ -727,11 +727,35 @@ func makeBazelCmd(bazel string, args []string, out io.Writer, config config.Conf
 	return cmd, nil
 }
 
-func runBazel(bazel string, args []string, out io.Writer, config config.Config) (int, error) {
+func isRunCommand(args []string) bool {
+	for _, arg := range args {
+		if arg == "--" {
+			return false
+		}
+		if arg == "run" {
+			return true
+		}
+	}
+	return false
+}
+
+func runBazel(bazel string, args []string, out io.Writer, config config.Config, useExecOnUnix bool) (int, error) {
 	cmd, makeCmdErr := makeBazelCmd(bazel, args, out, config)
 	if makeCmdErr != nil {
 		return 1, makeCmdErr
 	}
+	if useExecOnUnix && runtime.GOOS != "windows" {
+		execPath := cmd.Path
+		execArgs := cmd.Args
+		execEnv := cmd.Env
+		err := syscall.Exec(execPath, execArgs, execEnv)
+		if err != nil {
+			return 1, fmt.Errorf("could not exec Bazel: %v", err)
+		}
+		// This code is unreachable if exec succeeds
+		return 0, nil
+	}
+
 	err := cmd.Start()
 	if err != nil {
 		return 1, fmt.Errorf("could not start Bazel: %v", err)
@@ -750,7 +774,6 @@ func runBazel(bazel string, args []string, out io.Writer, config config.Config)
 	// by the terminal. As a side effect, we also suppress the printing of a
 	// Go stack trace upon receiving SIGQUIT, which is unhelpful as users tend
 	// to report it instead of the far more valuable Java thread dump.
-	// TODO(#512): We may want to treat a `bazel run` command differently.
 	// Since signal handlers are process-wide global state and bazelisk may be
 	// used as a library, reset the signal handlers after the process exits.
 	sigCh := make(chan os.Signal, 1)
@@ -761,7 +784,18 @@ func runBazel(bazel string, args []string, out io.Writer, config config.Config)
 	if err != nil {
 		if exitError, ok := err.(*exec.ExitError); ok {
 			waitStatus := exitError.Sys().(syscall.WaitStatus)
-			return waitStatus.ExitStatus(), nil
+			// it's only correct to use waitStatus.ExitStatus when the process terminated normally, i.e. waitStatus.Exited() == true
+			if waitStatus.Exited() {
+				return waitStatus.ExitStatus(), nil
+			}
+			// if the process was terminated by a signal on a POSIX-compatible system, let's report its exit code in the same way
+			// as shells do - as 128 + signal number.
+			// It's not a perfect solution, because information that a signal has terminated the process is lost,
+			// but at least we propagate a proper exit code.
+			if runtime.GOOS != "windows" && waitStatus.Signaled() {
+				return 128 + int(waitStatus.Signal()), nil
+			}
+			return 1, fmt.Errorf("unexpected wait status of Bazel: %v", waitStatus)
 		}
 		return 1, fmt.Errorf("could not launch Bazel: %v", err)
 	}
@@ -776,7 +810,7 @@ func getIncompatibleFlags(bazelPath, cmd string, config config.Config) ([]string
 	}
 
 	out := strings.Builder{}
-	if _, err := runBazel(bazelPath, []string{"help", cmd, "--short"}, &out, config); err != nil {
+	if _, err := runBazel(bazelPath, []string{"help", cmd, "--short"}, &out, config, false); err != nil {
 		return nil, fmt.Errorf("unable to determine incompatible flags with binary %s: %v", bazelPath, err)
 	}
 
@@ -852,7 +886,7 @@ func shutdownIfNeeded(bazelPath string, startupOptions []string, config config.C
 
 	args := append(startupOptions, "shutdown")
 	fmt.Printf("bazel %s\n", strings.Join(args, " "))
-	exitCode, err := runBazel(bazelPath, args, nil, config)
+	exitCode, err := runBazel(bazelPath, args, nil, config, false)
 	fmt.Printf("\n")
 	if err != nil {
 		log.Fatalf("failed to run bazel shutdown: %v", err)
@@ -871,7 +905,7 @@ func cleanIfNeeded(bazelPath string, startupOptions []string, config config.Conf
 
 	args := append(startupOptions, "clean", "--expunge")
 	fmt.Printf("bazel %s\n", strings.Join(args, " "))
-	exitCode, err := runBazel(bazelPath, args, nil, config)
+	exitCode, err := runBazel(bazelPath, args, nil, config, false)
 	fmt.Printf("\n")
 	if err != nil {
 		log.Fatalf("failed to run clean: %v", err)
@@ -1070,7 +1104,7 @@ func testWithBazelAtCommit(bazelCommit string, args []string, bazeliskHome strin
 	shutdownIfNeeded(bazelPath, startupOptions, config)
 	cleanIfNeeded(bazelPath, startupOptions, config)
 	fmt.Printf("bazel %s\n", strings.Join(args, " "))
-	bazelExitCode, err := runBazel(bazelPath, args, nil, config)
+	bazelExitCode, err := runBazel(bazelPath, args, nil, config, false)
 	if err != nil {
 		return -1, fmt.Errorf("could not run Bazel: %v", err)
 	}
@@ -1087,7 +1121,7 @@ func migrate(bazelPath string, baseArgs []string, flags []string, config config.
 	shutdownIfNeeded(bazelPath, startupOptions, config)
 	cleanIfNeeded(bazelPath, startupOptions, config)
 	fmt.Printf("bazel %s\n", strings.Join(args, " "))
-	exitCode, err := runBazel(bazelPath, args, nil, config)
+	exitCode, err := runBazel(bazelPath, args, nil, config, false)
 	if err != nil {
 		log.Fatalf("could not run Bazel: %v", err)
 	}
@@ -1102,7 +1136,7 @@ func migrate(bazelPath string, baseArgs []string, flags []string, config config.
 	shutdownIfNeeded(bazelPath, startupOptions, config)
 	cleanIfNeeded(bazelPath, startupOptions, config)
 	fmt.Printf("bazel %s\n", strings.Join(args, " "))
-	exitCode, err = runBazel(bazelPath, args, nil, config)
+	exitCode, err = runBazel(bazelPath, args, nil, config, false)
 	if err != nil {
 		log.Fatalf("could not run Bazel: %v", err)
 	}
@@ -1120,7 +1154,7 @@ func migrate(bazelPath string, baseArgs []string, flags []string, config config.
 		shutdownIfNeeded(bazelPath, startupOptions, config)
 		cleanIfNeeded(bazelPath, startupOptions, config)
 		fmt.Printf("bazel %s\n", strings.Join(args, " "))
-		exitCode, err = runBazel(bazelPath, args, nil, config)
+		exitCode, err = runBazel(bazelPath, args, nil, config, false)
 		if err != nil {
 			log.Fatalf("could not run Bazel: %v", err)
 		}