Merge branch 'master' into go_naming_convention

bazel-contrib · Jul 17, 2020 · 5a1e4db · 5a1e4db
2 parents 3dc0f77 + 58e5e2d
commit 5a1e4db
Show file tree

Hide file tree

Showing 12 changed files with 141 additions and 83 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <<EOF
-error: $0: script is out of date. Refresh it with the command:
-  bazel build $RUNNER_LABEL && cp -fv "$new_runner_script" "$0"
-EOF
-    exit 1
-  fi
-
-  # Rebuild and run Gazelle.
-  gazelle_exe=$(bazel_build_get_path "$GAZELLE_LABEL")
-  "$gazelle_exe" "${ARGS[@]}"
+# 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
+"$gazelle_short_path" "${ARGS[@]}"
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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",