go/tools/gopackagesdriver: add automatic target detection (bazel-cont…

…rib#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 <[email protected]> * workspace: add vscode configuration This allows to use gopls in the project itself. Signed-off-by: Steeve Morin <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Allow querying external dependencies with importpath Co-authored-by: Zhongpeng Lin <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * CHeck for empty labels when trying to build a target Signed-off-by: Steeve Morin <[email protected]> * 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 <[email protected]> * Properly match full and child importpaths Match a/ab and a/ab/c but not a/abc Signed-off-by: Steeve Morin <[email protected]> * Add a comment telling why we exit(0) Signed-off-by: Steeve Morin <[email protected]> * Better regexp for importpath matching Co-authored-by: Zhongpeng Lin <[email protected]> * Don't append empty bazel queries to bazel build Co-authored-by: Zhongpeng Lin <[email protected]> * Make completion on test files work Signed-off-by: Steeve Morin <[email protected]> * Handle tests that don't have embedded libraries Signed-off-by: Steeve Morin <[email protected]> Co-authored-by: Zhongpeng Lin <[email protected]>
limdor · Aug 28, 2021 · 70b8365 · 70b8365
1 parent 431d21c
commit 70b8365
Show file tree

Hide file tree

Showing 14 changed files with 437 additions and 174 deletions.
diff --git a/.vscode/extensions.json b/.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",
+  ]
+}
diff --git a/.vscode/settings.json b/.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",
+}
diff --git a/go/tools/builders/stdliblist.go b/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 {

diff --git a/go/tools/gopackagesdriver/BUILD.bazel b/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"],

diff --git a/go/tools/gopackagesdriver/aspect.bzl b/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 []),
         ),
     ]
 

diff --git a/go/tools/gopackagesdriver/bazel.go b/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"]
 }