Skip to content

Commit

Permalink
Save the output tree to the build metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
Tatskaari committed Nov 15, 2023
1 parent a652205 commit 2411115
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 39 deletions.
7 changes: 7 additions & 0 deletions src/core/build_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/remote/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion src/remote/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
55 changes: 18 additions & 37 deletions src/remote/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -73,19 +67,21 @@ 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
// a lot easier for us to handle (since it is impossible to merge two DirectoryNode protos
// 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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 2411115

Please sign in to comment.