Skip to content

Commit

Permalink
Gazelle: support empty prefixes
Browse files Browse the repository at this point in the history
Gazelle now allows go_prefix to be empty. It must still be set either
on the command line or in a go_prefix rule. When the prefix is empty,
all imports not in the standard library are resolved
locally. Dependencies will not point to the vendor directory or to
external repositories.

* Gazelle now hard-codes a list of standard packages from the SDK that
  was used to build it.
* All imports (including standard imports) are now added to
  packages.GoTarget. The resolve package filters out standard
  libraries.

Fixes bazel-contrib#870
  • Loading branch information
Jay Conrod committed Oct 13, 2017
1 parent 9cf23e2 commit 3781938
Show file tree
Hide file tree
Showing 20 changed files with 520 additions and 79 deletions.
1 change: 1 addition & 0 deletions go/private/BUILD.sdk.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,4 @@ filegroup(
]),
)

exports_files(["packages.txt"])
13 changes: 13 additions & 0 deletions go/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,16 @@ def go_importpath(ctx):
path = path[1:]
return path

def env_execute(ctx, arguments, environment = None, **kwargs):
"""env_executes a command in a repository context. It prepends "env -i"
to "arguments" before calling "ctx.execute".
Variables that aren't explicitly mentioned in "environment"
are removed from the environment. This should be preferred to "ctx.execut"e
in most situations.
"""
env_args = ["env", "-i"]
if environment:
for k, v in environment.items():
env_args += ["%s=%s" % (k, v)]
return ctx.execute(env_args + arguments, **kwargs)
15 changes: 1 addition & 14 deletions go/private/go_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@io_bazel_rules_go//go/private:common.bzl", "env_execute")
load("@io_bazel_rules_go//go/private:toolchain.bzl", "executable_extension")

def _go_repository_impl(ctx):
Expand Down Expand Up @@ -120,17 +121,3 @@ go_repository = repository_rule(
},
)
"""See go/workspace.rst#go-repository for full documentation."""

def env_execute(ctx, arguments, environment = None, **kwargs):
"""env_execute prepends "env -i" to "arguments" before passing it to
ctx.execute.
Variables that aren't explicitly mentioned in "environment"
are removed from the environment. This should be preferred to "ctx.execute"
in most situations.
"""
env_args = ["env", "-i"]
if environment:
for k, v in environment.items():
env_args += ["%s=%s" % (k, v)]
return ctx.execute(env_args + arguments, **kwargs)
15 changes: 15 additions & 0 deletions go/private/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@io_bazel_rules_go//go/private:common.bzl", "env_execute")

def executable_extension(ctx):
extension = ""
if ctx.os.name.startswith('windows'):
Expand Down Expand Up @@ -90,13 +92,26 @@ def _prepare(ctx):
else:
ctx.file("tmp/ignore", content="") # make a file to force the directory to exist
tmp = str(ctx.path("tmp").realpath)

# Build the standard library for valid cross compile platforms
#TODO: fix standard library cross compilation
if ctx.name.endswith("linux_amd64") and ctx.os.name == "linux":
_cross_compile_stdlib(ctx, "windows", "amd64", tmp)
if ctx.name.endswith("darwin_amd64") and ctx.os.name == "mac os x":
_cross_compile_stdlib(ctx, "linux", "amd64", tmp)

# Create a text file with a list of standard packages.
# OPT: just list directories under src instead of running "go list". No
# need to read all source files. We need a portable way to run code though.
result = env_execute(ctx,
arguments = ["bin/go", "list", "..."],
environment = {"GOROOT": str(ctx.path("."))},
)
if result.return_code != 0:
print(result.stderr)
fail("failed to list standard packages")
ctx.file("packages.txt", result.stdout)

go_sdk = repository_rule(
implementation = _go_sdk_impl,
attrs = {
Expand Down
46 changes: 46 additions & 0 deletions go/tools/gazelle/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,52 @@ go_grpc_library(
}})
}

func TestEmptyGoPrefix(t *testing.T) {
files := []fileSpec{
{path: "WORKSPACE"},
{
path: "foo/foo.go",
content: "package foo",
}, {
path: "bar/bar.go",
content: `
package bar
import (
_ "fmt"
_ "foo"
)
`,
},
}

dir, err := createFiles(files)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

args := []string{"-go_prefix", ""}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}

