diff --git a/lib/bazel/BUILD.bazel b/lib/bazel/BUILD.bazel index 842fd716e..d0ec1cc55 100644 --- a/lib/bazel/BUILD.bazel +++ b/lib/bazel/BUILD.bazel @@ -11,7 +11,8 @@ go_library( importpath = "github.com/enfabrica/enkit/lib/bazel", visibility = ["//visibility:public"], deps = [ - "//lib/bazel/proto:build_go_proto", + "//lib/bazel/proto:go_protos", + "//lib/multierror:go_default_library", "//lib/proto/delimited:go_default_library", "@com_github_bazelbuild_buildtools//wspace:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", @@ -21,6 +22,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "options_test.go", "query_test.go", "workspace_test.go", ], diff --git a/lib/bazel/affected_targets.go b/lib/bazel/affected_targets.go index bd4dc3a75..cc9c4f2f0 100644 --- a/lib/bazel/affected_targets.go +++ b/lib/bazel/affected_targets.go @@ -53,19 +53,29 @@ func GetAffectedTargets(start string, end string) ([]string, error) { return nil, fmt.Errorf("failed to open bazel workspace: %w", err) } + workspaceLogStart, err := WithTempWorkspaceRulesLog() + if err != nil { + return nil, fmt.Errorf("start workspace: %w", err) + } + workspaceLogEnd, err := WithTempWorkspaceRulesLog() + if err != nil { + return nil, fmt.Errorf("end workspace: %w", err) + } + // Get all target info for both VCS time points. - targets, err := startWorkspace.Query("deps(//...)", WithKeepGoing(), WithUnorderedOutput()) + startResults, err := startWorkspace.Query("deps(//...)", WithKeepGoing(), WithUnorderedOutput(), workspaceLogStart) if err != nil { return nil, fmt.Errorf("failed to query deps for start point: %w", err) } - targets, err = endWorkspace.Query("deps(//...)", WithKeepGoing(), WithUnorderedOutput()) + endResults, err := endWorkspace.Query("deps(//...)", WithKeepGoing(), WithUnorderedOutput(), workspaceLogEnd) if err != nil { return nil, fmt.Errorf("failed to query deps for end point: %w", err) } // TODO(scott): Implement diffing of returned targets - targets = targets + startResults = startResults + endResults = endResults return nil, fmt.Errorf("not implemented") } diff --git a/lib/bazel/commands/commands.go b/lib/bazel/commands/commands.go index 79b7aaffa..203c091fe 100644 --- a/lib/bazel/commands/commands.go +++ b/lib/bazel/commands/commands.go @@ -114,7 +114,7 @@ func (c *AffectedTargetsList) Run(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("can't generate worktree for committish %q: %w", c.parent.End, err) } - defer endTree.Close() + //defer endTree.Close() endTreePath = endTree.Root() } endWS := filepath.Clean(filepath.Join(endTreePath, gitToBazelPath)) diff --git a/lib/bazel/options.go b/lib/bazel/options.go index b8ae79fdb..f939a431e 100644 --- a/lib/bazel/options.go +++ b/lib/bazel/options.go @@ -1,7 +1,13 @@ package bazel import ( + "errors" + "fmt" + "io/ioutil" + "os" "os/exec" + + "github.com/enfabrica/enkit/lib/multierror" ) // subcommand is implemented by an arguments struct for each bazel subcommand @@ -51,6 +57,7 @@ type queryOptions struct { keepGoing bool unorderedOutput bool + workspaceLog *os.File } // Args returns the `query` and relevant subcommand arguments as passed to bazel. @@ -62,6 +69,11 @@ func (o *queryOptions) Args() []string { if o.unorderedOutput { f = append(f, "--order_output=no") } + if o.workspaceLog != nil { + // See https://github.com/bazelbuild/bazel/issues/6807 for tracking issue + // making this flag non-experimental + f = append(f, "--experimental_workspace_rules_log_file", o.workspaceLog.Name()) + } f = append(f, "--", o.query) return f } @@ -73,10 +85,11 @@ func (o *queryOptions) filterError(err error) error { return nil } - if err, ok := err.(*exec.ExitError); ok { + var execErr *exec.ExitError + if errors.As(err, &execErr) { // PARTIAL_ANALYSIS_FAILURE is expected when --keep_going is passed // https://github.com/bazelbuild/bazel/blob/86409b7a248d1cb966268451f9aa4db0763c3eb2/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L38 - if err.ExitCode() == 3 { + if execErr.ExitCode() == 3 { return nil } } @@ -84,6 +97,15 @@ func (o *queryOptions) filterError(err error) error { return err } +func (o *queryOptions) Close() error { + var errs []error + if o.workspaceLog != nil { + errs = append(errs, o.workspaceLog.Close()) + errs = append(errs, os.RemoveAll(o.workspaceLog.Name())) + } + return multierror.New(errs) +} + // QueryOption modifies bazel query subcommand flags. type QueryOption func(*queryOptions) @@ -103,6 +125,17 @@ func WithUnorderedOutput() QueryOption { } } +func WithTempWorkspaceRulesLog() (QueryOption, error) { + f, err := ioutil.TempFile("", "bazel_workspace_log_*.pb") + if err != nil { + return nil, fmt.Errorf("failed to create temp workspace log file: %w", err) + } + + return func(o *queryOptions) { + o.workspaceLog = f + }, nil +} + // apply applies all the options to this option struct. func (opts QueryOptions) apply(o *queryOptions) { for _, opt := range opts { diff --git a/lib/bazel/options_test.go b/lib/bazel/options_test.go new file mode 100644 index 000000000..402685514 --- /dev/null +++ b/lib/bazel/options_test.go @@ -0,0 +1,26 @@ +package bazel + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestQueryOptionsArgs(t *testing.T) { + tempLog, err := WithTempWorkspaceRulesLog() + assert.NoError(t, err) + opts := QueryOptions{ + WithKeepGoing(), + WithUnorderedOutput(), + tempLog, + } + opt := &queryOptions{} + opts.apply(opt) + got := opt.Args() + + assert.Contains(t, got, "query") + assert.Contains(t, got, "--output=streamed_proto") + assert.Contains(t, got, "--order_output=no") + assert.Contains(t, got, "--keep_going") + assert.Contains(t, got, "--experimental_workspace_rules_log_file") +} \ No newline at end of file diff --git a/lib/bazel/proto/BUILD.bazel b/lib/bazel/proto/BUILD.bazel index 4d0cf020c..476d6bdb2 100644 --- a/lib/bazel/proto/BUILD.bazel +++ b/lib/bazel/proto/BUILD.bazel @@ -35,3 +35,13 @@ go_proto_library( proto = ":workspace_log_proto", visibility = ["//lib/bazel:__subpackages__"], ) + +go_library( + name = "go_protos", + embed = [ + ":build_go_proto", + ":workspace_log_go_proto", + ], + importpath = "github.com/enfabrica/enkit/lib/bazel/proto", + visibility = ["//lib/bazel:__subpackages__"], +) diff --git a/lib/bazel/proto/workspace_log.proto b/lib/bazel/proto/workspace_log.proto index 6d7d00367..84dd6c492 100644 --- a/lib/bazel/proto/workspace_log.proto +++ b/lib/bazel/proto/workspace_log.proto @@ -18,7 +18,7 @@ // limitations under the License. syntax = "proto3"; -package lib.bazel.proto; +package blaze_query; // Information on "Execute" event in repository_ctx. message ExecuteEvent { diff --git a/lib/bazel/query.go b/lib/bazel/query.go index 8cb4268ae..b6c6921b9 100644 --- a/lib/bazel/query.go +++ b/lib/bazel/query.go @@ -3,6 +3,7 @@ package bazel import ( "fmt" "io" + "strings" "os/exec" bpb "github.com/enfabrica/enkit/lib/bazel/proto" @@ -40,19 +41,31 @@ var streamedBazelCommand = func(cmd *exec.Cmd) (io.Reader, chan error, error) { } err = cmd.Wait() if err != nil { - errChan <- err // Don't wrap, so raw ExitError can be picked up by the caller + errChan <- fmt.Errorf("command failed: `%s`: %w", strings.Join(cmd.Args, " "), err) } }() return pipeReader, errChan, nil } +// QueryResult contains the results of an arbitrary bazel query. +type QueryResult struct { + // Targets is filled with a map of "target label" to target node. + Targets map[string]*bpb.Target + + // If the WithTempWorkspaceRulesLog() option is passed, this contains a list + // of all the workspace events emitted during the bazel query. Otherwise, this + // is empty. + WorkspaceEvents []*bpb.WorkspaceEvent +} + // Query performs a `bazel query` using the provided query string. If // `keep_going` is set, then `--keep_going` is set on the bazel commandline, and // errors from the bazel process are ignored. -func (w *Workspace) Query(query string, options ...QueryOption) (map[string]*bpb.Target, error) { +func (w *Workspace) Query(query string, options ...QueryOption) (*QueryResult, error) { queryOpts := &queryOptions{query: query} QueryOptions(options).apply(queryOpts) + defer queryOpts.Close() cmd := w.bazelCommand(queryOpts) resultStream, errChan, err := streamedBazelCommand(cmd) @@ -79,7 +92,23 @@ func (w *Workspace) Query(query string, options ...QueryOption) (map[string]*bpb return nil, err } - return targets, nil + var workspaceEvents []*bpb.WorkspaceEvent + if queryOpts.workspaceLog != nil { + rdr := delimited.NewReader(queryOpts.workspaceLog) + var buf []byte + for buf, err = rdr.Next(); err == nil; buf, err = rdr.Next() { + var event bpb.WorkspaceEvent + if err := proto.Unmarshal(buf, &event); err != nil { + return nil, fmt.Errorf("failed to unmarshal WorkspaceEvent message: %w", err) + } + workspaceEvents = append(workspaceEvents, &event) + } + } + + return &QueryResult{ + Targets: targets, + WorkspaceEvents: workspaceEvents, + }, nil } // targetName returns the name of a Target message, which is part of a diff --git a/lib/bazel/query_test.go b/lib/bazel/query_test.go index 0fef95532..2962347a7 100644 --- a/lib/bazel/query_test.go +++ b/lib/bazel/query_test.go @@ -62,7 +62,7 @@ func TestQueryOutput(t *testing.T) { return } - assert.Equal(t, tc.wantCount, len(got)) + assert.Equal(t, tc.wantCount, len(got.Targets)) }) } } diff --git a/lib/git/worktree.go b/lib/git/worktree.go index 7697a1016..3537328be 100644 --- a/lib/git/worktree.go +++ b/lib/git/worktree.go @@ -31,11 +31,12 @@ func NewTempWorktree(repoPath string, committish string) (*TempWorktree, error) if err != nil { return nil, fmt.Errorf("failed to create temp directory: %w", err) } - cmd := exec.Command("git", "worktree", "add", tmpDir, committish) + // Command info: https://git-scm.com/docs/git-worktree + cmd := exec.Command("git", "worktree", "add", "--detach", tmpDir, committish) cmd.Dir = repoPath _, err = runCommand(cmd) if err != nil { - return nil, fmt.Errorf("failed to construct temp worktree: %w", err) + return nil, fmt.Errorf("failed to construct temp worktree with command %v: %w", cmd, err) } return &TempWorktree{ repoPath: repoPath, diff --git a/lib/git/worktree_test.go b/lib/git/worktree_test.go index b677f685c..b64b23207 100644 --- a/lib/git/worktree_test.go +++ b/lib/git/worktree_test.go @@ -22,8 +22,8 @@ func TestNewTempWorktree(t *testing.T) { assert.NoError(t, gotErr) assert.NoError(t, closeErr) assert.Equal(t, 2, len(gotCmds)) - assert.Equal(t, []string{"git", "worktree", "add"}, gotCmds[0].Args[0:3]) - assert.Equal(t, []string{"some_branch_name"}, gotCmds[0].Args[4:]) + assert.Equal(t, []string{"git", "worktree", "add", "--detach"}, gotCmds[0].Args[0:4]) + assert.Equal(t, []string{"some_branch_name"}, gotCmds[0].Args[5:]) assert.Equal(t, "/foo/bar", gotCmds[0].Dir) assert.Equal(t, []string{"git", "worktree", "remove"}, gotCmds[1].Args[0:3]) assert.Equal(t, "/foo/bar", gotCmds[1].Dir) diff --git a/machinist/testing/BUILD.bazel b/machinist/testing/BUILD.bazel index fdc515dc6..3e486a1d4 100644 --- a/machinist/testing/BUILD.bazel +++ b/machinist/testing/BUILD.bazel @@ -2,9 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( name = "go_default_test", - srcs = [ - "machinist_e2e_test.go", - ], + srcs = ["machinist_e2e_test.go"], deps = [ "//lib/knetwork:go_default_library", "//lib/knetwork/kdns:go_default_library", @@ -13,7 +11,6 @@ go_test( "//machinist/machine:go_default_library", "//machinist/mserver:go_default_library", "//machinist/state:go_default_library", - "@com_github_miekg_dns//:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_grpc//test/bufconn:go_default_library",