diff --git a/src/core/build_target.go b/src/core/build_target.go index f8aa20b131..5f3a4c80a7 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -10,7 +10,9 @@ import ( "sync/atomic" "time" + pb "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "golang.org/x/sync/errgroup" + "google.golang.org/protobuf/proto" "github.com/thought-machine/please/src/fs" ) @@ -262,6 +264,11 @@ type BuildMetadata struct { Cached bool } +func (m *BuildMetadata) SetRemoteOutputs(tree *pb.Tree) { + data, _ := proto.Marshal(tree) + m.RemoteOutputs = data +} + // A PreBuildFunction is a type that allows hooking a pre-build callback. type PreBuildFunction interface { fmt.Stringer diff --git a/src/remote/action.go b/src/remote/action.go index 7ffd0813b2..f80e8c5b48 100644 --- a/src/remote/action.go +++ b/src/remote/action.go @@ -485,7 +485,8 @@ func (c *Client) uploadLocalTarget(target *core.BuildTarget) error { if err := c.uploadIfMissing(context.Background(), entries); err != nil { return err } - return c.setOutputs(target, ar) + _, err = c.setOutputs(target, ar) + return err } // translateOS converts the OS name of a subrepo into a Bazel-style OS name. diff --git a/src/remote/remote.go b/src/remote/remote.go index 6f92d078c2..7d916c2d0d 100644 --- a/src/remote/remote.go +++ b/src/remote/remote.go @@ -322,9 +322,11 @@ func (c *Client) Build(target *core.BuildTarget) (*core.BuildMetadata, error) { hash, _ := hex.DecodeString(c.outputHash(ar)) c.state.TargetHasher.SetHash(target, hash) } - if err := c.setOutputs(target, ar); err != nil { + tree, err := c.setOutputs(target, ar) + if err != nil { return metadata, c.wrapActionErr(err, digest) } + metadata.SetRemoteOutputs(tree) if c.state.ShouldDownload(target) { c.state.LogBuildResult(target, core.TargetBuilding, "Downloading") diff --git a/src/remote/utils.go b/src/remote/utils.go index c3414aef33..d08bdbef7f 100644 --- a/src/remote/utils.go +++ b/src/remote/utils.go @@ -36,12 +36,6 @@ var downloadErrors = metrics.NewCounter( "Number of times the an error has been seen during a tree digest download", ) -var directoriesStored = metrics.NewCounter( - "remote", - "dirs_stored_total", - "Number of directories cached locally", -) - var directoriesRetrieved = metrics.NewCounter( "remote", "dirs_retrieved_total", @@ -73,11 +67,13 @@ func (c *Client) targetOutputs(label core.BuildLabel) *pb.Directory { } // setOutputs sets the outputs for a previously executed target. -func (c *Client) setOutputs(target *core.BuildTarget, ar *pb.ActionResult) error { - o := &pb.Directory{ - Files: make([]*pb.FileNode, len(ar.OutputFiles)), - Directories: make([]*pb.DirectoryNode, 0, len(ar.OutputDirectories)), - Symlinks: make([]*pb.SymlinkNode, len(ar.OutputFileSymlinks)+len(ar.OutputDirectorySymlinks)), +func (c *Client) setOutputs(target *core.BuildTarget, ar *pb.ActionResult) (*pb.Tree, error) { + o := &pb.Tree{ + Root: &pb.Directory{ + Files: make([]*pb.FileNode, len(ar.OutputFiles)), + Directories: make([]*pb.DirectoryNode, 0, len(ar.OutputDirectories)), + Symlinks: make([]*pb.SymlinkNode, len(ar.OutputFileSymlinks)+len(ar.OutputDirectorySymlinks)), + }, } // N.B. At this point the various things we stick into this Directory proto can be in // subdirectories. This is not how a Directory proto is meant to work but it makes things @@ -85,7 +81,7 @@ func (c *Client) setOutputs(target *core.BuildTarget, ar *pb.ActionResult) error // without downloading their respective Directory protos). Later on we sort this out in // uploadInputDir. for i, f := range ar.OutputFiles { - o.Files[i] = &pb.FileNode{ + o.Root.Files[i] = &pb.FileNode{ Name: f.Path, Digest: f.Digest, IsExecutable: f.IsExecutable, @@ -95,33 +91,35 @@ func (c *Client) setOutputs(target *core.BuildTarget, ar *pb.ActionResult) error tree := &pb.Tree{} if _, err := c.client.ReadProto(context.Background(), digest.NewFromProtoUnvalidated(d.TreeDigest), tree); err != nil { downloadErrors.Inc() - return wrap(err, "Downloading tree digest for %s [%s]", d.Path, d.TreeDigest.Hash) + return nil, wrap(err, "Downloading tree digest for %s [%s]", d.Path, d.TreeDigest.Hash) } + o.Children = append(o.Children, tree.Children...) + if outDir := maybeGetOutDir(d.Path, target.OutputDirectories); outDir != "" { files, dirs, err := c.getOutputsForOutDir(target, outDir, tree) if err != nil { - return err + return nil, err } - o.Directories = append(o.Directories, dirs...) - o.Files = append(o.Files, files...) + o.Root.Directories = append(o.Root.Directories, dirs...) + o.Root.Files = append(o.Root.Files, files...) } else { - o.Directories = append(o.Directories, &pb.DirectoryNode{ + o.Root.Directories = append(o.Root.Directories, &pb.DirectoryNode{ Name: d.Path, Digest: c.digestMessage(tree.Root), }) } } for i, s := range append(ar.OutputFileSymlinks, ar.OutputDirectorySymlinks...) { - o.Symlinks[i] = &pb.SymlinkNode{ + o.Root.Symlinks[i] = &pb.SymlinkNode{ Name: s.Path, Target: s.Target, } } c.outputMutex.Lock() defer c.outputMutex.Unlock() - c.outputs[target.Label] = o - return nil + c.outputs[target.Label] = o.Root + return o, nil } func (c *Client) getOutputsForOutDir(target *core.BuildTarget, outDir core.OutputDirectory, tree *pb.Tree) ([]*pb.FileNode, []*pb.DirectoryNode, error) { @@ -219,23 +217,6 @@ func (c *Client) locallyCacheResults(target *core.BuildTarget, dg *pb.Digest, me metadata.RemoteAction = data metadata.Timestamp = time.Now() - if len(ar.OutputDirectories) > 0 { - tree := pb.Tree{} - for _, d := range ar.OutputDirectories { - t := pb.Tree{} - if _, err := c.client.ReadProto(context.Background(), digest.NewFromProtoUnvalidated(d.TreeDigest), &t); err == nil { - tree.Children = append(tree.Children, t.Root) - tree.Children = append(tree.Children, t.Children...) - } - } - for _, dir := range tree.Children { - c.directories.Store(c.digestMessage(dir).Hash, dir) - } - directoriesStored.Add(float64(len(tree.Children))) - data, _ := proto.Marshal(&tree) - metadata.RemoteOutputs = data - } - if err := c.mdStore.storeMetadata(dg.Hash, metadata); err != nil { log.Warningf("Failed to store build metadata for target %s: %v", target.Label, err) }