Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prototype gopackagesdriver based on bazel query. #2835

Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
Benjamin Staffin <[email protected]>
Brian Silverman <[email protected]>
David Santiago <[email protected]>
Derivita Inc.
Google Inc.
Jake Voytko <[email protected]>
Yuki Yugui Sonoda <[email protected]>
12 changes: 12 additions & 0 deletions go/private/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ def go_rules_dependencies(is_rules_go = False):
strip_prefix = "rules_cc-b1c40e1de81913a3c40e5948f78719c28152486d",
)

# Needed for gopackagesdriver
_maybe(
http_archive,
name = "com_github_bazelbuild_bazel_watcher",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really really leery of adding new dependencies. It adds a great deal of maintenance complexity.

It looks like these packages are used to invoke Bazel commands and to parse bazel query output. Can we do something more minimal with our own package here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would copying the protos and bazel.go here be better?
If so, should I put them into //third_party or here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copying the pre-generated .pb.go files is good. They can go anywhere under go/tools/gopackagesdriver; nothing else should use them.

# master, as of 2021-03-03
urls = [
"https://github.com/bazelbuild/bazel-watcher/archive/512875a4a20b48d7539151e55b896f00be8c4a60.zip",
],
sha256 = "e822f17ff2c533a068c48fd75764028aaee99f65381469c1b2f269e602e8fad7",
strip_prefix = "bazel-watcher-512875a4a20b48d7539151e55b896f00be8c4a60",
)

# Proto dependencies
# These are limited as much as possible. In most cases, users need to
# declare these on their own (probably via go_repository rules generated
Expand Down
11 changes: 11 additions & 0 deletions go/tools/gopackagesdriver/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary")

go_binary(
name = "gopackagesdriver",
srcs = ["gopackagesdriver.go"],
visibility = ["//visibility:public"],
deps = [
"//go/tools/gopackagesdriver/protocol",
"//go/tools/gopackagesdriver/bazelquerydriver",
],
)
18 changes: 18 additions & 0 deletions go/tools/gopackagesdriver/bazelquerydriver/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "bazelquerydriver",
srcs = [
"driver.go",
"sdk.go",
],
importpath = "github.com/bazelbuild/rules_go/go/tools/gopackagesdriver/bazelquerydriver",
visibility = ["//go/tools/gopackagesdriver:__subpackages__"],
deps = [
"//go/tools/gopackagesdriver/bazelquerydriver/pkgconv",
"//go/tools/gopackagesdriver/protocol",
"@com_github_bazelbuild_bazel_watcher//third_party/bazel/master/src/main/protobuf/blaze_query:go_default_library",
"@com_github_bazelbuild_bazel_watcher//bazel:go_default_library",
"@org_golang_x_tools//go/packages:go_default_library",
],
)
310 changes: 310 additions & 0 deletions go/tools/gopackagesdriver/bazelquerydriver/driver.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,310 @@
// Copyright 2021 The Bazel Go Rules Authors. All rights reserved.
ribrdb marked this conversation as resolved.
Show resolved Hide resolved
//
// 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.

// bazelquerydriver implements the driver interface for
// golang.org/x/tools/go/packages. It uses `bazel query` to gather
// information about packages built using bazel.
package bazelquerydriver
ribrdb marked this conversation as resolved.
Show resolved Hide resolved

import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"go/types"
"log"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"

"github.com/bazelbuild/bazel-watcher/bazel"
"github.com/bazelbuild/bazel-watcher/third_party/bazel/master/src/main/protobuf/blaze_query"
"github.com/bazelbuild/rules_go/go/tools/gopackagesdriver/bazelquerydriver/pkgconv"
"github.com/bazelbuild/rules_go/go/tools/gopackagesdriver/protocol"
"golang.org/x/tools/go/packages"
)

const fileQueryPrefix = "file="

const supportedModes = packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedImports | packages.NeedDeps | packages.NeedTypesSizes | packages.NeedModule

type bazelDriver struct {
cfg protocol.Request
resp protocol.Response
bazel bazel.Bazel
sdk *gosdk
workspaceRoot string
bazelQueries []string
stdlibImports map[string]bool
fileQueries map[string]bool
importQueries map[string]bool
wildcardQuery bool
}

// LoadPackages uses bazel query to answer a gopackagesdriver request.
func LoadPackages(cfg protocol.Request, patterns ...string) (*protocol.Response, error) {
bzl := bazel.New()
sdk, err := newGoSDK(bzl)
if err != nil {
return nil, err
}
driver := &bazelDriver{
cfg: cfg,
bazel: bzl,
sdk: sdk,
stdlibImports: make(map[string]bool),
fileQueries: make(map[string]bool),
importQueries: make(map[string]bool),
}
return driver.loadPackages(patterns...)
}

