nogo: migrate to configuration transitions (bazelbuild#2473)

The nogo binary to use is now specified using a Starlark flag.
For example:

    bazel build --@io_bazel_rules_go//go/config:nogo=@io_bazel_rules_go//:tools_nogo //:hello

.bazelrc is now the recommended way to set this flag. Setting nogo in
go_register_toolchains will still work, though that function may be
deprecated soon. The flag overrides the go_register_toolchains
argument when both are set.

Updates bazelbuild#2374
Fixes bazelbuild#2470

.bazelci/presubmit.yml

@@ -316,7 +316,9 @@ platforms:
     - "-//tests/core/go_proto_library:transitive_test"
     - "-//tests/core/go_test:data_test"
     - "-//tests/core/go_test:pwd_test"
+    - "-//tests/core/nogo/config:config_test"
     - "-//tests/core/nogo/coverage:coverage_test"
+    - "-//tests/core/nogo/flag:flag_test"
     - "-//tests/core/nogo/vet:vet_test"
     - "-//tests/core/stdlib:buildid_test"
     - "-//tests/examples/executable_name:executable_name"

BUILD.bazel

@@ -4,6 +4,7 @@ load(
 )
 load(
     "@io_bazel_rules_go//go/private:rules/nogo.bzl",
+    "default_nogo",
     "nogo",
 )
 load(
@@ -38,7 +39,7 @@ stdlib(
 # default_nogo is the nogo target that nogo references by default. It
 # does not analyze anything, which means no binary is built or run
 # at compile time.
-nogo(
+default_nogo(
     name = "default_nogo",
     visibility = ["//visibility:public"],
 )
@@ -53,6 +54,18 @@ nogo(
     deps = TOOLS_NOGO,
 )
 
+# nogo_alias points to the nogo flag which points to the active nogo binary
+# when nogo is enabled.
+# TODO(bazelbuild/bazel#11291): This exists to avoid a dependency cycle, but
+# the cycle shouldn't exist when transitions are taken into account.
+alias(
+    name = "nogo_alias",
+    actual = select({
+        "//go/private:need_nogo": "//go/config:nogo",
+        "//conditions:default": "//:default_nogo",
+    }),
+)
+
 # go_context_data collects build options and is depended on by all Go targets.
 # It may depend on cgo_context_data if CGo isn't disabled.
 go_context_data(
@@ -63,7 +76,7 @@ go_context_data(
     }),
     coverdata = "//go/tools/coverdata",
     go_config = ":go_config",
-    nogo = "@io_bazel_rules_nogo//:nogo",
+    nogo = ":nogo_alias",
     stdlib = ":stdlib",
     visibility = ["//visibility:public"],
 )

go/config/BUILD.bazel

@@ -58,6 +58,12 @@ string_list_flag(
     visibility = ["//visibility:public"],
 )
 
+label_flag(
+    name = "nogo",
+    build_setting_default = "@io_bazel_rules_nogo//:nogo",
+    visibility = ["//visibility:public"],
+)
+
 filegroup(
     name = "all_files",
     testonly = True,

go/nogo.rst

@@ -19,10 +19,6 @@
 .. footer:: The ``nogo`` logo was derived from the Go gopher, which was designed by Renee French. (http://reneefrench.blogspot.com/) The design is licensed under the Creative Commons 3.0 Attributions license. Read this article for more details: http://blog.golang.org/gopher
 
 
-**WARNING**: This functionality is experimental, so its API might change.
-Please do not rely on it for production use, but feel free to use it and file
-issues.
-
 ``nogo`` is a tool that analyzes the source code of Go programs. It runs
 alongside the Go compiler in the Bazel Go rules and rejects programs that
 contain disallowed coding patterns. In addition, ``nogo`` may report
@@ -67,17 +63,19 @@ want to run.
         visibility = ["//visibility:public"],
     )
 
-Pass a label for your `nogo`_ target to ``go_register_toolchains`` in your
-``WORKSPACE`` file.
+To build with this ``nogo`` target, use the
+``--@io_bazel_rules_go//go/config:nogo`` command line flag.
 
-.. code:: bzl
+.. code::
+
+    bazel build --@io_bazel_rules_go//go/config:nogo=//:my_nogo //:my_binary
 
-    load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains")
-    go_rules_dependencies()
-    go_register_toolchains(nogo = "@//:my_nogo") # my_nogo is in the top-level BUILD file of this workspace
+For convenience, you can add this to your workspace's ``.bazelrc`` file to
+avoid having to type this out for every command.
 
-**NOTE**: You must include ``"@//"`` prefix when referring to targets in the local
-workspace.
+.. code::
+
+    build --@io_bazel_rules_go//go/config:nogo=//:my_nogo
 
 The `nogo`_ rule will generate a program that executes all the supplied
 analyzers at build-time. The generated ``nogo`` program will run alongside the
@@ -86,13 +84,13 @@ even if the target is imported from an external repository. However, ``nogo``
 will not run when targets from the current repository are imported into other
 workspaces and built there.
 
-To run all the ``golang.org/x/tools`` analyzers, use ``@io_bazel_rules_go//:tools_nogo``.
+The target ``@io_bazel_rules_go//:tools_nogo`` contains the analyzers from
+``golang.org/x/tools``. This is the same set of analyzers that ``go vet`` uses.
+You can use this instead of declaring your own ``nogo`` target.
 
-.. code:: bzl
+.. code::
 
-    load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains")
-    go_rules_dependencies()
-    go_register_toolchains(nogo = "@io_bazel_rules_go//:tools_nogo")
+    build --@io_bazel_rules_go//go/config:nogo=@io_bazel_rules_go//:tools_nogo
 
 To run the analyzers from ``tools_nogo`` together with your own analyzers, use
 the ``TOOLS_NOGO`` list of dependencies.

go/private/BUILD.bazel

@@ -1,3 +1,5 @@
+load("@bazel_skylib//rules:common_settings.bzl", "bool_setting")
+
 filegroup(
     name = "all_rules",
     srcs = glob(["**/*.bzl"]),
@@ -30,3 +32,21 @@ config_setting(
     name = "stamp",
     values = {"stamp": "true"},
 )
+
+# need_nogo_flag is set to false by nogo's incoming edge transition. It's used
+# to control whether //:nogo_alias depends on //go/config:nogo to break
+# a dependency cycle.
+# TODO(bazelbuild/bazel#11291): the cycle shouldn't exist if transitions
+# are taken into account, so this should be removed at some point.
+bool_setting(
+    name = "need_nogo_flag",
+    build_setting_default = True,
+)
+
+config_setting(
+    name = "need_nogo",
+    flag_values = {
+        ":need_nogo_flag": "True",
+    },
+    visibility = ["//:__pkg__"],
+)

go/private/context.bzl

@@ -493,7 +493,12 @@ def go_context(ctx, attr = None):
 
 def _go_context_data_impl(ctx):
     coverdata = ctx.attr.coverdata[GoArchive]
-    nogo = ctx.files.nogo[0] if ctx.files.nogo else None
+    nogo = None
+    if ctx.attr.nogo and DefaultInfo in ctx.attr.nogo:
+        # TODO: may need multiple files and runfiles.
+        nogo_files = ctx.attr.nogo[DefaultInfo].files.to_list()
+        if nogo_files:
+            nogo = nogo_files[0]
     providers = [
         GoContextInfo(
             coverdata = ctx.attr.coverdata[GoArchive],
@@ -519,7 +524,6 @@ go_context_data = rule(
             providers = [GoConfigInfo],
         ),
         "nogo": attr.label(
-            mandatory = True,
             cfg = "exec",
         ),
         "stdlib": attr.label(

go/private/nogo.bzl

@@ -12,8 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-DEFAULT_NOGO = "@io_bazel_rules_go//:default_nogo"
-
 def _go_register_nogo_impl(ctx):
     ctx.template(
         "BUILD.bazel",
@@ -31,6 +29,6 @@ def _go_register_nogo_impl(ctx):
 go_register_nogo = repository_rule(
     _go_register_nogo_impl,
     attrs = {
-        "nogo": attr.string(mandatory = True),
+        "nogo": attr.string(default = "@io_bazel_rules_go//:default_nogo"),
     },
 )

go/private/repositories.bzl

@@ -16,7 +16,7 @@
 
 load("@io_bazel_rules_go//go/private:common.bzl", "MINIMUM_BAZEL_VERSION")
 load("@io_bazel_rules_go//go/private:skylib/lib/versions.bzl", "versions")
-load("@io_bazel_rules_go//go/private:nogo.bzl", "DEFAULT_NOGO", "go_register_nogo")
+load("@io_bazel_rules_go//go/private:nogo.bzl", "go_register_nogo")
 load("@io_bazel_rules_go//proto:gogo.bzl", "gogo_special_proto")
 load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
@@ -252,7 +252,6 @@ def go_rules_dependencies(is_rules_go = False):
     _maybe(
         go_register_nogo,
         name = "io_bazel_rules_nogo",
-        nogo = DEFAULT_NOGO,
     )
 
     go_name_hack(

go/private/rules/nogo.bzl

@@ -16,13 +16,59 @@ load(
     "@io_bazel_rules_go//go/private:context.bzl",
     "go_context",
 )
+load(
+    "@io_bazel_rules_go//go/private:mode.bzl",
+    "LINKMODE_NORMAL",
+)
 load(
     "@io_bazel_rules_go//go/private:providers.bzl",
     "EXPORT_PATH",
     "GoArchive",
     "GoLibrary",
     "get_archive",
 )
+load(
+    "@io_bazel_rules_go//go/private:rules/transition.bzl",
+    "filter_transition_label",
+)
+
+_nogo_transition_dict = {
+    "@io_bazel_rules_go//go/config:static": False,
+    "@io_bazel_rules_go//go/config:msan": False,
+    "@io_bazel_rules_go//go/config:race": False,
+    "@io_bazel_rules_go//go/config:pure": False,
+    "@io_bazel_rules_go//go/config:strip": False,
+    "@io_bazel_rules_go//go/config:debug": False,
+    "@io_bazel_rules_go//go/config:linkmode": LINKMODE_NORMAL,
+    "@io_bazel_rules_go//go/config:tags": [],
+    "@io_bazel_rules_go//go/private:need_nogo_flag": False,
+}
+
+_nogo_transition_keys = sorted([filter_transition_label(label) for label in _nogo_transition_dict.keys()])
+
+def _nogo_transition_impl(settings, attr):
+    """Ensures nogo is built in a safe configuration without a dependency cycle.
+
+    nogo_transition sets all of the //go/config settings to their default
+    values. The nogo binary shouldn't depend on the link mode or tags of the
+    binary being checked. This transition doesn't explicitly change the
+    platform (goos, goarch), but nogo dependencies should have `cfg = "exec"`,
+    so nogo binaries should be built for the execution platform.
+    """
+    # TODO(bazelbuild/bazel#11291): When this bug is resolved, instead of
+    # setting //go/private:need_nogo_flag, we should set //go/config:nogo to
+    # //:default_nogo.
+
+    settings = dict(settings)
+    for label, value in _nogo_transition_dict.items():
+        settings[filter_transition_label(label)] = value
+    return settings
+
+nogo_transition = transition(
+    implementation = _nogo_transition_impl,
+    inputs = _nogo_transition_keys,
+    outputs = _nogo_transition_keys,
+)
 
 def _nogo_impl(ctx):
     if not ctx.attr.deps:
@@ -94,8 +140,12 @@ nogo = rule(
         "_cgo_context_data": attr.label(default = "//:cgo_context_data_proxy"),
         "_go_config": attr.label(default = "//:go_config"),
         "_stdlib": attr.label(default = "//:stdlib"),
+        "_whitelist_function_transition": attr.label(
+            default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
+        ),
     },
     toolchains = ["@io_bazel_rules_go//go:toolchain"],
+    cfg = nogo_transition,
 )
 
 def nogo_wrapper(**kwargs):
@@ -109,3 +159,10 @@ def nogo_wrapper(**kwargs):
         ]
         kwargs = {k: v for k, v in kwargs.items() if k != "vet"}
     nogo(**kwargs)
+
+def _default_nogo_impl(ctx):
+    pass
+
+default_nogo = rule(
+    implementation = _default_nogo_impl,
+)

go/private/rules/transition.bzl

@@ -26,7 +26,7 @@ load(
     "IS_RULES_GO",
 )
 
-def _filter_transition_label(label):
+def filter_transition_label(label):
     """Transforms transition labels for the current workspace.
 
     This is a workaround for bazelbuild/bazel#10499. If a transition refers to
@@ -120,7 +120,7 @@ def _go_transition_impl(settings, attr):
         platform = "@io_bazel_rules_go//go/toolchain:{}_{}{}".format(goos, goarch, "_cgo" if cgo else "")
         settings["//command_line_option:platforms"] = platform
     if pure != "auto":
-        pure_label = _filter_transition_label("@io_bazel_rules_go//go/config:pure")
+        pure_label = filter_transition_label("@io_bazel_rules_go//go/config:pure")
         settings[pure_label] = pure == "on"
 
     _set_ternary(settings, attr, "static")
@@ -129,21 +129,21 @@ def _go_transition_impl(settings, attr):
 
     tags = getattr(attr, "gotags", [])
     if tags:
-        tags_label = _filter_transition_label("@io_bazel_rules_go//go/config:tags")
+        tags_label = filter_transition_label("@io_bazel_rules_go//go/config:tags")
         settings[tags_label] = tags
 
     linkmode = getattr(attr, "linkmode", "auto")
     if linkmode != "auto":
         if linkmode not in LINKMODES:
             fail("linkmode: invalid mode {}; want one of {}".format(linkmode, ", ".join(LINKMODES)))
-        linkmode_label = _filter_transition_label("@io_bazel_rules_go//go/config:linkmode")
+        linkmode_label = filter_transition_label("@io_bazel_rules_go//go/config:linkmode")
         settings[linkmode_label] = linkmode
 
     return settings
 
 go_transition = transition(
     implementation = _go_transition_impl,
-    inputs = [_filter_transition_label(label) for label in [
+    inputs = [filter_transition_label(label) for label in [
         "//command_line_option:platforms",
         "@io_bazel_rules_go//go/config:static",
         "@io_bazel_rules_go//go/config:msan",
@@ -152,7 +152,7 @@ go_transition = transition(
         "@io_bazel_rules_go//go/config:tags",
         "@io_bazel_rules_go//go/config:linkmode",
     ]],
-    outputs = [_filter_transition_label(label) for label in [
+    outputs = [filter_transition_label(label) for label in [
         "//command_line_option:platforms",
         "@io_bazel_rules_go//go/config:static",
         "@io_bazel_rules_go//go/config:msan",
@@ -171,5 +171,5 @@ def _set_ternary(settings, attr, name):
     value = getattr(attr, name, "auto")
     _check_ternary(name, value)
     if value != "auto":
-        label = _filter_transition_label("@io_bazel_rules_go//go/config:{}".format(name))
+        label = filter_transition_label("@io_bazel_rules_go//go/config:{}".format(name))
         settings[label] = value == "on"

tests/core/nogo/README.rst

@@ -9,6 +9,7 @@ Contents
 .. Child list start
 
 * `Vet check <vet/README.rst>`_
+* `Nogo flags <flag/README.rst>`_
 * `Nogo configuration <config/README.rst>`_
 * `nogo analyzers with dependencies <deps/README.rst>`_
 * `Custom nogo analyzers <custom/README.rst>`_

tests/core/nogo/config/README.rst

@@ -3,6 +3,8 @@ Nogo configuration
 
 .. _nogo: /go/nogo.rst
 .. _go_binary: /go/core.rst#_go_binary
+.. _#1850: https://github.com/bazelbuild/rules_go/issues/1850
+.. _#2470: https://github.com/bazelbuild/rules_go/issues/2470
 
 Tests that verify nogo_ works on targets compiled in non-default configurations.
 
@@ -12,4 +14,4 @@ config_test
 -----------
 
 Verifies that a `go_binary`_ can be built in non-default configurations with
-nogo. Verifies #1850.
+nogo. Verifies `#1850`_, `#2470`_.