Share Command Creator for Windows

Author: Nils Wireklint

pkg/runner/local_runner.go

@@ -3,10 +3,12 @@ package runner
 import (
 	"context"
 	"errors"
+	"math"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"syscall"
 
 	"github.com/buildbarn/bb-remote-execution/pkg/proto/runner"
 	"github.com/buildbarn/bb-storage/pkg/filesystem"
@@ -91,6 +93,37 @@ func (r *localRunner) openLog(logPath string) (filesystem.FileAppender, error) {
 // on whether the action needs to be run in a chroot() or not.
 type CommandCreator func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error)
 
+// NewPlainCommandCreator returns a CommandCreator for cases where we don't
+// need to chroot into the input root directory.
+func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
+	return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
+		workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
+		if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
+			return nil, util.StatusWrap(err, "Failed to resolve working directory")
+		}
+		workingDirectoryStr, err := path.GetLocalString(workingDirectory)
+		if err != nil {
+			return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
+		}
+		executablePath, err := lookupExecutable(workingDirectory, pathVariable, arguments[0])
+		if err != nil {
+			return nil, err
+		}
+
+		// exec.CommandContext() has some smartness to call
+		// exec.LookPath() under the hood, which we don't want.
+		// Call it with a placeholder path, followed by setting
+		// cmd.Path and cmd.Args manually. This ensures that our
+		// own values remain respected.
+		cmd := exec.CommandContext(ctx, "/nonexistent")
+		cmd.Args = arguments
+		cmd.Dir = workingDirectoryStr
+		cmd.Path = executablePath
+		cmd.SysProcAttr = sysProcAttr
+		return cmd, nil
+	}
+}
+
 // NewLocalRunner returns a Runner capable of running commands on the
 // local system directly.
 func NewLocalRunner(buildDirectory filesystem.Directory, buildDirectoryPath *path.Builder, commandCreator CommandCreator, setTmpdirEnvironmentVariable bool) runner.RunnerServer {
@@ -225,7 +258,9 @@ func getExecutablePath(baseDirectory *path.Builder, searchPathStr, argv0 string)
 // This operates on platform native paths, or unix-style slash paths.
 // The latter are customarily sent by Bazel in multiplatform builds.
 func lookupExecutable(workingDirectory *path.Builder, pathVariable, argv0 string) (string, error) {
-	if strings.ContainsRune(argv0, os.PathSeparator) {
+	if strings.ContainsFunc(argv0, func(r rune) bool {
+		return r <= math.MaxUint8 && os.IsPathSeparator(uint8(r))
+	}) {
 		// No PATH processing needs to be performed.
 		return argv0, nil
 	}

pkg/runner/local_runner_unix.go

@@ -18,37 +18,6 @@ import (
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
-// NewPlainCommandCreator returns a CommandCreator for cases where we don't
-// need to chroot into the input root directory.
-func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
-	return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
-		workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
-		if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
-			return nil, util.StatusWrap(err, "Failed to resolve working directory")
-		}
-		workingDirectoryStr, err := path.GetLocalString(workingDirectory)
-		if err != nil {
-			return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
-		}
-		executablePath, err := lookupExecutable(workingDirectory, pathVariable, arguments[0])
-		if err != nil {
-			return nil, err
-		}
-
-		// exec.CommandContext() has some smartness to call
-		// exec.LookPath() under the hood, which we don't want.
-		// Call it with a placeholder path, followed by setting
-		// cmd.Path and cmd.Args manually. This ensures that our
-		// own values remain respected.
-		cmd := exec.CommandContext(ctx, "/nonexistent")
-		cmd.Args = arguments
-		cmd.Dir = workingDirectoryStr
-		cmd.Path = executablePath
-		cmd.SysProcAttr = sysProcAttr
-		return cmd, nil
-	}
-}
-
 // NewChrootedCommandCreator returns a CommandCreator for cases where we
 // need to chroot into the input root directory.
 func NewChrootedCommandCreator(sysProcAttr *syscall.SysProcAttr) (CommandCreator, error) {

pkg/runner/local_runner_windows.go

@@ -4,81 +4,22 @@
 package runner
 
 import (
-	"context"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"syscall"
 
 	"github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage"
-	"github.com/buildbarn/bb-storage/pkg/filesystem/path"
-	"github.com/buildbarn/bb-storage/pkg/util"
 
 	"golang.org/x/sys/windows"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
-// NewPlainCommandCreator returns a CommandCreator for cases where we don't
-// need to chroot into the input root directory.
-func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
-	return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
-		// TODO: This may not work correctly if the action sets
-		// the PATH environment variable explicitly.
-		//
-		// exec.CommandContext() has some smartness to call
-		// exec.LookPath() under the hood, which we don't want.
-		// Call it with a placeholder path, followed by setting
-		// cmd.Path and cmd.Args manually. This ensures that our
-		// own values remain respected.
-		argv0 := arguments[0]
-
-		pathBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
-		if err := path.Resolve(path.NewLocalParser(argv0), scopeWalker); err != nil {
-			return nil, err
-		}
-		argv0, err := pathBuilder.GetWindowsString()
-		if err != nil {
-			return nil, err
-		}
-
-		arguments[0] = "\\nonexistent-place-back-later"
-		cmd := exec.CommandContext(ctx, arguments[0], arguments[1:]...)
-		cmd.SysProcAttr = sysProcAttr
-
-		// Set the working relative to be relative to the input
-		// root directory.
-		workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
-		if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
-			return nil, util.StatusWrap(err, "Failed to resolve working directory")
-		}
-		workingDirectoryStr, err := path.GetLocalString(workingDirectory)
-		if err != nil {
-			return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
-		}
-		cmd.Dir = workingDirectoryStr
-
-		executablePath, err := lookupExecutable(workingDirectory, pathVariable, argv0)
-		if err != nil {
-			return nil, err
-		}
-
-		// Must use backslashes to execute relative paths
-		executablePath = filepath.FromSlash(executablePath)
-		arguments[0] = executablePath
-
-		cmd.Args = arguments
-		cmd.Path = executablePath
-		cmd.SysProcAttr = sysProcAttr
-		return cmd, nil
-	}
-}
-
 // NewChrootedCommandCreator gives an error on Windows, as chroot is not
 // supported on the platform.
 func NewChrootedCommandCreator(sysProcAttr *syscall.SysProcAttr) (CommandCreator, error) {
-	return nil, status.Error(codes.InvalidArgument, "Chroot not supported on Windows")
+	return nil, status.Error(codes.InvalidArgument, "Chroot is not supported on Windows")
 }
 
 var temporaryDirectoryEnvironmentVariablePrefixes = [...]string{"TMP=", "TEMP="}