chore(copybara): sync commits from Aspect-internal silo

cmd/aspect/root/BUILD.bazel

@@ -35,12 +35,12 @@ go_library(
         "//cmd/aspect/sync",
         "//cmd/aspect/test",
         "//cmd/aspect/version",
-        "//docs/help/topics",
         "//pkg/aspect/root/flags",
         "//pkg/aspecterrors",
         "//pkg/bazel",
         "//pkg/ioutils",
         "//pkg/plugin/system",
+        "//docs/help/topics",
         "@com_github_fatih_color//:color",
         "@com_github_mattn_go_isatty//:go-isatty",
         "@com_github_spf13_cobra//:cobra",

cmd/aspect/root/root.go

@@ -1,19 +1,3 @@
-/*
- * Copyright 2023 Aspect Build Systems, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 /*
  * Copyright 2022 Aspect Build Systems, Inc.
  *
@@ -69,12 +53,12 @@ import (
 	"aspect.build/cli/cmd/aspect/sync"
 	"aspect.build/cli/cmd/aspect/test"
 	"aspect.build/cli/cmd/aspect/version"
-	help_docs "aspect.build/cli/docs/help/topics"
 	"aspect.build/cli/pkg/aspect/root/flags"
 	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/bazel"
 	"aspect.build/cli/pkg/ioutils"
 	"aspect.build/cli/pkg/plugin/system"
+	help_docs "aspect.build/cli/docs/help"
 )
 
 var (

docs/bazelrc/ci.bazelrc

@@ -1,10 +1,3 @@
-# We recommend enforcing a policy that keeps your CI from being slowed down
-# by individual test targets that should be optimized
-# or split up into multiple test targets with sharding or manually.
-# Set this flag to exclude targets that have their timeout set to eternal (>15m) from running on CI.
-# Docs: https://bazel.build/docs/user-manual#test-timeout-filters
-test --test_timeout_filters=-eternal
-
 # Set this flag to enable re-tries of failed tests on CI.
 # When any test target fails, try one or more times. This applies regardless of whether the "flaky"
 # tag appears on the target definition.

gazelle/bzl/gazelle.go

@@ -13,7 +13,7 @@ distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
-*/
+*/ 
 
 // Package bzl generates a `bzl_library` target for every `.bzl` file in
 // each package.

integration_tests/aspect/flags_test.bats

@@ -46,27 +46,27 @@ BAZELISK_BASE_URL=https://static.aspect.build/aspect
 USE_BAZEL_VERSION=aspect/1.2.3
 EOF
 
-    run aspect --version
+    run aspect
     assert_failure
     assert_output --partial "could not download Bazel"
 
-    run aspect --version --aspect:lock_version
+    run aspect --aspect:lock_version
     assert_success
-    assert_output --partial "Locking Aspect CLI version to"
+    assert_output --partial "Aspect CLI is a better frontend for running bazel"
 }
 
 @test 'lock_version flag should prevent downloading and running config version' {
     cat > version-config.yaml << 'EOF'
 version: 1.2.3
 EOF
 
-    run aspect --aspect:config="version-config.yaml" --version
+    run aspect --aspect:config="version-config.yaml"
     assert_failure
     assert_output --partial "could not download Bazel"
 
-    run aspect --aspect:config="version-config.yaml" --version --aspect:lock_version
+    run aspect --aspect:config="version-config.yaml" --aspect:lock_version
     assert_success
-    assert_output --partial "Locking Aspect CLI version to"
+    assert_output --partial "Aspect CLI is a better frontend for running bazel"
 }
 
 @test '--[no]able flags should work' {

pkg/aspect/configure/configure.go

@@ -23,7 +23,7 @@ import (
 	"os"
 	"strings"
 
-	bzl "aspect.build/cli/gazelle/bzl"
+	bzl "aspect.build/cli/gazelle/bzl" 
 	js "aspect.build/cli/gazelle/js"
 	kotlin "aspect.build/cli/gazelle/kotlin"
 	"aspect.build/cli/pkg/aspecterrors"

pkg/aspect/lint/BUILD.bazel

@@ -19,5 +19,6 @@ go_library(
         "@com_github_reviewdog_errorformat//writer",
         "@com_github_spf13_cobra//:cobra",
         "@com_github_spf13_viper//:viper",
+        "@org_golang_x_sync//errgroup",
     ],
 )

pkg/aspect/lint/lint.go

@@ -26,6 +26,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"aspect.build/cli/bazel/buildeventstream"
 	"aspect.build/cli/pkg/aspect/root/flags"
@@ -40,6 +41,7 @@ import (
 	"github.com/reviewdog/errorformat/writer"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	"golang.org/x/sync/errgroup"
 )
 
 type Linter struct {
@@ -49,12 +51,14 @@ type Linter struct {
 
 type LintBEPHandler struct {
 	ioutils.Streams
-	report        bool
-	fix           bool
-	diff          bool
-	output        string
-	reports       map[string]*buildeventstream.NamedSetOfFiles
-	workspaceRoot string
+	ctx                   context.Context
+	report                bool
+	fix                   bool
+	diff                  bool
+	output                string
+	reports               map[string]*buildeventstream.NamedSetOfFiles
+	workspaceRoot         string
+	handleResultsErrgroup *errgroup.Group
 }
 
 // Align with rules_lint
@@ -64,15 +68,17 @@ const (
 	LINT_RESULT_REGEX = ".*aspect_rules_lint.*"
 )
 
-func newLintBEPHandler(streams ioutils.Streams, printReport, applyFix, printDiff bool, output string, workspaceRoot string) *LintBEPHandler {
+func newLintBEPHandler(ctx context.Context, streams ioutils.Streams, printReport, applyFix, printDiff bool, output string, workspaceRoot string, handleResultsErrgroup *errgroup.Group) *LintBEPHandler {
 	return &LintBEPHandler{
-		Streams:       streams,
-		report:        printReport,
-		fix:           applyFix,
-		diff:          printDiff,
-		output:        output,
-		reports:       make(map[string]*buildeventstream.NamedSetOfFiles),
-		workspaceRoot: workspaceRoot,
+		Streams:               streams,
+		ctx:                   ctx,
+		report:                printReport,
+		fix:                   applyFix,
+		diff:                  printDiff,
+		output:                output,
+		reports:               make(map[string]*buildeventstream.NamedSetOfFiles),
+		workspaceRoot:         workspaceRoot,
+		handleResultsErrgroup: handleResultsErrgroup,
 	}
 }
 
@@ -121,6 +127,8 @@ lint:
 	// TODO: in Bazel 7 this was renamed without the --experimental_ prefix
 	bazelCmd = append(bazelCmd, fmt.Sprintf("--experimental_remote_download_regex='%s'", LINT_RESULT_REGEX))
 
+	handleResultsErrgroup, handleResultsCtx := errgroup.WithContext(context.Background())
+
 	// Currently Bazel only supports a single --bes_backend so adding ours after
 	// any user supplied value will result in our bes_backend taking precedence.
 	// There is a very old & stale issue to add support for multiple BES
@@ -142,12 +150,18 @@ lint:
 			return fmt.Errorf("failed to find workspace root: %w", err)
 		}
 
-		lintBEPHandler := newLintBEPHandler(runner.Streams, printReport, applyFix, printDiff, output, workspaceRoot)
+		lintBEPHandler := newLintBEPHandler(handleResultsCtx, runner.Streams, printReport, applyFix, printDiff, output, workspaceRoot, handleResultsErrgroup)
 		besBackend.RegisterSubscriber(lintBEPHandler.BEPEventCallback)
 	}
 
 	err := runner.bzl.RunCommand(runner.Streams, nil, bazelCmd...)
 
+	// Wait for completion and return the first error (if any)
+	wgErr := handleResultsErrgroup.Wait()
+	if wgErr != nil && err == nil {
+		err = wgErr
+	}
+
 	// Check for subscriber errors
 	subscriberErrors := bep.BESErrors(ctx)
 	if len(subscriberErrors) > 0 {
@@ -182,29 +196,30 @@ func (runner *LintBEPHandler) BEPEventCallback(event *buildeventstream.BuildEven
 		for _, outputGroup := range event.GetCompleted().OutputGroup {
 			for _, fileSetId := range outputGroup.FileSets {
 				if fileSet := runner.reports[fileSetId.Id]; fileSet != nil {
-					for _, f := range fileSet.GetFiles() {
+					runner.reports[fileSetId.Id] = nil
 
-						var err error
+					for _, file := range fileSet.GetFiles() {
+						l := label
+						f := file
 						if outputGroup.Name == LINT_PATCH_GROUP {
-							err = runner.patchLintResult(label, f, runner.fix, runner.diff)
+							runner.handleResultsErrgroup.Go(func() error {
+								return runner.patchLintResult(l, f, runner.fix, runner.diff)
+							})
 						} else if outputGroup.Name == LINT_REPORT_GROUP && runner.report {
 							switch runner.output {
 							case "text":
-								err = runner.outputLintResultText(label, f)
+								runner.handleResultsErrgroup.Go(func() error {
+									return runner.outputLintResultText(l, f)
+								})
 							case "sarif":
-								// FIXME: print one serif document, not one per report file
-								err = runner.outputLintResultSarif(label, f)
+								runner.handleResultsErrgroup.Go(func() error {
+									return runner.outputLintResultSarif(l, f)
+								})
 							default:
-								err = fmt.Errorf("Unsupported output kind %s", runner.output)
+								return fmt.Errorf("unsupported output kind %s", runner.output)
 							}
 						}
-
-						if err != nil {
-							return err
-						}
 					}
-
-					runner.reports[fileSetId.Id] = nil
 				}
 			}
 		}
@@ -350,6 +365,10 @@ func (runner *LintBEPHandler) readBEPFile(file *buildeventstream.File) (string,
 		if uri.Scheme == "file" {
 			resultsFile = filepath.Clean(uri.Path)
 		} else if uri.Scheme == "bytestream" {
+			if strings.HasSuffix(uri.Path, "/0") {
+				// No reason to read an empty results file from disk
+				return "", nil
+			}
 			// Because we set --experimental_remote_download_regex, we can depend on the results file being
 			// in the output tree even when using a remote cache with build without the bytes.
 			resultsFile = path.Join(runner.workspaceRoot, path.Join(file.PathPrefix...), file.Name)
@@ -360,10 +379,33 @@ func (runner *LintBEPHandler) readBEPFile(file *buildeventstream.File) (string,
 		return "", fmt.Errorf("unsupported BES file type")
 	}
 
-	buf, err := os.ReadFile(resultsFile)
-	if err != nil {
-		return "", fmt.Errorf("failed to read lint results file %s: %v", resultsFile, err)
+	start := time.Now()
+	for {
+		// TODO: also check that the bazel remote cache downloader is not still writing
+		// to the results file
+		_, err := os.Stat(resultsFile)
+		if err != nil {
+			// if more than 60s has passed then give up
+			// TODO: make this timeout configurable
+			if time.Since(start) > 60*time.Second {
+				return "", fmt.Errorf("failed to read lint results file %s: %v", resultsFile, err)
+			}
+		} else {
+			buf, err := os.ReadFile(resultsFile)
+			if err != nil {
+				return "", fmt.Errorf("failed to read lint results file %s: %v", resultsFile, err)
+			}
+			return string(buf), nil
+		}
+		// we're in a go routine so yield for 100ms and try again
+		// TODO: watch the file system for the file creation instead of polling
+		t := time.NewTimer(time.Millisecond * 100)
+		select {
+		case <-runner.ctx.Done():
+			t.Stop()
+			return "", fmt.Errorf("failed to read lint results file %s: interrupted", resultsFile)
+		case <-t.C:
+		}
 	}
 
-	return string(buf), nil
 }

pkg/aspect/outputs/hash.go

@@ -27,8 +27,8 @@ import (
 	"sort"
 	"strings"
 
-	"aspect.build/cli/pkg/bazel"
 	"github.com/alphadose/haxmap"
+	"aspect.build/cli/pkg/bazel"
 	"github.com/rogpeppe/go-internal/dirhash"
 	concurrently "github.com/tejzpr/ordered-concurrently/v3"
 	"github.com/twmb/murmur3"

pkg/bazel/bazel.go

@@ -29,7 +29,6 @@ import (
 
 	"aspect.build/cli/bazel/analysis"
 	"aspect.build/cli/bazel/flags"
-	"aspect.build/cli/buildinfo"
 	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/bazel/workspace"
 	"aspect.build/cli/pkg/ioutils"
@@ -142,14 +141,10 @@ func (b *bazel) HandleReenteringAspect(streams ioutils.Streams, args []string, a
 		return false, err
 	}
 
-	if bazelisk.AspectShouldReenter {
-		if !aspectLockVersion {
-			repos := createRepositories()
-			err := bazelisk.Run(args, repos, streams, b.env, nil)
-			return true, err
-		} else {
-			fmt.Fprintf(streams.Stderr, "Locking Aspect CLI version to %s %s which differs from the version configured in .bazeliskrc or the Aspect CLI config file\n", buildinfo.Current().GnuName(), buildinfo.Current().Version())
-		}
+	if bazelisk.AspectShouldReenter && !aspectLockVersion {
+		repos := createRepositories()
+		err := bazelisk.Run(args, repos, streams, b.env, nil)
+		return true, err
 	}
 
 	return false, nil

pkg/bazel/bazelisk.go

@@ -165,8 +165,11 @@ func (bazelisk *Bazelisk) Run(args []string, repos *core.Repositories, streams i
 		return fmt.Errorf("could not run Bazel: %v", err)
 	}
 	if exitCode != 0 {
+		// Just bubble up the exit code so the Aspect CLI exits with the same code; don't specify any error
+		// message since Bazel should have already printed the error to stderr if appropriate and we don't
+		// want to print any additional error messages to stderr.
 		return &aspecterrors.ExitError{
-			Err:      fmt.Errorf("bazel exited with exit code: %v", exitCode),
+			Err:      nil,
 			ExitCode: exitCode,
 		}
 	}