From e95641649b5b4d3c582c89daabfaabeb8189dd77 Mon Sep 17 00:00:00 2001 From: Ed Schouten Date: Tue, 12 Apr 2022 15:20:52 +0200 Subject: [PATCH] Tidy up the semantics of Command.arguments[0] (#210) Bazel Buildfarm, Buildgrid and Buildbarn all perform resolution relative to the working directory of an action; not the input root directory. Instead of requiring that all implementations are updated, we should consider just altering the spec. Performing resolution relative to the input root directory can also be very tricky, as it means that argv[0] as visible to the calling process must also be rewritten. Applications may get confused otherwise. For example, consider the case where the working directory is "foo" and argv[0] is "bar/baz". In that case argv[0] as visible to the calling process must become "../bar/baz" or be made absolute. Making it absolute is inconsistent with what Bazel does right now. Attempting to keep it relative can be complex when symbolic links are involved. Furthermore, the specification doesn't mention what kind of path separators are used for argv[0]. The only reasonable solution here is to use path separators that are native to the host, as successive arguments also need to be provided in that form. --- .../remote/execution/v2/remote_execution.pb.go | 18 +++++++++++++++--- .../remote/execution/v2/remote_execution.proto | 18 +++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/build/bazel/remote/execution/v2/remote_execution.pb.go b/build/bazel/remote/execution/v2/remote_execution.pb.go index 64dbae81..1375a3e3 100755 --- a/build/bazel/remote/execution/v2/remote_execution.pb.go +++ b/build/bazel/remote/execution/v2/remote_execution.pb.go @@ -470,9 +470,21 @@ type Command struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - // The arguments to the command. The first argument must be the path to the - // executable, which must be either a relative path, in which case it is - // evaluated with respect to the input root, or an absolute path. + // The arguments to the command. + // + // The first argument specifies the command to run, which may be either an + // absolute path, a path relative to the working directory, or an unqualified + // path (without path separators) which will be resolved using the operating + // system's equivalent of the PATH environment variable. Path separators + // native to the operating system running on the worker SHOULD be used. If the + // `environment_variables` list contains an entry for the PATH environment + // variable, it SHOULD be respected. If not, the resolution process is + // implementation-defined. + // + // Changed in v2.3. v2.2 and older require that no PATH lookups are performed, + // and that relative paths are resolved relative to the input root. This + // behavior can, however, not be relied upon, as most implementations already + // followed the rules described above. Arguments []string `protobuf:"bytes,1,rep,name=arguments,proto3" json:"arguments,omitempty"` // The environment variables to set when running the program. The worker may // provide its own default environment variables; these defaults can be diff --git a/build/bazel/remote/execution/v2/remote_execution.proto b/build/bazel/remote/execution/v2/remote_execution.proto index 247d7741..86dbdd4f 100644 --- a/build/bazel/remote/execution/v2/remote_execution.proto +++ b/build/bazel/remote/execution/v2/remote_execution.proto @@ -538,9 +538,21 @@ message Command { string value = 2; } - // The arguments to the command. The first argument must be the path to the - // executable, which must be either a relative path, in which case it is - // evaluated with respect to the input root, or an absolute path. + // The arguments to the command. + // + // The first argument specifies the command to run, which may be either an + // absolute path, a path relative to the working directory, or an unqualified + // path (without path separators) which will be resolved using the operating + // system's equivalent of the PATH environment variable. Path separators + // native to the operating system running on the worker SHOULD be used. If the + // `environment_variables` list contains an entry for the PATH environment + // variable, it SHOULD be respected. If not, the resolution process is + // implementation-defined. + // + // Changed in v2.3. v2.2 and older require that no PATH lookups are performed, + // and that relative paths are resolved relative to the input root. This + // behavior can, however, not be relied upon, as most implementations already + // followed the rules described above. repeated string arguments = 1; // The environment variables to set when running the program. The worker may