diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 3826d1ba9..0f39e8de5 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -27,4 +27,4 @@ platforms: # It requires UNIX domain sockets. - "-//cmd/autogazelle/..." # Fails to execute, apparently due to command-line length limit. - - "-//internal:go_repository_test" + - "-//internal:bazel_test" diff --git a/README.rst b/README.rst index 2efa07b11..123a9d90f 100644 --- a/README.rst +++ b/README.rst @@ -73,10 +73,10 @@ should look like this: http_archive( name = "io_bazel_rules_go", - sha256 = "a8d6b1b354d371a646d2f7927319974e0f9e52f73a2452d2b3877118169eb6bb", + sha256 = "2d536797707dd1697441876b2e862c58839f975c8fc2f0f96636cbd428f45866", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz", - "https://github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.5/rules_go-v0.23.5.tar.gz", + "https://github.com/bazelbuild/rules_go/releases/download/v0.23.5/rules_go-v0.23.5.tar.gz", ], ) @@ -466,29 +466,33 @@ The following flags are accepted: | | | This flag can only be used with ``-from_file``. | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| :flag:`-build_file_names file1,file2,...` | | +| :flag:`-build_directives arg1,arg2,...` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| Sets the ``build_file_name`` attribute for the generated `go_repository`_ rule(s). | +| Sets the ``build_directives attribute`` for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | :flag:`-build_external external|vendored` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | Sets the ``build_external`` attribute for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ +| :flag:`-build_extra_args arg1,arg2,...` | | ++----------------------------------------------------------------------------------------------------------+----------------------------------------------+ +| Sets the ``build_extra_args attribute`` for the generated `go_repository`_ rule(s). | ++----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | :flag:`-build_file_generation auto|on|off` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | Sets the ``build_file_generation`` attribute for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| :flag:`-build_tags tag1,tag2,...` | | +| :flag:`-build_file_names file1,file2,...` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| Sets the ``build_tags`` attribute for the generated `go_repository`_ rule(s). | +| Sets the ``build_file_name`` attribute for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | :flag:`-build_file_proto_mode default|package|legacy|disable|disable_global` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | Sets the ``build_file_proto_mode`` attribute for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| :flag:`-build_extra_args arg1,arg2,...` | | +| :flag:`-build_tags tag1,tag2,...` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| Sets the ``build_exra_args attribute`` for the generated `go_repository`_ rule(s). | +| Sets the ``build_tags`` attribute for the generated `go_repository`_ rule(s). | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ Directives diff --git a/WORKSPACE b/WORKSPACE index d0ac2182b..3b941746a 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,10 +4,10 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", - sha256 = "a8d6b1b354d371a646d2f7927319974e0f9e52f73a2452d2b3877118169eb6bb", + sha256 = "2d536797707dd1697441876b2e862c58839f975c8fc2f0f96636cbd428f45866", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz", - "https://github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.5/rules_go-v0.23.5.tar.gz", + "https://github.com/bazelbuild/rules_go/releases/download/v0.23.5/rules_go-v0.23.5.tar.gz", ], ) diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go index bf13e299e..2f71dd170 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -97,22 +97,36 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro return fmt.Errorf("-patch set but -mode is %s, not diff", ucr.mode) } + var wd string + if wsDir := os.Getenv("BUILD_WORKSPACE_DIRECTORY"); wsDir != "" { + // If Gazelle was invoked by 'bazel run', the working directory is outside + // the repository, and we should use BUILD_WORKSPACE_DIRECTORY to resolve + // relative paths on the command line. We don't want to change the + // working directory, since it would interfere with runfiles lookups that + // some extensions may rely on. + wd = wsDir + } else { + var err error + if wd, err = os.Getwd(); err != nil { + return err + } + } dirs := fs.Args() if len(dirs) == 0 { dirs = []string{"."} } uc.dirs = make([]string, len(dirs)) - for i := range dirs { - dir, err := filepath.Abs(dirs[i]) - if err != nil { - return fmt.Errorf("%s: failed to find absolute path: %v", dirs[i], err) + for i, arg := range dirs { + dir := arg + if !filepath.IsAbs(dir) { + dir = filepath.Join(wd, dir) } - dir, err = filepath.EvalSymlinks(dir) + dir, err := filepath.EvalSymlinks(dir) if err != nil { - return fmt.Errorf("%s: failed to resolve symlinks: %v", dirs[i], err) + return fmt.Errorf("%s: failed to resolve symlinks: %v", arg, err) } if !isDescendingDir(dir, c.RepoRoot) { - return fmt.Errorf("dir %q is not a subdirectory of repo root %q", dir, c.RepoRoot) + return fmt.Errorf("%s: not a subdirectory of repo root %s", arg, c.RepoRoot) } uc.dirs[i] = dir } diff --git a/config/config.go b/config/config.go index 81f35b3f4..944ac23d3 100644 --- a/config/config.go +++ b/config/config.go @@ -30,6 +30,7 @@ import ( "flag" "fmt" "log" + "os" "path/filepath" "strings" @@ -196,8 +197,11 @@ func (cc *CommonConfigurer) RegisterFlags(fs *flag.FlagSet, cmd string, c *Confi func (cc *CommonConfigurer) CheckFlags(fs *flag.FlagSet, c *Config) error { var err error if cc.repoRoot == "" { - cc.repoRoot, err = wspace.FindRepoRoot(".") - if err != nil { + if wsDir := os.Getenv("BUILD_WORKSPACE_DIRECTORY"); wsDir != "" { + cc.repoRoot = wsDir + } else if parent, err := wspace.FindRepoRoot("."); err == nil { + cc.repoRoot = parent + } else { return fmt.Errorf("-repo_root not specified, and WORKSPACE cannot be found: %v", err) } } diff --git a/internal/BUILD.bazel b/internal/BUILD.bazel index 3481f4f68..b692d3753 100644 --- a/internal/BUILD.bazel +++ b/internal/BUILD.bazel @@ -1,14 +1,17 @@ load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test") -# gazelle:exclude go_repository_test.go +# gazelle:exclude *_test.go go_bazel_test( - name = "go_repository_test", - srcs = ["go_repository_test.go"], - deps = ["//testtools:go_default_library"], + name = "bazel_test", + srcs = [ + "go_repository_test.go", + "runner_test.go", + ], rule_files = [ "@bazel_gazelle//:all_files", "@io_bazel_rules_go//:all_files", ], + deps = ["//testtools:go_default_library"], ) # TODO(jayconrod): test fetch_repo error cases. diff --git a/internal/gazelle.bash.in b/internal/gazelle.bash.in index ed2659934..622195bb1 100644 --- a/internal/gazelle.bash.in +++ b/internal/gazelle.bash.in @@ -14,7 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - @@GENERATED_MESSAGE@@ set -euo pipefail @@ -41,16 +40,6 @@ function find_runfile { # printing nothing indicates failure } -# bazel_build_get_path builds a given target and prints the absolute path -# to the generated binary. This only works for rules that produce a single file. -function bazel_build_get_path { - local build_log=$(mktemp gazelle_build.XXXX.json --tmpdir) - bazel build --build_event_json_file="$build_log" "$1" - grep "^{\"id\":{\"targetCompleted\":{\"label\":\"$1\"" "$build_log" | \ - sed -e 's!^.*file://\([^"]*\).*$!\1!' - rm -f "$build_log" -} - # set_goroot attempts to set GOROOT to the SDK used by rules_go. gazelle # invokes tools inside the Go SDK for dependency management. It's good to # use the SDK used by the workspace in case the Go SDK is not installed @@ -85,35 +74,17 @@ elif [ $# -ne 0 ]; then ARGS=("$@") fi -if [ "$is_bazel_run" = true ]; then - # If the script was invoked by "bazel run", jump out of the execroot, into - # the workspace before running Gazelle. - # TODO(jayconrod): detect when a command can't be run this way. - set_goroot - gazelle_short_path=$(find_runfile "$GAZELLE_SHORT_PATH") - if [ -z "$gazelle_short_path" ]; then - echo "error: could not locate gazelle binary" >&2 - exit 1 - fi - if [ -z "${BUILD_WORKSPACE_DIRECTORY-}" ]; then - echo "error: BUILD_WORKSPACE_DIRECOTRY not set" >&2 - exit 1 - fi - cd "$BUILD_WORKSPACE_DIRECTORY" - "$gazelle_short_path" "${ARGS[@]}" -else - # If the script was invoked directly, check whether the script is out of - # date before proceeding. - new_runner_script=$(bazel_build_get_path "$RUNNER_LABEL") - if ! diff "$new_runner_script" "$0" &>/dev/null; then - cat - >&2 <&2 + exit 1 +fi +if [ -z "${BUILD_WORKSPACE_DIRECTORY-}" ]; then + echo "error: BUILD_WORKSPACE_DIRECOTRY not set" >&2 + exit 1 fi +"$gazelle_short_path" "${ARGS[@]}" diff --git a/internal/go_repository_test.go b/internal/go_repository_test.go index ab41b7d64..5fa6b998a 100644 --- a/internal/go_repository_test.go +++ b/internal/go_repository_test.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package go_repository_test +package bazel_test import ( "bytes" @@ -30,6 +30,16 @@ import ( var testArgs = bazel_testing.Args{ Main: ` -- BUILD.bazel -- +load("@bazel_gazelle//:def.bzl", "gazelle") + +# gazelle:prefix example.com/m + +gazelle(name = "gazelle") + +-- hello.go -- +package main + +func main() {} `, WorkspaceSuffix: ` load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") diff --git a/internal/runner_test.go b/internal/runner_test.go new file mode 100644 index 000000000..4daf5e91d --- /dev/null +++ b/internal/runner_test.go @@ -0,0 +1,43 @@ +/* Copyright 2020 The Bazel Authors. All rights reserved. + +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. +*/ + +package bazel_test + +import ( + "strings" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/bazel_testing" +) + +func TestRunner(t *testing.T) { + if err := bazel_testing.RunBazel("run", "//:gazelle"); err != nil { + t.Fatal(err) + } + out, err := bazel_testing.BazelOutput("query", "//:all") + if err != nil { + t.Fatal(err) + } + got := make(map[string]bool) + for _, target := range strings.Split(strings.TrimSpace(string(out)), "\n") { + got[target] = true + } + want := []string{"//:m", "//:go_default_library"} + for _, target := range want { + if !got[target] { + t.Errorf("target missing from query output: %s", target) + } + } +} diff --git a/language/go/config.go b/language/go/config.go index 74ad0f101..04c12ac22 100644 --- a/language/go/config.go +++ b/language/go/config.go @@ -116,10 +116,11 @@ type goConfig struct { // in internal packages. submodules []moduleRepo - // buildExternalAttr, buildFileNamesAttr, buildFileGenerationAttr, - // buildTagsAttr, buildFileProtoModeAttr, and buildExtraArgsAttr are - // attributes for go_repository rules, set on the command line. - buildExternalAttr, buildFileNamesAttr, buildFileGenerationAttr, buildTagsAttr, buildFileProtoModeAttr, buildExtraArgsAttr string + // buildDirectives, buildExternalAttr, buildExtraArgsAttr, + // buildFileGenerationAttr, buildFileNamesAttr, buildFileProtoModeAttr and + // buildTagsAttr are attributes for go_repository rules, set on the command + // line. + buildDirectivesAttr, buildExternalAttr, buildExtraArgsAttr, buildFileGenerationAttr, buildFileNamesAttr, buildFileProtoModeAttr, buildTagsAttr string } var ( @@ -367,6 +368,10 @@ func (*goLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) { "controls generated library names. One of (go_default_library, import, import_alias)") case "update-repos": + fs.StringVar(&gc.buildDirectivesAttr, + "build_directives", + "", + "Sets the build_directives attribute for the generated go_repository rule(s).") fs.Var(&gzflag.AllowedStringFlag{Value: &gc.buildExternalAttr, Allowed: validBuildExternalAttr}, "build_external", "Sets the build_external attribute for the generated go_repository rule(s).") diff --git a/language/go/update.go b/language/go/update.go index f6eba8340..dbddef7fd 100644 --- a/language/go/update.go +++ b/language/go/update.go @@ -89,25 +89,29 @@ func (*goLang) ImportRepos(args language.ImportReposArgs) language.ImportReposRe } func setBuildAttrs(gc *goConfig, r *rule.Rule) { + if gc.buildDirectivesAttr != "" { + buildDirectives := strings.Split(gc.buildDirectivesAttr, ",") + r.SetAttr("build_directives", buildDirectives) + } if gc.buildExternalAttr != "" { r.SetAttr("build_external", gc.buildExternalAttr) } - if gc.buildFileNamesAttr != "" { - r.SetAttr("build_file_name", gc.buildFileNamesAttr) + if gc.buildExtraArgsAttr != "" { + extraArgs := strings.Split(gc.buildExtraArgsAttr, ",") + r.SetAttr("build_extra_args", extraArgs) } if gc.buildFileGenerationAttr != "" { r.SetAttr("build_file_generation", gc.buildFileGenerationAttr) } - if gc.buildTagsAttr != "" { - buildTags := strings.Split(gc.buildTagsAttr, ",") - r.SetAttr("build_tags", buildTags) + if gc.buildFileNamesAttr != "" { + r.SetAttr("build_file_name", gc.buildFileNamesAttr) } if gc.buildFileProtoModeAttr != "" { r.SetAttr("build_file_proto_mode", gc.buildFileProtoModeAttr) } - if gc.buildExtraArgsAttr != "" { - extraArgs := strings.Split(gc.buildExtraArgsAttr, ",") - r.SetAttr("build_extra_args", extraArgs) + if gc.buildTagsAttr != "" { + buildTags := strings.Split(gc.buildTagsAttr, ",") + r.SetAttr("build_tags", buildTags) } } diff --git a/language/proto/BUILD.bazel b/language/proto/BUILD.bazel index 93f4fb929..c46bd782c 100644 --- a/language/proto/BUILD.bazel +++ b/language/proto/BUILD.bazel @@ -42,8 +42,8 @@ go_library( "fix.go", "generate.go", "kinds.go", - "known_imports.go", "known_go_imports.go", + "known_imports.go", "known_proto_imports.go", "lang.go", "package.go", @@ -102,8 +102,8 @@ filegroup( "generate.go", "generate_test.go", "kinds.go", - "known_imports.go", "known_go_imports.go", + "known_imports.go", "known_proto_imports.go", "lang.go", "package.go",