func (d *bazelDriver) loadPackages(patterns ...string) (*protocol.Response, error) {
if info, err := d.bazel.Info(); err != nil {
return &protocol.Response{NotHandled: true}, nil
} else {
d.workspaceRoot = info["workspace"]
}

log.Printf("mode: %v", d.cfg.Mode)
unsupportedModes := d.cfg.Mode &^ supportedModes
if unsupportedModes != 0 {
return nil, fmt.Errorf("%v (%b) not implemented", unsupportedModes, unsupportedModes)
}

if len(patterns) == 0 {
patterns = append(patterns, ".")
}

for _, patt := range patterns {
switch {
case strings.HasPrefix(patt, fileQueryPrefix):
fp := strings.TrimPrefix(patt, fileQueryPrefix)
if err := d.prepareFileQuery(fp); err != nil {
return nil, err
}
case patt == ".":
d.bazelQueries = append(d.bazelQueries, goFilter(":all"))
d.wildcardQuery = true
case patt == "./...":
d.bazelQueries = append(d.bazelQueries, goFilter("..."))
d.wildcardQuery = true
case d.sdk.packages[patt]:
d.stdlibImports[patt] = true
default:
// TODO: do we need to check for a package ID instead of import path?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should assume arguments are Bazel labels, not import paths.

We'll need to handle import paths in the standard library, since there's no label for individual packages.

If we must process import paths without wildcards, we should make an attempt to find the directory without scanning the whole repo. bazel query //... can be very expensive for large repos.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely seeing some queries for import paths from gopls in my logfile.
How would we find the directory without scanning the whole repo?
Maybe we need to maintain a cache on disk of import path -> package and generate filename -> package?

Note that the first thing gopls does is query for "./..." which does take a long time. But then if you're lucky bazel keeps that loaded so later queries are fast.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I imagine we'll need to work with the gopls folks later on to avoid import path queries.

For now, let's add TODOs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a todo in sdk.go

d.importQueries[patt] = true
d.bazelQueries = append(d.bazelQueries, fmt.Sprintf("attr(importpath, '%v', deps(%v))", patt, goFilter("//...")))
}
}

var resp protocol.Response

for pkg := range d.stdlibImports {
resp.Roots = append(resp.Roots, pkg)
}

if len(d.bazelQueries) != 0 {
pkgs, roots, err := d.packagesFromQueries(d.bazelQueries)
if err != nil {
return nil, err
}
resp.Packages = append(resp.Packages, pkgs...)
resp.Roots = append(resp.Roots, roots...)
}

stdlib, err := d.sdk.loadPackages(&d.cfg, d.stdlibImports)
if err != nil {
return nil, err
}
resp.Packages = append(resp.Packages, stdlib...)

if d.cfg.Mode&packages.NeedTypesSizes != 0 {
goarch := protocol.GetEnv(&d.cfg, "GOARCH", runtime.GOARCH)
// SizesFor always return StdSizesm,
resp.Sizes = types.SizesFor("gc", goarch).(*types.StdSizes)
}

return &resp, nil
}

// prepareFileQuery parses queries starting with file=.
// It determines whether the file is inside the standard library,
// and adds the appropriate stdlib or bazel query to `d`.
// TODO: handle queries for generated files
func (d *bazelDriver) prepareFileQuery(fp string) error {
if len(fp) == 0 {
return fmt.Errorf("\"file=\" prefix given with no query after it")
}
stdlibRoot := filepath.Join(d.sdk.goroot, "src") + string(filepath.Separator)

if filepath.IsAbs(fp) {
if strings.HasPrefix(fp, stdlibRoot) {
jayconrod marked this conversation as resolved.
Show resolved Hide resolved
if rel, err := filepath.Rel(stdlibRoot, fp); err == nil {
pkg := filepath.Dir(rel)
if d.sdk.packages[pkg] {
d.stdlibImports[pkg] = true
}
}
return nil
jayconrod marked this conversation as resolved.
Show resolved Hide resolved
}
if rel, err := filepath.Rel(d.workspaceRoot, fp); err == nil {
fp = rel
}
}

query, err := d.convertFileQuery(fp)
if err != nil {
return err
}
d.bazelQueries = append(d.bazelQueries, query)
d.fileQueries[fp] = true
return nil
}