checkFiles(t, dir, []fileSpec{{
path: filepath.Join("bar", config.DefaultValidBuildFileNames[0]),
content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["bar.go"],
importpath = "bar",
visibility = ["//visibility:public"],
deps = ["//foo:go_default_library"],
)
`,
}})
}

// TODO(jayconrod): more tests
// run in fix mode in testdata directories to create new files
// run in diff mode in testdata directories to update existing files (no change)
33 changes: 29 additions & 4 deletions go/tools/gazelle/gazelle/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"flag"
"fmt"
"go/build"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -302,7 +303,8 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error)
buildFileName := fs.String("build_file_name", "BUILD.bazel,BUILD", "comma-separated list of valid build file names.\nThe first element of the list is the name of output build files to generate.")
buildTags := fs.String("build_tags", "", "comma-separated list of build tags. If not specified, Gazelle will not\n\tfilter sources with build constraints.")
external := fs.String("external", "external", "external: resolve external packages with go_repository\n\tvendored: resolve external packages as packages in vendor/")
goPrefix := fs.String("go_prefix", "", "go_prefix of the target workspace")
var goPrefix explicitFlag
fs.Var(&goPrefix, "go_prefix", "prefix of import paths in the current workspace")
repoRoot := fs.String("repo_root", "", "path to a directory which corresponds to go_prefix, otherwise gazelle searches for it.")
fs.Var(&knownImports, "known_import", "import path for which external resolution is skipped (can specify multiple times)")
mode := fs.String("mode", "fix", "print: prints all of the updated BUILD files\n\tfix: rewrites all of the BUILD files in place\n\tdiff: computes the rewrite but then just does a diff")
Expand Down Expand Up @@ -364,12 +366,17 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error)
c.Platforms = config.DefaultPlatformTags
c.PreprocessTags()

c.GoPrefix = *goPrefix
if c.GoPrefix == "" {
if goPrefix.set {
c.GoPrefix = goPrefix.value
} else {
c.GoPrefix, err = loadGoPrefix(&c)
if err != nil {
return nil, cmd, nil, fmt.Errorf("-go_prefix not set and not root BUILD file found")
return nil, cmd, nil, fmt.Errorf("-go_prefix not set")
}
// TODO(jayconrod): read prefix directives when they are supported.
}
if strings.HasPrefix(c.GoPrefix, "/") || build.IsLocalImport(c.GoPrefix) {
return nil, cmd, nil, fmt.Errorf("invalid go_prefix: %q", c.GoPrefix)
}

c.ShouldFix = cmd == fixCmd
Expand Down Expand Up @@ -400,6 +407,24 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error)
return &c, cmd, emit, err
}

type explicitFlag struct {
set bool
value string
}

func (f *explicitFlag) Set(value string) error {
f.set = true
f.value = value
return nil
}

func (f *explicitFlag) String() string {
if f == nil {
return ""
}
return f.value
}

func loadBuildFile(c *config.Config, dir string) (*bf.File, error) {
var buildPath string
for _, base := range c.ValidBuildFileNames {
Expand Down
6 changes: 0 additions & 6 deletions go/tools/gazelle/packages/fileinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,6 @@ func escapeOption(opt string) string {
).Replace(opt)
}

// isStandard determines if importpath points a Go standard package.
func isStandard(goPrefix, importpath string) bool {
seg := strings.SplitN(importpath, "/", 2)[0]
return !strings.Contains(seg, ".") && !strings.HasPrefix(importpath, goPrefix+"/")
}

// otherFileInfo returns information about a non-.go file. It will parse
// part of the file to determine build tags. If the file can't be read, an
// error will be logged, and partial information will be returned.
Expand Down
4 changes: 2 additions & 2 deletions go/tools/gazelle/packages/fileinfo_go.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ func goFileInfo(c *config.Config, dir, rel, name string) fileInfo {
log.Printf("%s: error reading go file: %v", info.path, err)
}
}
} else if !isStandard(c.GoPrefix, path) {
info.imports = append(info.imports, path)
continue
}
info.imports = append(info.imports, path)
}
}

Expand Down
3 changes: 2 additions & 1 deletion go/tools/gazelle/packages/fileinfo_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,15 @@ import (
},
},
{
"standard imports not included",
"standard imports included",
"foo.go",
`package foo
import "fmt"
`,
fileInfo{
packageName: "foo",
imports: []string{"fmt"},
},
},
{
Expand Down
25 changes: 0 additions & 25 deletions go/tools/gazelle/packages/fileinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,31 +301,6 @@ func TestJoinOptions(t *testing.T) {
}
}

func TestIsStandard(t *testing.T) {
for _, tc := range []struct {
goPrefix, importpath string
want bool
}{
{"", "fmt", true},
{"", "encoding/json", true},
{"", "foo/bar", true},
{"", "foo.com/bar", false},
{"foo", "fmt", true},
{"foo", "encoding/json", true},
{"foo", "foo", true},
{"foo", "foo/bar", false},
{"foo", "foo.com/bar", false},
{"foo.com/bar", "fmt", true},
{"foo.com/bar", "encoding/json", true},
{"foo.com/bar", "foo/bar", true},
{"foo.com/bar", "foo.com/bar", false},
} {
if got := isStandard(tc.goPrefix, tc.importpath); got != tc.want {
t.Errorf("for prefix %q, importpath %q: got %#v; want %#v", tc.goPrefix, tc.importpath, got, tc.want)
}
}
}

func TestReadTags(t *testing.T) {
for _, tc := range []struct {
desc, source string
Expand Down
16 changes: 12 additions & 4 deletions go/tools/gazelle/packages/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package packages

import (
"errors"
"fmt"
"path"
"sort"
Expand Down Expand Up @@ -331,14 +332,19 @@ func uniq(ss []string) []string {
return result
}

// Map applies a function that processes individual strings to the strings in
// "ps" and returns a new PlatformStrings with the results.
var Skip = errors.New("Skip")

// Map applies a function f to the individual strings in ps. Map returns a
// new PlatformStrings with the results and a slice of errors that f returned.
// When f returns the error Skip, neither the result nor the error are recorded.
func (ps *PlatformStrings) Map(f func(string) (string, error)) (PlatformStrings, []error) {
result := PlatformStrings{Generic: make([]string, 0, len(ps.Generic))}
var errors []error
for _, s := range ps.Generic {
if r, err := f(s); err != nil {
errors = append(errors, err)
if err != Skip {
errors = append(errors, err)
}
} else {
result.Generic = append(result.Generic, r)
}
Expand All @@ -350,7 +356,9 @@ func (ps *PlatformStrings) Map(f func(string) (string, error)) (PlatformStrings,
result.Platform[n] = make([]string, 0, len(ss))
for _, s := range ss {
if r, err := f(s); err != nil {
errors = append(errors, err)
if err != Skip {
errors = append(errors, err)
}
} else {
result.Platform[n] = append(result.Platform[n], r)
}
Expand Down
13 changes: 9 additions & 4 deletions go/tools/gazelle/packages/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"path"
"reflect"
"strings"
"testing"
)

Expand Down Expand Up @@ -132,15 +133,19 @@ func TestCleanPlatformStrings(t *testing.T) {

func TestMapPlatformStrings(t *testing.T) {
f := func(s string) (string, error) {
if len(s) > 0 && s[0] == 'e' {
switch {
case strings.HasPrefix(s, "e"):
return "", fmt.Errorf("invalid string: %s", s)
case strings.HasPrefix(s, "s"):
return "", Skip
default:
return s + "x", nil
}
return s + "x", nil
}
ps := PlatformStrings{
Generic: []string{"a", "e1"},
Generic: []string{"a", "e1", "s1"},
Platform: map[string][]string{
"linux": []string{"b", "e2"},
"linux": []string{"b", "e2", "s2"},
},
}
got, gotErrors := ps.Map(f)
Expand Down
12 changes: 12 additions & 0 deletions go/tools/gazelle/resolve/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_library(
"resolve.go",
"resolve_external.go",
"resolve_vendored.go",
"std_package_list.go",
],
visibility = ["//visibility:public"],
deps = [
Expand All @@ -30,3 +31,14 @@ go_test(
"@org_golang_x_tools//go/vcs:go_default_library",
],
)

# TODO(jayconrod): test that the checked-in static file matches the generated
# file. The generated code is checked in so that Gazelle can still be built
# with "go get".
genrule(
name = "std_package_list",
srcs = ["@go_sdk//:packages.txt"],
outs = ["std_package_list.go"],
tools = ["//go/tools/gazelle/resolve/internal/gen_std_package_list"],
cmd = "$(location //go/tools/gazelle/resolve/internal/gen_std_package_list) $< $@",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary")

go_binary(
name = "gen_std_package_list",
srcs = ["gen_std_package_list.go"],
visibility = ["//go/tools/gazelle/resolve:__subpackages__"],
)
Loading

0 comments on commit 3781938

Please sign in to comment.