Skip to content

Commit 9850e82

Browse files
Nils WireklintEdSchouten
Nils Wireklint
authored and
EdSchouten
committed
Share Command Creator for Windows
1 parent 73fa988 commit 9850e82

File tree

3 files changed

+98
-120
lines changed

3 files changed

+98
-120
lines changed

‎pkg/runner/local_runner.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ package runner
33
import (
44
"context"
55
"errors"
6+
"math"
7+
"os"
68
"os/exec"
9+
"path/filepath"
10+
"strings"
11+
"syscall"
712

813
"github.com/buildbarn/bb-remote-execution/pkg/proto/runner"
914
"github.com/buildbarn/bb-storage/pkg/filesystem"
@@ -88,6 +93,37 @@ func (r *localRunner) openLog(logPath string) (filesystem.FileAppender, error) {
8893
// on whether the action needs to be run in a chroot() or not.
8994
type CommandCreator func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error)
9095

96+
// NewPlainCommandCreator returns a CommandCreator for cases where we don't
97+
// need to chroot into the input root directory.
98+
func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
99+
return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
100+
workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
101+
if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
102+
return nil, util.StatusWrap(err, "Failed to resolve working directory")
103+
}
104+
workingDirectoryStr, err := path.GetLocalString(workingDirectory)
105+
if err != nil {
106+
return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
107+
}
108+
executablePath, err := lookupExecutable(workingDirectory, pathVariable, arguments[0])
109+
if err != nil {
110+
return nil, err
111+
}
112+
113+
// exec.CommandContext() has some smartness to call
114+
// exec.LookPath() under the hood, which we don't want.
115+
// Call it with a placeholder path, followed by setting
116+
// cmd.Path and cmd.Args manually. This ensures that our
117+
// own values remain respected.
118+
cmd := exec.CommandContext(ctx, "/nonexistent")
119+
cmd.Args = arguments
120+
cmd.Dir = workingDirectoryStr
121+
cmd.Path = executablePath
122+
cmd.SysProcAttr = sysProcAttr
123+
return cmd, nil
124+
}
125+
}
126+
91127
// NewLocalRunner returns a Runner capable of running commands on the
92128
// local system directly.
93129
func NewLocalRunner(buildDirectory filesystem.Directory, buildDirectoryPath *path.Builder, commandCreator CommandCreator, setTmpdirEnvironmentVariable bool) runner.RunnerServer {
@@ -201,3 +237,63 @@ func (r *localRunner) CheckReadiness(ctx context.Context, request *runner.CheckR
201237

202238
return &emptypb.Empty{}, nil
203239
}
240+
241+
// getExecutablePath returns the path of an executable within a given
242+
// search path that is part of the PATH environment variable.
243+
func getExecutablePath(baseDirectory *path.Builder, searchPathStr, argv0 string) (string, error) {
244+
searchPath, scopeWalker := baseDirectory.Join(path.VoidScopeWalker)
245+
if err := path.Resolve(path.NewLocalParser(searchPathStr), scopeWalker); err != nil {
246+
return "", err
247+
}
248+
249+
executablePath, scopeWalker := searchPath.Join(path.VoidScopeWalker)
250+
if err := path.Resolve(path.NewLocalParser(argv0), scopeWalker); err != nil {
251+
return "", err
252+
}
253+
return path.GetLocalString(executablePath)
254+
}
255+
256+
// lookupExecutable returns the path of an executable, taking the PATH
257+
// environment variable into account.
258+
func lookupExecutable(workingDirectory *path.Builder, pathVariable, argv0 string) (string, error) {
259+
if strings.ContainsFunc(argv0, func(r rune) bool {
260+
return r <= math.MaxUint8 && os.IsPathSeparator(uint8(r))
261+
}) {
262+
// No PATH processing needs to be performed.
263+
return argv0, nil
264+
}
265+
266+
// Executable path does not contain any slashes. Perform PATH
267+
// lookups.
268+
//
269+
// We cannot use exec.LookPath() directly, as that function
270+
// disregards the working directory of the action. It also uses
271+
// the PATH environment variable of the current process, as
272+
// opposed to respecting the value that is provided as part of
273+
// the action. Do call into this function to validate the
274+
// existence of the executable.
275+
for _, searchPathStr := range filepath.SplitList(pathVariable) {
276+
executablePathAbs, err := getExecutablePath(workingDirectory, searchPathStr, argv0)
277+
if err != nil {
278+
return "", util.StatusWrapf(err, "Failed to resolve executable %#v in search path %#v", argv0, searchPathStr)
279+
}
280+
if _, err := exec.LookPath(executablePathAbs); err == nil {
281+
// Regular compiled executables will receive the
282+
// argv[0] that we provide, but scripts starting
283+
// with '#!' will receive the literal executable
284+
// path.
285+
//
286+
// Most shells seem to guarantee that if argv[0]
287+
// is relative, the executable path is relative
288+
// as well. Prevent these scripts from breaking
289+
// by recomputing the executable path once more,
290+
// but relative.
291+
executablePathRel, err := getExecutablePath(&path.EmptyBuilder, searchPathStr, argv0)
292+
if err != nil {
293+
return "", util.StatusWrapf(err, "Failed to resolve executable %#v in search path %#v", argv0, searchPathStr)
294+
}
295+
return executablePathRel, nil
296+
}
297+
}
298+
return "", status.Errorf(codes.InvalidArgument, "Cannot find executable %#v in search paths %#v", argv0, pathVariable)
299+
}

‎pkg/runner/local_runner_unix.go‎

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"context"
88
"os"
99
"os/exec"
10-
"path/filepath"
1110
"strings"
1211
"syscall"
1312

@@ -16,100 +15,9 @@ import (
1615
"github.com/buildbarn/bb-storage/pkg/util"
1716

1817
"golang.org/x/sys/unix"
19-
"google.golang.org/grpc/codes"
20-
"google.golang.org/grpc/status"
2118
"google.golang.org/protobuf/types/known/durationpb"
2219
)
2320

24-
// getExecutablePath returns the path of an executable within a given
25-
// search path that is part of the PATH environment variable.
26-
func getExecutablePath(baseDirectory *path.Builder, searchPathStr, argv0 string) (string, error) {
27-
searchPath, scopeWalker := baseDirectory.Join(path.VoidScopeWalker)
28-
if err := path.Resolve(path.NewLocalParser(searchPathStr), scopeWalker); err != nil {
29-
return "", err
30-
}
31-
32-
executablePath, scopeWalker := searchPath.Join(path.VoidScopeWalker)
33-
if err := path.Resolve(path.NewLocalParser(argv0), scopeWalker); err != nil {
34-
return "", err
35-
}
36-
return path.GetLocalString(executablePath)
37-
}
38-
39-
// lookupExecutable returns the path of an executable, taking the PATH
40-
// environment variable into account.
41-
func lookupExecutable(workingDirectory *path.Builder, pathVariable, argv0 string) (string, error) {
42-
if strings.ContainsRune(argv0, os.PathSeparator) {
43-
// No PATH processing needs to be performed.
44-
return argv0, nil
45-
}
46-
47-
// Executable path does not contain any slashes. Perform PATH
48-
// lookups.
49-
//
50-
// We cannot use exec.LookPath() directly, as that function
51-
// disregards the working directory of the action. It also uses
52-
// the PATH environment variable of the current process, as
53-
// opposed to respecting the value that is provided as part of
54-
// the action. Do call into this function to validate the
55-
// existence of the executable.
56-
for _, searchPathStr := range filepath.SplitList(pathVariable) {
57-
executablePathAbs, err := getExecutablePath(workingDirectory, searchPathStr, argv0)
58-
if err != nil {
59-
return "", util.StatusWrapf(err, "Failed to resolve executable %#v in search path %#v", argv0, searchPathStr)
60-
}
61-
if _, err := exec.LookPath(executablePathAbs); err == nil {
62-
// Regular compiled executables will receive the
63-
// argv[0] that we provide, but scripts starting
64-
// with '#!' will receive the literal executable
65-
// path.
66-
//
67-
// Most shells seem to guarantee that if argv[0]
68-
// is relative, the executable path is relative
69-
// as well. Prevent these scripts from breaking
70-
// by recomputing the executable path once more,
71-
// but relative.
72-
executablePathRel, err := getExecutablePath(&path.EmptyBuilder, searchPathStr, argv0)
73-
if err != nil {
74-
return "", util.StatusWrapf(err, "Failed to resolve executable %#v in search path %#v", argv0, searchPathStr)
75-
}
76-
return executablePathRel, nil
77-
}
78-
}
79-
return "", status.Errorf(codes.InvalidArgument, "Cannot find executable %#v in search paths %#v", argv0, pathVariable)
80-
}
81-
82-
// NewPlainCommandCreator returns a CommandCreator for cases where we don't
83-
// need to chroot into the input root directory.
84-
func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
85-
return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
86-
workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
87-
if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
88-
return nil, util.StatusWrap(err, "Failed to resolve working directory")
89-
}
90-
workingDirectoryStr, err := path.GetLocalString(workingDirectory)
91-
if err != nil {
92-
return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
93-
}
94-
executablePath, err := lookupExecutable(workingDirectory, pathVariable, arguments[0])
95-
if err != nil {
96-
return nil, err
97-
}
98-
99-
// exec.CommandContext() has some smartness to call
100-
// exec.LookPath() under the hood, which we don't want.
101-
// Call it with a placeholder path, followed by setting
102-
// cmd.Path and cmd.Args manually. This ensures that our
103-
// own values remain respected.
104-
cmd := exec.CommandContext(ctx, "/nonexistent")
105-
cmd.Args = arguments
106-
cmd.Dir = workingDirectoryStr
107-
cmd.Path = executablePath
108-
cmd.SysProcAttr = sysProcAttr
109-
return cmd, nil
110-
}
111-
}
112-
11321
// NewChrootedCommandCreator returns a CommandCreator for cases where we
11422
// need to chroot into the input root directory.
11523
func NewChrootedCommandCreator(sysProcAttr *syscall.SysProcAttr) (CommandCreator, error) {

‎pkg/runner/local_runner_windows.go‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,56 +4,30 @@
44
package runner
55

66
import (
7-
"context"
87
"os"
98
"os/exec"
109
"syscall"
1110

1211
"github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage"
13-
"github.com/buildbarn/bb-storage/pkg/filesystem/path"
14-
"github.com/buildbarn/bb-storage/pkg/util"
1512

1613
"golang.org/x/sys/windows"
1714
"google.golang.org/grpc/codes"
1815
"google.golang.org/grpc/status"
1916
"google.golang.org/protobuf/types/known/durationpb"
2017
)
2118

22-
// NewPlainCommandCreator returns a CommandCreator for cases where we don't
23-
// need to chroot into the input root directory.
24-
func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
25-
return func(ctx context.Context, arguments []string, inputRootDirectory *path.Builder, workingDirectoryParser path.Parser, pathVariable string) (*exec.Cmd, error) {
26-
// TODO: This may not work correctly if the action sets
27-
// the PATH environment variable explicitly.
28-
cmd := exec.CommandContext(ctx, arguments[0], arguments[1:]...)
29-
cmd.SysProcAttr = sysProcAttr
30-
31-
// Set the working relative to be relative to the input
32-
// root directory.
33-
workingDirectory, scopeWalker := inputRootDirectory.Join(path.VoidScopeWalker)
34-
if err := path.Resolve(workingDirectoryParser, scopeWalker); err != nil {
35-
return nil, util.StatusWrap(err, "Failed to resolve working directory")
36-
}
37-
workingDirectoryStr, err := path.GetLocalString(workingDirectory)
38-
if err != nil {
39-
return nil, util.StatusWrap(err, "Failed to create local representation of working directory")
40-
}
41-
cmd.Dir = workingDirectoryStr
42-
return cmd, nil
43-
}
44-
}
45-
4619
// NewChrootedCommandCreator gives an error on Windows, as chroot is not
4720
// supported on the platform.
4821
func NewChrootedCommandCreator(sysProcAttr *syscall.SysProcAttr) (CommandCreator, error) {
49-
return nil, status.Error(codes.InvalidArgument, "Chroot not supported on Windows")
22+
return nil, status.Error(codes.InvalidArgument, "Chroot is not supported on Windows")
5023
}
5124

5225
var temporaryDirectoryEnvironmentVariablePrefixes = [...]string{"TMP=", "TEMP="}
5326

5427
var invalidArgumentErrs = [...]error{exec.ErrNotFound, os.ErrPermission, os.ErrNotExist, windows.ERROR_BAD_EXE_FORMAT}
5528

5629
func getPOSIXResourceUsage(cmd *exec.Cmd) *resourceusage.POSIXResourceUsage {
30+
// TODO: These do not work.
5731
processState := cmd.ProcessState
5832
return &resourceusage.POSIXResourceUsage{
5933
UserTime: durationpb.New(processState.SystemTime()),

0 commit comments

Comments
 (0)