// convertFileQuery returns a bazel query expression to find targets where `srcs` includes `path`.
func (d *bazelDriver) convertFileQuery(path string) (string, error) {
ribrdb marked this conversation as resolved.
Show resolved Hide resolved
log.Printf("bazel query %#v", path)
result, err := d.bazel.Query(path)
if err != nil {
return "", err
}
if len(result.Target) == 0 {
return "", fmt.Errorf("no source file %#v", path)
} else if len(result.Target) > 1 {
return "", fmt.Errorf("multiple results for %#v", path)
}

t := result.Target[0]
if t.GetType() != blaze_query.Target_SOURCE_FILE {
return "", fmt.Errorf("wanted SOURCE_FILE, but %v is %v", path, t.GetType())
}
name := t.GetSourceFile().GetName()
pkg := strings.Split(name, ":")[0]
return fmt.Sprintf("attr(srcs, '%v', '%v:*')", name, pkg), nil
}

func (d *bazelDriver) packagesFromQueries(queries []string) (pkgs []*packages.Package, roots []string, err error) {
query := strings.Join(queries, "+")
if d.cfg.Mode&packages.NeedDeps != 0 {
query = goFilter(fmt.Sprintf("deps(%v)", query))
}

log.Printf("bazel query %#v", query)
results, err := d.bazel.Query(query)
if err != nil {
return
}

pkgs = pkgconv.Load(results.GetTarget())

log.Printf("file queries: %v", d.fileQueries)
for _, p := range pkgs {
if d.includeInRoots(p) {
roots = append(roots, p.ID)
}
}

if d.cfg.Mode&packages.NeedCompiledGoFiles != 0 {
for _, pkg := range pkgs {
pkg.CompiledGoFiles = pkg.GoFiles
}
}
if d.cfg.Mode&(packages.NeedImports|packages.NeedName) != 0 {
for _, pkg := range pkgs {
name, imports, err := d.parsePackage(pkg)
if err != nil {
return nil, nil, fmt.Errorf("parsePackage: %w", err)
}
pkg.Name = name
for _, i := range imports {
if i, err = strconv.Unquote(i); err == nil {
if d.sdk.packages[i] {
if pkg.Imports == nil {
pkg.Imports = make(map[string]*packages.Package)
}
pkg.Imports[i] = &packages.Package{ID: i}
d.stdlibImports[i] = true
}
}
}
}
}
return
}

func (d *bazelDriver) includeInRoots(pkg *packages.Package) bool {
if d.wildcardQuery {
return strings.HasPrefix(pkg.ID, "//")
}
if pkg.PkgPath != "" && d.importQueries[pkg.PkgPath] {
return true
} else if len(d.fileQueries) > 0 && !strings.HasPrefix(pkg.ID, "@") {
for _, f := range pkg.GoFiles {
if d.fileQueries[f] {
return true
}
if rel, err := filepath.Rel(d.workspaceRoot, f); err == nil && d.fileQueries[rel] {
return true
}
}
}
return false
}

func (d *bazelDriver) parsePackage(pkg *packages.Package) (packageName string, imports []string, err error) {
triedBuild := false
dedupe := make(map[string]bool)
fset := token.NewFileSet()

for _, filename := range pkg.GoFiles {
var f *os.File
f, err = os.Open(filename)
if os.IsNotExist(err) && !triedBuild {
triedBuild = true
log.Printf("bazel build %v\n", pkg.ID)
d.bazel.Build(pkg.ID)
f, err = os.Open(filename)
}
if err != nil {
return
}
defer f.Close()
var syntax *ast.File
if syntax, err = parser.ParseFile(fset, filename, f, parser.ImportsOnly); err == nil {
packageName = syntax.Name.Name
for _, i := range syntax.Imports {
pkg := i.Path.Value
if !dedupe[pkg] {
dedupe[pkg] = true
imports = append(imports, pkg)
}
}
if d.cfg.Mode&packages.NeedImports == 0 {
// We don't need to parse all the files just to get the package name.
return
}
}
}
if packageName == "" {
packageName = filepath.Base(pkg.PkgPath)
}
return
}

func goFilter(expr string) string {
return fmt.Sprintf("kind(\"alias|proto_library|go_\", %s)", expr)
}
18 changes: 18 additions & 0 deletions go/tools/gopackagesdriver/bazelquerydriver/pkgconv/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "pkgconv",
srcs = [
"go_embed_data.go",
"go_library.go",
"go_proto_library.go",
"gotest.go",
"pkgconv.go",
],
importpath = "github.com/bazelbuild/rules_go/go/tools/gopackagesdriver/bazelquerydriver/pkgconv",
visibility = ["//go/tools/gopackagesdriver:__subpackages__"],
deps = [
"@org_golang_x_tools//go/packages:go_default_library",
"@com_github_bazelbuild_bazel_watcher//third_party/bazel/master/src/main/protobuf/blaze_query:go_default_library",
],
)
Loading