go/tools/gopackagesdriver: add automatic target detection (#2932)

* go/tools/gopackagesdriver: add automatic target detection

This commit introduces automatic target detection so that no user input
is required after the `GOPACKAGESDRIVER` setup. This effectively
deprecates the following environment variables:
- `GOPACKAGESDRIVER_BAZEL_TARGETS`
- `GOPACKAGESDRIVER_BAZEL_QUERY`
- `GOPACKAGESDRIVER_BAZEL_TAG_FILTERS`

It works as follows:
- for `importpath` queries, it will `bazel query` a matching `go_library`
  matching it
- for `file=` queries, it will try to find the matching `go_library` in the
  same package
- since it supports multiple queries at the same time, it will `union` those
  queries and query that

Once the `go_library` targets are found, it will try to build them directly,
which should dramatically speed up compilation times, at the loss of transition
support (which wasn't used as much as I thought it would be).

I may reintroduce it in the future via a user-defined flag (to only build the
part of the graph that needs building).

In any case, toolchain or platforms can be switched with the following
environment variables it configuration transitions are needed:
- `GOPACKAGESDRIVER_BAZEL_FLAGS` which will be passed to `bazel` invocations
- `GOPACKAGESDRIVER_BAZEL_QUERY_FLAGS` which will be passed to `bazel query`
   invocations
- `GOPACKAGESDRIVER_BAZEL_QUERY_SCOPE` which controls the scope of `bazel query` invocations
- `GOPACKAGESDRIVER_BAZEL_BUILD_FLAGS` which will be passed to `bazel build`
  invocations

Finally, the driver will not fail in case of a build failure, and even so
uses `--keep_going` and will return whatever packages that did build.

Signed-off-by: Steeve Morin <steeve@zen.ly>

* workspace: add vscode configuration

This allows to use gopls in the project itself.

Signed-off-by: Steeve Morin <steeve@zen.ly>

* go/tools/gopackagesdriver: don't use //... universe scope for file queries

When using `--universe_scope=//...`, the whole `//...` is loaded even though
we only use `same_pkg_direct_rdeps`. So remove it and specify the scope in the
query, such as `importpath` ones.

Signed-off-by: Steeve Morin <steeve@zen.ly>

* go/tools/gopackagesdriver: add GOPACKAGESDRIVER_BAZEL_QUERY_SCOPE

Add the `GOPACKAGESDRIVER_BAZEL_QUERY_SCOPE` environment variable so that
bazel queries are limited the given scope. This should save time when doing
`importpath` queries on large monorepos.

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Allow querying external dependencies with importpath

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

* go/tools/gopackagesdriver: fetch output_base from bazel info

While at it, fetch and parse the whole bazel info data.

* go/tools/gopackagesdriver: pull source files and return stdlib by default

When fetching the whole graph, default to returning the stdlib packages so that
vscode doesn't complain.

* Fix typo

* Fix formatting

* Enable queries on package registry

* Move some utiliy functions to the utils file

Easier to put them there

* Remove unsued bazel struct members

* Don't match only by string prefix when matching /...

Guard against packages with the same prefix by happending `/` to HasPrefix
and match the exact name if needed.

* Stdlib files are relative to the output_base, not execroot

While it used to work, this is better.

* Simpler error checking for bazel errors

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

* Don't use some on bazel query for package

When there is no such go_library, the some function would lead to errors like
ERROR: Evaluation of query failed: argument set is empty, with exit code 7. It
is unclear whether the query has a bug or there is no matching package. If we
don't have the some function, Bazel query will exit normally with empty result.
We can report a better error to gopls for this case.

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

* CHeck for empty labels when trying to build a target

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Honor build tags when listing files

This is done using a brand new build context that is configured using
the regular environment variables.

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Properly match full and child importpaths

Match a/ab and a/ab/c but not a/abc

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Add a comment telling why we exit(0)

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Better regexp for importpath matching

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

* Don't append empty bazel queries to bazel build

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

* Make completion on test files work

Signed-off-by: Steeve Morin <steeve@zen.ly>

* Handle tests that don't have embedded libraries

Signed-off-by: Steeve Morin <steeve@zen.ly>

Co-authored-by: Zhongpeng Lin <zplin@uber.com>

.vscode/extensions.json

@@ -0,0 +1,8 @@
+{
+  // See http://go.microsoft.com/fwlink/?LinkId=827846
+  // for the documentation about the extensions.json format
+  "recommendations": [
+    "bazelbuild.vscode-bazel",
+    "golang.go",
+  ]
+}

.vscode/settings.json

@@ -0,0 +1,32 @@
+{
+  "editor.formatOnSave": true,
+  "files.trimTrailingWhitespace": true,
+  "files.insertFinalNewline": true,
+  "go.goroot": "${workspaceFolder}/bazel-${workspaceFolderBasename}/external/go_sdk",
+  "go.toolsEnvVars": {
+    "GOPACKAGESDRIVER": "${workspaceFolder}/tools/gopackagesdriver.sh"
+  },
+  "go.enableCodeLens": {
+    "references": false,
+    "runtest": false
+  },
+  "gopls": {
+    "formatting.gofumpt": true,
+    "formatting.local": "github.com/bazelbuild/rules_go",
+    "ui.completion.usePlaceholders": true,
+    "ui.semanticTokens": true,
+    "ui.codelenses": {
+      "gc_details": false,
+      "regenerate_cgo": false,
+      "generate": false,
+      "test": false,
+      "tidy": false,
+      "upgrade_dependency": false,
+      "vendor": false
+    },
+  },
+  "go.useLanguageServer": true,
+  "go.buildOnSave": "off",
+  "go.lintOnSave": "off",
+  "go.vetOnSave": "off",
+}

go/tools/builders/stdliblist.go

@@ -111,7 +111,7 @@ func stdlibPackageID(importPath string) string {
 
 func execRootPath(execRoot, p string) string {
 	dir, _ := filepath.Rel(execRoot, p)
-	return filepath.Join("__BAZEL_EXECROOT__", dir)
+	return filepath.Join("__BAZEL_OUTPUT_BASE__", dir)
 }
 
 func absoluteSourcesPaths(execRoot, pkgDir string, srcs []string) []string {

go/tools/gopackagesdriver/BUILD.bazel

@@ -5,11 +5,13 @@ go_library(
     srcs = [
         "bazel.go",
         "bazel_json_builder.go",
+        "build_context.go",
         "driver_request.go",
         "flatpackage.go",
         "json_packages_driver.go",
         "main.go",
         "packageregistry.go",
+        "utils.go",
     ],
     importpath = "github.com/bazelbuild/rules_go/go/tools/gopackagesdriver",
     visibility = ["//visibility:private"],

go/tools/gopackagesdriver/aspect.bzl

@@ -17,90 +17,123 @@ load(
     "GoArchive",
     "GoStdLib",
 )
-load(
-    "//go/private:context.bzl",
-    "go_context",
-)
 load(
     "@bazel_skylib//lib:paths.bzl",
     "paths",
 )
-load(
-    "@bazel_skylib//lib:collections.bzl",
-    "collections",
-)
 
 GoPkgInfo = provider()
 
 def _is_file_external(f):
     return f.owner.workspace_root != ""
 
 def _file_path(f):
-    if f.is_source and not _is_file_external(f):
-        return paths.join("__BAZEL_WORKSPACE__", f.path)
-    return paths.join("__BAZEL_EXECROOT__", f.path)
+    prefix = "__BAZEL_WORKSPACE__"
+    if not f.is_source:
+        prefix = "__BAZEL_EXECROOT__"
+    elif _is_file_external(f):
+        prefix = "__BAZEL_OUTPUT_BASE__"
+    return paths.join(prefix, f.path)
+
+def _go_archive_to_pkg(archive):
+    return struct(
+        ID = str(archive.data.label),
+        PkgPath = archive.data.importpath,
+        ExportFile = _file_path(archive.data.export_file),
+        GoFiles = [
+            _file_path(src)
+            for src in archive.data.orig_srcs
+        ],
+        CompiledGoFiles = [
+            _file_path(src)
+            for src in archive.data.srcs
+        ],
+    )
+
+def _make_pkg_json(ctx, archive, pkg_info):
+    pkg_json_file = ctx.actions.declare_file(archive.data.name + ".pkg.json")
+    ctx.actions.write(pkg_json_file, content = pkg_info.to_json())
+    return pkg_json_file
 
 def _go_pkg_info_aspect_impl(target, ctx):
     # Fetch the stdlib JSON file from the inner most target
     stdlib_json_file = None
 
     deps_transitive_json_file = []
     deps_transitive_export_file = []
-    for dep in getattr(ctx.rule.attr, "deps", []):
-        if GoPkgInfo in dep:
-            pkg_info = dep[GoPkgInfo]
-            deps_transitive_json_file.append(pkg_info.transitive_json_file)
-            deps_transitive_export_file.append(pkg_info.transitive_export_file)
-            # Fetch the stdlib json from the first dependency
-            if not stdlib_json_file:
-                stdlib_json_file = pkg_info.stdlib_json_file
-
-    # If deps are embedded, do not gather their json or export_file since they
-    # are included in the current target, but do gather their deps'.
-    for dep in getattr(ctx.rule.attr, "embed", []):
-        if GoPkgInfo in dep:
-            pkg_info = dep[GoPkgInfo]
-            deps_transitive_json_file.append(pkg_info.deps_transitive_json_file)
-            deps_transitive_export_file.append(pkg_info.deps_transitive_export_file)
-
-    pkg_json_file = None
-    export_file = None
+    deps_transitive_compiled_go_files = []
+
+    for attr in ["deps", "embed"]:
+        for dep in getattr(ctx.rule.attr, attr, []):
+            if GoPkgInfo in dep:
+                pkg_info = dep[GoPkgInfo]
+                if attr == "deps":
+                    deps_transitive_json_file.append(pkg_info.transitive_json_file)
+                    deps_transitive_export_file.append(pkg_info.transitive_export_file)
+                    deps_transitive_compiled_go_files.append(pkg_info.transitive_compiled_go_files)
+                elif attr == "embed":
+                    # If deps are embedded, do not gather their json or export_file since they
+                    # are included in the current target, but do gather their deps'.
+                    deps_transitive_json_file.append(pkg_info.deps_transitive_json_file)
+                    deps_transitive_export_file.append(pkg_info.deps_transitive_export_file)
+                    deps_transitive_compiled_go_files.append(pkg_info.deps_transitive_compiled_go_files)
+
+                # Fetch the stdlib json from the first dependency
+                if not stdlib_json_file:
+                    stdlib_json_file = pkg_info.stdlib_json_file
+
+    pkg_json_files = []
+    compiled_go_files = []
+    export_files = []
+
     if GoArchive in target:
         archive = target[GoArchive]
-        export_file = archive.data.export_file
-        pkg = struct(
-            ID = str(archive.data.label),
-            PkgPath = archive.data.importpath,
-            ExportFile = _file_path(archive.data.export_file),
-            GoFiles = [
-                _file_path(src)
-                for src in archive.data.orig_srcs
-            ],
-            CompiledGoFiles = [
-                _file_path(src)
-                for src in archive.data.srcs
-            ],
-        )
-        pkg_json_file = ctx.actions.declare_file(archive.data.name + ".pkg.json")
-        ctx.actions.write(pkg_json_file, content = pkg.to_json())
-        # If there was no stdlib json in any dependencies, fetch it from the
-        # current go_ node.
-        if not stdlib_json_file:
-            stdlib_json_file = ctx.attr._go_stdlib[GoStdLib]._list_json
+        compiled_go_files.extend(archive.source.srcs)
+        export_files.append(archive.data.export_file)
+        pkg = _go_archive_to_pkg(archive)
+        pkg_json_files.append(_make_pkg_json(ctx, archive, pkg))
+
+        # if the rule is a test, we need to get the embedded go_library with the current
+        # test's sources. For that, consume the dependency via GoArchive.direct so that
+        # the test source files are there. Then, create the pkg json file directly. Only
+        # do that for direct dependencies that are not defined as deps, and use the
+        # importpath to find which.
+        if ctx.rule.kind == "go_test":
+            deps_targets = [
+                dep[GoArchive].data.importpath
+                for dep in ctx.rule.attr.deps
+                if GoArchive in dep
+            ]
+            for archive in target[GoArchive].direct:
+                if archive.data.importpath not in deps_targets:
+                    pkg = _go_archive_to_pkg(archive)
+                    pkg_json_files.append(_make_pkg_json(ctx, archive, pkg))
+                    compiled_go_files.extend(archive.source.srcs)
+                    export_files.append(archive.data.export_file)
+
+    # If there was no stdlib json in any dependencies, fetch it from the
+    # current go_ node.
+    if not stdlib_json_file:
+        stdlib_json_file = ctx.attr._go_stdlib[GoStdLib]._list_json
 
     pkg_info = GoPkgInfo(
-        json = pkg_json_file,
         stdlib_json_file = stdlib_json_file,
         transitive_json_file = depset(
-            direct = [pkg_json_file] if pkg_json_file else [],
+            direct = pkg_json_files,
             transitive = deps_transitive_json_file,
         ),
         deps_transitive_json_file = depset(
             transitive = deps_transitive_json_file,
         ),
-        export_file = export_file,
+        transitive_compiled_go_files = depset(
+            direct = compiled_go_files,
+            transitive = deps_transitive_compiled_go_files,
+        ),
+        deps_transitive_compiled_go_files = depset(
+            transitive = deps_transitive_compiled_go_files,
+        ),
         transitive_export_file = depset(
-            direct = [export_file] if export_file else [],
+            direct = export_files,
             transitive = deps_transitive_export_file,
         ),
         deps_transitive_export_file = depset(
@@ -112,8 +145,9 @@ def _go_pkg_info_aspect_impl(target, ctx):
         pkg_info,
         OutputGroupInfo(
             go_pkg_driver_json_file = pkg_info.transitive_json_file,
+            go_pkg_driver_srcs = pkg_info.transitive_compiled_go_files,
             go_pkg_driver_export_file = pkg_info.transitive_export_file,
-            go_pkg_driver_stdlib_json_file = depset([pkg_info.stdlib_json_file] if pkg_info.stdlib_json_file else [])
+            go_pkg_driver_stdlib_json_file = depset([pkg_info.stdlib_json_file] if pkg_info.stdlib_json_file else []),
         ),
     ]
 

go/tools/gopackagesdriver/bazel.go

@@ -15,8 +15,11 @@
 package main
 
 import (
+	"bufio"
+	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/url"
@@ -32,8 +35,8 @@ const (
 
 type Bazel struct {
 	bazelBin      string
-	execRoot      string
 	workspaceRoot string
+	info          map[string]string
 }
 
 // Minimal BEP structs to access the build outputs
@@ -51,22 +54,34 @@ func NewBazel(ctx context.Context, bazelBin, workspaceRoot string) (*Bazel, erro
 		bazelBin:      bazelBin,
 		workspaceRoot: workspaceRoot,
 	}
-	if execRoot, err := b.run(ctx, "info", "execution_root"); err != nil {
-		return nil, fmt.Errorf("unable to find execution root: %w", err)
-	} else {
-		b.execRoot = strings.TrimSpace(execRoot)
+	if err := b.fillInfo(ctx); err != nil {
+		return nil, fmt.Errorf("unable to query bazel info: %w", err)
 	}
 	return b, nil
 }
 
+func (b *Bazel) fillInfo(ctx context.Context) error {
+	b.info = map[string]string{}
+	output, err := b.run(ctx, "info")
+	if err != nil {
+		return err
+	}
+	scanner := bufio.NewScanner(bytes.NewBufferString(output))
+	for scanner.Scan() {
+		parts := strings.SplitN(strings.TrimSpace(scanner.Text()), ":", 2)
+		b.info[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1])
+	}
+	return nil
+}
+
 func (b *Bazel) run(ctx context.Context, command string, args ...string) (string, error) {
 	cmd := exec.CommandContext(ctx, b.bazelBin, append([]string{
 		command,
 		"--tool_tag=" + toolTag,
 		"--ui_actions_shown=0",
 	}, args...)...)
 	fmt.Fprintln(os.Stderr, "Running:", cmd.Args)
-	cmd.Dir = b.workspaceRoot
+	cmd.Dir = b.WorkspaceRoot()
 	cmd.Stderr = os.Stderr
 	output, err := cmd.Output()
 	return string(output), err
@@ -88,7 +103,13 @@ func (b *Bazel) Build(ctx context.Context, args ...string) ([]string, error) {
 		"--build_event_json_file_path_conversion=no",
 	}, args...)
 	if _, err := b.run(ctx, "build", args...); err != nil {
-		return nil, fmt.Errorf("bazel build failed: %w", err)
+		// Ignore a regular build failure to get partial data.
+		// See https://docs.bazel.build/versions/main/guide.html#what-exit-code-will-i-get on
+		// exit codes.
+		var exerr *exec.ExitError
+		if !errors.As(err, &exerr) || exerr.ExitCode() != 1 {
+			return nil, fmt.Errorf("bazel build failed: %w", err)
+		}
 	}
 
 	files := make([]string, 0)
@@ -120,10 +141,22 @@ func (b *Bazel) Query(ctx context.Context, args ...string) ([]string, error) {
 	return strings.Split(strings.TrimSpace(output), "\n"), nil
 }
 
+func (b *Bazel) QueryLabels(ctx context.Context, args ...string) ([]string, error) {
+	output, err := b.run(ctx, "query", args...)
+	if err != nil {
+		return nil, fmt.Errorf("bazel query failed: %w", err)
+	}
+	return strings.Split(strings.TrimSpace(output), "\n"), nil
+}
+
 func (b *Bazel) WorkspaceRoot() string {
 	return b.workspaceRoot
 }
 
 func (b *Bazel) ExecutionRoot() string {
-	return b.execRoot
+	return b.info["execution_root"]
+}
+
+func (b *Bazel) OutputBase() string {
+	return b.info["output_base"]
 }