fix: wait for BES completion event before reading lint results files …

…(#6578)

The last attempt at the fix failed. Still as the "interrupted" failure
on 5.11.0-rc0:
https://app.circleci.com/pipelines/github/aspect-build/bazel-lib/3779/workflows/3e23685c-5b9f-4dbf-8f55-bce4af9d0aca/jobs/23251

This should do the trick.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Fix bug where processing lint results was interrupted.

### Test plan

- Covered by existing test cases
- New test cases added
- Manual testing; please provide instructions so we can reproduce:

Cut a dev release and ship to bazel-lib where we have the repro

GitOrigin-RevId: 4ca32d1230daddb944de6ff5b42ee70962b061bb

pkg/aspect/lint/BUILD.bazel

@@ -23,7 +23,6 @@ go_library(
         "@com_github_spf13_cobra//:cobra",
         "@com_github_spf13_pflag//:pflag",
         "@com_github_spf13_viper//:viper",
-        "@org_golang_x_sync//errgroup",
     ],
 )
 

pkg/aspect/lint/bep.go

@@ -27,7 +27,6 @@ import (
 	"time"
 
 	"aspect.build/cli/bazel/buildeventstream"
-	"golang.org/x/sync/errgroup"
 )
 
 // ResultForLabel aggregates the relevant files we find in the BEP for
@@ -40,20 +39,20 @@ type ResultForLabel struct {
 }
 
 type LintBEPHandler struct {
-	ctx                   context.Context
-	namedSets             map[string]*buildeventstream.NamedSetOfFiles
-	workspaceRoot         string
-	handleResultsErrgroup *errgroup.Group
-	resultsByLabel        map[string]*ResultForLabel
+	ctx            context.Context
+	namedSets      map[string]*buildeventstream.NamedSetOfFiles
+	workspaceRoot  string
+	besCompleted   chan<- struct{}
+	resultsByLabel map[string]*ResultForLabel
 }
 
-func newLintBEPHandler(ctx context.Context, workspaceRoot string, handleResultsErrgroup *errgroup.Group) *LintBEPHandler {
+func newLintBEPHandler(ctx context.Context, workspaceRoot string, besCompleted chan<- struct{}) *LintBEPHandler {
 	return &LintBEPHandler{
-		ctx:                   ctx,
-		namedSets:             make(map[string]*buildeventstream.NamedSetOfFiles),
-		resultsByLabel:        make(map[string]*ResultForLabel),
-		workspaceRoot:         workspaceRoot,
-		handleResultsErrgroup: handleResultsErrgroup,
+		ctx:            ctx,
+		namedSets:      make(map[string]*buildeventstream.NamedSetOfFiles),
+		resultsByLabel: make(map[string]*ResultForLabel),
+		workspaceRoot:  workspaceRoot,
+		besCompleted:   besCompleted,
 	}
 }
 
@@ -167,6 +166,16 @@ func (runner *LintBEPHandler) bepEventCallback(event *buildeventstream.BuildEven
 				}
 			}
 		}
+
+	case *buildeventstream.BuildEvent_Finished:
+		// signal that the BES build finished event has been received and we're done processing lint
+		// result files from the BEP; we should only receive this event once but clear the channel
+		// out to be safe
+		if runner.besCompleted != nil {
+			runner.besCompleted <- struct{}{}
+			close(runner.besCompleted)
+			runner.besCompleted = nil
+		}
 	}
 
 	return nil

pkg/aspect/lint/lint.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	"aspect.build/cli/pkg/aspect/root/flags"
 	"aspect.build/cli/pkg/aspecterrors"
@@ -38,7 +39,6 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
-	"golang.org/x/sync/errgroup"
 )
 
 type LintResult struct {
@@ -139,7 +139,7 @@ func separateFlags(flags *pflag.FlagSet, args []string) ([]string, []string, []s
 }
 
 func (runner *Linter) Run(ctx context.Context, cmd *cobra.Command, args []string) error {
-	isInteractiveMode, err := cmd.Root().PersistentFlags().GetBool(flags.AspectInteractiveFlagName)
+	isInteractiveMode, _ := cmd.Root().PersistentFlags().GetBool(flags.AspectInteractiveFlagName)
 	linters := viper.GetStringSlice("lint.aspects")
 
 	if len(linters) == 0 {
@@ -209,7 +209,7 @@ lint:
 
 	bazelCmd = append(bazelCmd, fmt.Sprintf("%s='%s'", downloadFlag, LINT_RESULT_REGEX))
 
-	handleResultsErrgroup, handleResultsCtx := errgroup.WithContext(context.WithoutCancel(context.Background()))
+	besCompleted := make(chan struct{}, 1)
 
 	// Currently Bazel only supports a single --bes_backend so adding ours after
 	// any user supplied value will result in our bes_backend taking precedence.
@@ -233,7 +233,7 @@ lint:
 			return fmt.Errorf("failed to find workspace root: %w", err)
 		}
 
-		lintBEPHandler = newLintBEPHandler(handleResultsCtx, workspaceRoot, handleResultsErrgroup)
+		lintBEPHandler = newLintBEPHandler(ctx, workspaceRoot, besCompleted)
 		besBackend.RegisterSubscriber(lintBEPHandler.bepEventCallback)
 	}
 
@@ -247,10 +247,15 @@ lint:
 		return err
 	}
 
-	// Wait for completion and return the first error (if any)
-	wgErr := handleResultsErrgroup.Wait()
-	if wgErr != nil {
-		return wgErr
+	if lintBEPHandler == nil {
+		return fmt.Errorf("BES should always be initiated when running lint")
+	}
+
+	// Wait for BES completion event for some maximum amount fo time
+	select {
+	case <-besCompleted:
+	case <-time.After(60 * time.Millisecond):
+		return fmt.Errorf("timed out waiting for build completed event")
 	}
 
 	// Check for subscriber errors
@@ -262,10 +267,6 @@ lint:
 		return fmt.Errorf("%v BES subscriber error(s)", len(subscriberErrors))
 	}
 
-	if lintBEPHandler == nil {
-		return fmt.Errorf("BES should always be initiated when running lint")
-	}
-
 	// Convert raw results to list of LintResult structs
 	results := make([]*LintResult, 0, len(lintBEPHandler.resultsByLabel))
 	for _, r := range lintBEPHandler.resultsByLabel {