Skip to content

Commit

Permalink
nogo: stop using go_tool_library (bazel-contrib#2474)
Browse files Browse the repository at this point in the history
nogo binaries no longer depend on themselves. Since bazel-contrib#2473, the nogo
rule uses a configuration transition to disable nogo for itself and
its dependencies.

This means there's no longer any need for go_tool_library rules for
analyzers and utility packages in org_golang_x_tools. So with this
change, tools_nogo depends on the regular go_library analyzers. The
documentation is updated not to mention go_tool_library anymore.

Additionally, this change replaces the go_tool_library targets in
org_golang_x_tools with aliases to the go_library targets. So nogo
targets that depends on the old symbols should work.

Fixes bazel-contrib#2374
  • Loading branch information
Jay Conrod authored May 5, 2020
1 parent 8bdd81a commit 8e6cfa5
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 807 deletions.
9 changes: 6 additions & 3 deletions go/core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Core Go rules
.. _test_arg: https://docs.bazel.build/versions/master/user-manual.html#flag--test_arg
.. _test_filter: https://docs.bazel.build/versions/master/user-manual.html#flag--test_filter
.. _write a CROSSTOOL file: https://github.com/bazelbuild/bazel/wiki/Yet-Another-CROSSTOOL-Writing-Tutorial
.. _Deprecation schedule: https://github.com/bazelbuild/rules_go/wiki/Deprecation-schedule

.. role:: param(kbd)
.. role:: type(emphasis)
Expand Down Expand Up @@ -385,9 +386,11 @@ the same package.

This rule is a limited variant of ``go_library`` which may be used to
bootstrap tools used by rules_go. This avoids a circular dependency.
If you are building analyzers to be linked into a `nogo`_ binary, you'll
need to use ``go_tool_library`` since ``go_library`` depends on `nogo`_
implicitly.

**DEPRECATED:** This rule should no longer be used. ``go_library`` should be
used instead. This was previously needed for ``nogo`` to avoid a circular
dependency, but it's no longer necessary. See `Deprecation schedule`_ for more
information.

Providers
^^^^^^^^^
Expand Down
68 changes: 34 additions & 34 deletions go/def.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,41 +80,41 @@ load(
# This is not backward compatible, so use caution when depending on this --
# new analyses may discover issues in existing builds.
TOOLS_NOGO = [
"@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/assign:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/atomic:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/atomicalign:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/bools:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/buildssa:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/buildtag:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library",
"@org_golang_x_tools//go/analysis/passes/assign:go_default_library",
"@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
"@org_golang_x_tools//go/analysis/passes/atomicalign:go_default_library",
"@org_golang_x_tools//go/analysis/passes/bools:go_default_library",
"@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library",
"@org_golang_x_tools//go/analysis/passes/buildtag:go_default_library",
# TODO(#2396): pass raw cgo sources to cgocall and re-enable.
# "@org_golang_x_tools//go/analysis/passes/cgocall:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/composite:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/copylock:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/ctrlflow:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/deepequalerrors:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/errorsas:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/findcall:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/httpresponse:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/loopclosure:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/lostcancel:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/nilfunc:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/nilness:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/pkgfact:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/shadow:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/shift:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/sortslice:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/stdmethods:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/stringintconv:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/structtag:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/testinggoroutine:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/tests:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/unmarshal:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/unreachable:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/unsafeptr:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/unusedresult:go_tool_library",
# "@org_golang_x_tools//go/analysis/passes/cgocall:go_default_library",
"@org_golang_x_tools//go/analysis/passes/composite:go_default_library",
"@org_golang_x_tools//go/analysis/passes/copylock:go_default_library",
"@org_golang_x_tools//go/analysis/passes/ctrlflow:go_default_library",
"@org_golang_x_tools//go/analysis/passes/deepequalerrors:go_default_library",
"@org_golang_x_tools//go/analysis/passes/errorsas:go_default_library",
"@org_golang_x_tools//go/analysis/passes/findcall:go_default_library",
"@org_golang_x_tools//go/analysis/passes/httpresponse:go_default_library",
"@org_golang_x_tools//go/analysis/passes/inspect:go_default_library",
"@org_golang_x_tools//go/analysis/passes/loopclosure:go_default_library",
"@org_golang_x_tools//go/analysis/passes/lostcancel:go_default_library",
"@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library",
"@org_golang_x_tools//go/analysis/passes/nilness:go_default_library",
"@org_golang_x_tools//go/analysis/passes/pkgfact:go_default_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
"@org_golang_x_tools//go/analysis/passes/shadow:go_default_library",
"@org_golang_x_tools//go/analysis/passes/shift:go_default_library",
"@org_golang_x_tools//go/analysis/passes/sortslice:go_default_library",
"@org_golang_x_tools//go/analysis/passes/stdmethods:go_default_library",
"@org_golang_x_tools//go/analysis/passes/stringintconv:go_default_library",
"@org_golang_x_tools//go/analysis/passes/structtag:go_default_library",
"@org_golang_x_tools//go/analysis/passes/testinggoroutine:go_default_library",
"@org_golang_x_tools//go/analysis/passes/tests:go_default_library",
"@org_golang_x_tools//go/analysis/passes/unmarshal:go_default_library",
"@org_golang_x_tools//go/analysis/passes/unreachable:go_default_library",
"@org_golang_x_tools//go/analysis/passes/unsafeptr:go_default_library",
"@org_golang_x_tools//go/analysis/passes/unusedresult:go_default_library",
]

# Current version or next version to be tagged. Gazelle and other tools may
Expand Down
40 changes: 15 additions & 25 deletions go/nogo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

.. _nogo: nogo.rst#nogo
.. _go_library: core.rst#go_library
.. _go_tool_library: core.rst#go_tool_library
.. _analysis: https://godoc.org/golang.org/x/tools/go/analysis
.. _Analyzer: https://godoc.org/golang.org/x/tools/go/analysis#Analyzer
.. _GoLibrary: providers.rst#GoLibrary
Expand Down Expand Up @@ -50,16 +49,16 @@ want to run.
# analyzer from the local repository
":importunsafe",
# analyzer from a remote repository
"@org_golang_x_tools//go/analysis/passes/printf:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
],
visibility = ["//visibility:public"], # must have public visibility
)
go_tool_library(
go_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
Expand Down Expand Up @@ -97,7 +96,7 @@ the ``TOOLS_NOGO`` list of dependencies.

.. code:: bzl
load("@io_bazel_rules_go//go:def.bzl", "nogo", "TOOLS_NOGO")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo", "TOOLS_NOGO")
nogo(
name = "my_nogo",
Expand All @@ -108,11 +107,11 @@ the ``TOOLS_NOGO`` list of dependencies.
visibility = ["//visibility:public"], # must have public visibility
)
go_tool_library(
go_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
Expand Down Expand Up @@ -157,40 +156,35 @@ already been run. For example:
Any diagnostics reported by the analyzer will stop the build. Do not emit
diagnostics unless they are severe enough to warrant stopping the build.

Each analyzer must be written as a `go_tool_library`_ rule and must import
`@org_golang_x_tools//go/analysis:go_tool_library`, the `go_tool_library`_
version of the package `analysis`_ target.
Each analyzer must be written as a `go_library`_ rule and should import
`@org_golang_x_tools//go/analysis:go_default_library`, the package anaysis
framework.

For example:

.. code:: bzl
load("@io_bazel_rules_go//go:def.bzl", "go_tool_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_tool_library(
go_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "unsafedom",
srcs = [
"check_dom.go",
"dom_utils.go",
],
importpath = "unsafedom",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
**NOTE**: `go_tool_library`_ is a limited variant of ``go_library`` which avoids
a circular dependency: `go_library`_ implicitly depends on `nogo`_, which
depends on analyzer libraries, which must not depend on `nogo`_.
`go_tool_library`_ does not have the same implicit dependency.

Pass labels for these targets to the ``deps`` attribute of your `nogo`_ target,
as described in the `Setup`_ section.

Expand Down Expand Up @@ -301,7 +295,7 @@ See the full list of available nogo checks:

.. code:: shell
bazel query 'kind(go_tool_library, @org_golang_x_tools//go/analysis/passes/...)'
bazel query 'kind(go_library, @org_golang_x_tools//go/analysis/passes/...)'
API
Expand Down Expand Up @@ -330,10 +324,6 @@ Attributes
| |
| These libraries must declare an ``analysis.Analyzer`` variable named `Analyzer` to ensure that |
| the analyzers they implement are called by nogo. |
| |
| To avoid bootstrapping problems, these libraries must be `go_tool_library`_ targets, and must |
| import `@org_golang_x_tools//go/analysis:go_tool_library`, the `go_tool_library`_ version of |
| the package `analysis`_ target. |
+----------------------------+-----------------------------+---------------------------------------+
| :param:`config` | :type:`label` | :value:`None` |
+----------------------------+-----------------------------+---------------------------------------+
Expand Down
14 changes: 6 additions & 8 deletions go/private/rules/nogo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,7 @@ nogo = rule(
"_nogo_srcs": attr.label(
default = "@io_bazel_rules_go//go/tools/builders:nogo_srcs",
),
"_cgo_context_data": attr.label(default = "//:cgo_context_data_proxy"),
"_go_config": attr.label(default = "//:go_config"),
"_stdlib": attr.label(default = "//:stdlib"),
"_go_context_data": attr.label(default = "//:go_context_data"),
"_whitelist_function_transition": attr.label(
default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
),
Expand All @@ -151,11 +149,11 @@ nogo = rule(
def nogo_wrapper(**kwargs):
if kwargs.get("vet"):
kwargs["deps"] = kwargs.get("deps", []) + [
"@org_golang_x_tools//go/analysis/passes/atomic:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/bools:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/buildtag:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/nilfunc:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_tool_library",
"@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
"@org_golang_x_tools//go/analysis/passes/bools:go_default_library",
"@org_golang_x_tools//go/analysis/passes/buildtag:go_default_library",
"@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
]
kwargs = {k: v for k, v in kwargs.items() if k != "vet"}
nogo(**kwargs)
Expand Down
11 changes: 3 additions & 8 deletions go/tools/builders/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,12 @@ go_source(
"flags.go",
"nogo_main.go",
],
# //go/tools/builders:nogo_srcs is considered a different target by
# Bazel's visibility check than
# @io_bazel_rules_go//go/tools/builders:nogo_srcs. Only the latter is
# allowed to depend on
# @org_golang_x_tools//go/analysis/internal/facts:go_tool_library.
tags = ["manual"],
visibility = ["//visibility:public"],
deps = [
"@org_golang_x_tools//go/analysis:go_tool_library",
"@org_golang_x_tools//go/analysis/internal/facts:go_tool_library",
"@org_golang_x_tools//go/gcexportdata:go_tool_library",
"@org_golang_x_tools//go/analysis:go_default_library",
"@org_golang_x_tools//go/analysis/internal/facts:go_default_library",
"@org_golang_x_tools//go/gcexportdata:go_default_library",
],
)

Expand Down
4 changes: 2 additions & 2 deletions tests/core/nogo/coverage/coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_test", "go_tool_library", "nogo")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "nogo")
go_test(
name = "coverage_target",
srcs = ["coverage_target_test.go"],
deps = [":coverage_target_dep"],
)
go_tool_library(
go_library(
name = "coverage_target_dep",
importmap = "mapped/coverage_target/dep",
importpath = "coverage_target/dep",
Expand Down
16 changes: 8 additions & 8 deletions tests/core/nogo/custom/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestMain(m *testing.M) {
Nogo: "@//:nogo",
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_tool_library", "nogo")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo")
nogo(
name = "nogo",
Expand All @@ -44,29 +44,29 @@ nogo(
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "importfmt",
srcs = ["importfmt.go"],
importpath = "importfmtanalyzer",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "foofuncname",
srcs = ["foofuncname.go"],
importpath = "foofuncanalyzer",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "visibility",
srcs = ["visibility.go"],
importpath = "visibilityanalyzer",
deps = [
"@org_golang_x_tools//go/analysis:go_tool_library",
"@org_golang_x_tools//go/ast/inspector:go_tool_library",
"@org_golang_x_tools//go/analysis:go_default_library",
"@org_golang_x_tools//go/ast/inspector:go_default_library",
],
visibility = ["//visibility:public"],
)
Expand Down
18 changes: 9 additions & 9 deletions tests/core/nogo/deps/deps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestMain(m *testing.M) {
Nogo: "@//:nogo",
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_tool_library", "nogo")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo")
nogo(
name = "nogo",
Expand All @@ -39,44 +39,44 @@ nogo(
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "a",
srcs = ["a.go"],
importpath = "a",
deps = [
":c",
"@org_golang_x_tools//go/analysis:go_tool_library"
"@org_golang_x_tools//go/analysis:go_default_library"
],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "b",
srcs = ["b.go"],
importpath = "b",
deps = [
":c",
"@org_golang_x_tools//go/analysis:go_tool_library"
"@org_golang_x_tools//go/analysis:go_default_library"
],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "c",
srcs = ["c.go"],
importpath = "c",
deps = [
":d",
"@org_golang_x_tools//go/analysis:go_tool_library"
"@org_golang_x_tools//go/analysis:go_default_library"
],
visibility = ["//visibility:public"],
)
go_tool_library(
go_library(
name = "d",
srcs = ["d.go"],
importpath = "d",
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
visibility = ["//visibility:public"],
)
Expand Down
Loading

0 comments on commit 8e6cfa5

Please sign in to comment.