diff --git a/ChangeLog b/ChangeLog index 904a552040..d38dc5b556 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Version 14.1.3 +-------------- + + * More small fixes to remote execution code. + + Version 14.1.2 -------------- diff --git a/VERSION b/VERSION index 900dba2dda..e84178ff7c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -14.1.2 +14.1.3 diff --git a/bootstrap.sh b/bootstrap.sh index be5515f619..af3ea3ce12 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -45,11 +45,6 @@ if [ "$GOOS" != "linux" ] ; then warn "cc_module tests disabled due to not being on Linux" EXCLUDES="${EXCLUDES} --exclude=cc_module" fi -# TODO(peterebden): Once Go 1.13 is out we will need to update (or remove) this. -if [ ! "`go version | grep 1.12`" ]; then - warn "Go version != 1.12 detected, excluding assembly tests" - EXCLUDES="${EXCLUDES} --exclude=asm --exclude=//test/go_rules/asm_binary/... --exclude=//test/go_rules/asm/..." -fi if ! hash python2 2>/dev/null ; then warn "python2 not found, excluding python2 tests" EXCLUDES="${EXCLUDES} --exclude=py2" diff --git a/src/core/build_env.go b/src/core/build_env.go index 0d984612f3..795d9da38d 100644 --- a/src/core/build_env.go +++ b/src/core/build_env.go @@ -62,7 +62,7 @@ func BuildEnvironment(state *BuildState, target *BuildTarget, tmpDir string) Bui env := buildEnvironment(state, target) sources := target.AllSourcePaths(state.Graph) outEnv := target.GetTmpOutputAll(target.Outputs()) - abs := tmpDir != "" + abs := path.IsAbs(tmpDir) env = append(env, "TMP_DIR="+tmpDir, @@ -187,11 +187,12 @@ func initStampEnv() { } func toolPath(state *BuildState, tool BuildInput, abs bool) string { - label := tool.Label() - if label != nil { + if label := tool.Label(); label != nil { return state.Graph.TargetOrDie(*label).toolPath(abs) + } else if abs { + return tool.Paths(state.Graph)[0] } - return tool.Paths(state.Graph)[0] + return tool.LocalPaths(state.Graph)[0] } func toolPaths(state *BuildState, tools []BuildInput, abs bool) []string { diff --git a/src/core/build_input.go b/src/core/build_input.go index 81bb17d9a8..7c0c433f65 100644 --- a/src/core/build_input.go +++ b/src/core/build_input.go @@ -174,7 +174,7 @@ func (label SystemPathLabel) FullPaths(graph *BuildGraph) []string { // LocalPaths returns paths within the local package func (label SystemPathLabel) LocalPaths(graph *BuildGraph) []string { - return label.FullPaths(graph) + return []string{label.Name} } // Label returns the build rule associated with this input. For a SystemPathLabel it's always nil. diff --git a/src/remote/action.go b/src/remote/action.go index 0c6439b49d..831ddbb254 100644 --- a/src/remote/action.go +++ b/src/remote/action.go @@ -47,6 +47,9 @@ func (c *Client) buildCommand(target *core.BuildTarget, stamp []byte, isTest boo if isTest { return c.buildTestCommand(target) } + // We can't predict what variables like this should be so we sneakily bung something on + // the front of the command. It'd be nicer if there were a better way though... + const commandPrefix = "export TMP_DIR=\"`pwd`\" && " files, dirs := outputs(target) return &pb.Command{ Platform: &pb.Platform{ @@ -65,9 +68,9 @@ func (c *Client) buildCommand(target *core.BuildTarget, stamp []byte, isTest boo // remote one (which is probably OK on the same OS, but not between say Linux and // FreeBSD where bash is not idiomatically in the same place). Arguments: []string{ - c.bashPath, "--noprofile", "--norc", "-u", "-o", "pipefail", "-c", c.getCommand(target), + c.bashPath, "--noprofile", "--norc", "-u", "-o", "pipefail", "-c", commandPrefix + c.getCommand(target), }, - EnvironmentVariables: buildEnv(core.StampedBuildEnvironment(c.state, target, stamp, "")), + EnvironmentVariables: buildEnv(core.StampedBuildEnvironment(c.state, target, stamp, ".")), OutputFiles: files, OutputDirectories: dirs, } @@ -85,6 +88,7 @@ func (c *Client) buildTestCommand(target *core.BuildTarget) *pb.Command { } else { files = append(files, core.TestResultsFile) } + const commandPrefix = "export TMP_DIR=\"`pwd`\" TEST_DIR=\"`pwd`\" && " return &pb.Command{ Platform: &pb.Platform{ Properties: []*pb.Platform_Property{ @@ -95,7 +99,7 @@ func (c *Client) buildTestCommand(target *core.BuildTarget) *pb.Command { }, }, Arguments: []string{ - c.bashPath, "--noprofile", "--norc", "-u", "-o", "pipefail", "-c", target.GetTestCommand(c.state), + c.bashPath, "--noprofile", "--norc", "-u", "-o", "pipefail", "-c", commandPrefix + target.GetTestCommand(c.state), }, EnvironmentVariables: buildEnv(core.TestEnvironment(c.state, target, "")), OutputFiles: files, @@ -297,14 +301,13 @@ func (c *Client) buildInputRoot(target *core.BuildTarget, upload, isTest bool) ( // buildMetadata converts an ActionResult into one of our BuildMetadata protos. func (c *Client) buildMetadata(ar *pb.ActionResult, needStdout, needStderr bool) (*core.BuildMetadata, error) { - if ar.ExecutionMetadata == nil { - return nil, fmt.Errorf("Build server returned no execution metadata for target; remote build failed or did not run") - } metadata := &core.BuildMetadata{ - StartTime: toTime(ar.ExecutionMetadata.ExecutionStartTimestamp), - EndTime: toTime(ar.ExecutionMetadata.ExecutionCompletedTimestamp), - Stdout: ar.StdoutRaw, - Stderr: ar.StderrRaw, + Stdout: ar.StdoutRaw, + Stderr: ar.StderrRaw, + } + if ar.ExecutionMetadata != nil { + metadata.StartTime = toTime(ar.ExecutionMetadata.ExecutionStartTimestamp) + metadata.EndTime = toTime(ar.ExecutionMetadata.ExecutionCompletedTimestamp) } if needStdout && len(metadata.Stdout) == 0 { if ar.StdoutDigest == nil { diff --git a/src/remote/remote.go b/src/remote/remote.go index 1dcd806524..7c15963b92 100644 --- a/src/remote/remote.go +++ b/src/remote/remote.go @@ -451,12 +451,11 @@ func (c *Client) execute(tid int, target *core.BuildTarget, digest *pb.Digest, t log.Debug("Bad result from build server: %+v", response) return nil, nil, fmt.Errorf("Build server did not return valid result") } - // TODO(henryaj): if messages are only emitted on error, we should upgrade this to a warning if response.Message != "" { + // Informational messages can be emitted on successful actions. log.Debug("Message from build server:\n %s", response.Message) } - // TODO(henryaj): are there cases where a non-zero exit code isn't a failed build? - if response.Result.ExitCode > 0 { + if response.Result.ExitCode != 0 { return nil, nil, fmt.Errorf("Remotely executed command exited with %d", response.Result.ExitCode) } metadata, err := c.buildMetadata(response.Result, needStdout || respErr != nil, respErr != nil) @@ -473,6 +472,9 @@ func (c *Client) execute(tid int, target *core.BuildTarget, digest *pb.Digest, t // updateProgress updates the progress of a target based on its metadata. func (c *Client) updateProgress(tid int, target *core.BuildTarget, metadata *pb.ExecuteOperationMetadata) { + if c.state.Config.Remote.DisplayURL != "" { + log.Debug("Remote progress for %s: %s (action: %s/action/%s/%s/%d)", target.Label, metadata.Stage, c.state.Config.Remote.DisplayURL, c.state.Config.Remote.Instance, metadata.ActionDigest.Hash, metadata.ActionDigest.SizeBytes) + } if target.State() >= core.Built { switch metadata.Stage { case pb.ExecutionStage_CACHE_CHECK: diff --git a/src/remote/remote_test.go b/src/remote/remote_test.go index 87cae183ed..5166a677f1 100644 --- a/src/remote/remote_test.go +++ b/src/remote/remote_test.go @@ -217,6 +217,20 @@ func TestNoAbsolutePaths(t *testing.T) { } } +func TestNoAbsolutePaths2(t *testing.T) { + c := newClientInstance("test") + tool := core.NewBuildTarget(core.BuildLabel{PackageName: "package", Name: "tool"}) + tool.AddOutput("bin") + c.state.Graph.AddTarget(tool) + target := core.NewBuildTarget(core.BuildLabel{PackageName: "package", Name: "target5"}) + target.AddOutput("remote_test") + target.AddTool(core.SystemPathLabel{Path: []string{os.Getenv("TMP_DIR")}, Name: "remote_test"}) + cmd := c.buildCommand(target, []byte("hello"), false) + for _, env := range cmd.EnvironmentVariables { + assert.False(t, path.IsAbs(env.Value), "Env var %s has an absolute path: %s", env.Name, env.Value) + } +} + func newClient() *Client { return newClientInstance("") } diff --git a/src/remote/utils.go b/src/remote/utils.go index a8e3eddbad..60375b2842 100644 --- a/src/remote/utils.go +++ b/src/remote/utils.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "fmt" "os" + "path" "strings" "time" @@ -147,7 +148,7 @@ func outputs(target *core.BuildTarget) (files, dirs []string) { outs := target.Outputs() files = make([]string, 0, len(outs)) for _, out := range outs { - if !strings.ContainsRune(out, '.') && !strings.HasSuffix(out, "file") { + if !strings.ContainsRune(path.Base(out), '.') && !strings.HasSuffix(out, "file") { dirs = append(dirs, out) } else { files = append(files, out)