Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert changes migrating nogo to configuration transitions #2481

Merged
merged 2 commits into from
May 8, 2020
Merged

Revert changes migrating nogo to configuration transitions #2481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a614d6
Revert "nogo: stop using go_tool_library (#2474)"
May 8, 2020
a4bd0ab
Revert "nogo: migrate to configuration transitions (#2473)"
May 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .bazelci/presubmit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,7 @@ 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"
Expand Down
17 changes: 2 additions & 15 deletions BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ load(
)
load(
"@io_bazel_rules_go//go/private:rules/nogo.bzl",
"default_nogo",
"nogo",
)
load(
Expand Down Expand Up @@ -39,7 +38,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.
default_nogo(
nogo(
name = "default_nogo",
visibility = ["//visibility:public"],
)
Expand All @@ -54,18 +53,6 @@ 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(
Expand All @@ -76,7 +63,7 @@ go_context_data(
}),
coverdata = "//go/tools/coverdata",
go_config = ":go_config",
nogo = ":nogo_alias",
nogo = "@io_bazel_rules_nogo//:nogo",
stdlib = ":stdlib",
visibility = ["//visibility:public"],
)
Expand Down
6 changes: 0 additions & 6 deletions go/config/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ 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,
Expand Down
9 changes: 3 additions & 6 deletions go/core.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ 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 @@ -386,11 +385,9 @@ 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.

**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.
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.

Providers
^^^^^^^^^
Expand Down
68 changes: 34 additions & 34 deletions go/def.bzl
View file
Open in desktop
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_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",
"@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",
# TODO(#2396): pass raw cgo sources to cgocall and re-enable.
# "@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",
# "@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",
]

# Current version or next version to be tagged. Gazelle and other tools may
Expand Down
72 changes: 42 additions & 30 deletions go/nogo.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

.. _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 All @@ -18,6 +19,10 @@
.. 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
Expand Down Expand Up @@ -49,32 +54,30 @@ want to run.
# analyzer from the local repository
":importunsafe",
# analyzer from a remote repository
"@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
"@org_golang_x_tools//go/analysis/passes/printf:go_tool_library",
],
visibility = ["//visibility:public"], # must have public visibility
)

go_library(
go_tool_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
visibility = ["//visibility:public"],
)

To build with this ``nogo`` target, use the
``--@io_bazel_rules_go//go/config:nogo`` command line flag.

.. code::
Pass a label for your `nogo`_ target to ``go_register_toolchains`` in your
``WORKSPACE`` file.

bazel build --@io_bazel_rules_go//go/config:nogo=//:my_nogo //:my_binary

For convenience, you can add this to your workspace's ``.bazelrc`` file to
avoid having to type this out for every command.
.. code:: bzl

.. code::
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

build --@io_bazel_rules_go//go/config:nogo=//:my_nogo
**NOTE**: You must include ``"@//"`` prefix when referring to targets in the local
workspace.

The `nogo`_ rule will generate a program that executes all the supplied
analyzers at build-time. The generated ``nogo`` program will run alongside the
Expand All @@ -83,20 +86,20 @@ 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.

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.
To run all the ``golang.org/x/tools`` analyzers, use ``@io_bazel_rules_go//:tools_nogo``.

.. code::
.. code:: bzl

build --@io_bazel_rules_go//go/config:nogo=@io_bazel_rules_go//:tools_nogo
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")

To run the analyzers from ``tools_nogo`` together with your own analyzers, use
the ``TOOLS_NOGO`` list of dependencies.

.. code:: bzl

load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo", "TOOLS_NOGO")
load("@io_bazel_rules_go//go:def.bzl", "nogo", "TOOLS_NOGO")

nogo(
name = "my_nogo",
Expand All @@ -107,11 +110,11 @@ the ``TOOLS_NOGO`` list of dependencies.
visibility = ["//visibility:public"], # must have public visibility
)

go_library(
go_tool_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
visibility = ["//visibility:public"],
)

Expand Down Expand Up @@ -156,35 +159,40 @@ 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_library`_ rule and should import
`@org_golang_x_tools//go/analysis:go_default_library`, the package anaysis
framework.
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.

For example:

.. code:: bzl

load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_tool_library")

go_library(
go_tool_library(
name = "importunsafe",
srcs = ["importunsafe.go"],
importpath = "importunsafe",
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
deps = ["@org_golang_x_tools//go/analysis:go_tool_library"],
visibility = ["//visibility:public"],
)

go_library(
go_tool_library(
name = "unsafedom",
srcs = [
"check_dom.go",
"dom_utils.go",
],
importpath = "unsafedom",
deps = ["@org_golang_x_tools//go/analysis:go_default_library"],
deps = ["@org_golang_x_tools//go/analysis:go_tool_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 @@ -295,7 +303,7 @@ See the full list of available nogo checks:

.. code:: shell

bazel query 'kind(go_library, @org_golang_x_tools//go/analysis/passes/...)'
bazel query 'kind(go_tool_library, @org_golang_x_tools//go/analysis/passes/...)'


API
Expand Down Expand Up @@ -324,6 +332,10 @@ 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
20 changes: 0 additions & 20 deletions go/private/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
load("@bazel_skylib//rules:common_settings.bzl", "bool_setting")

filegroup(
name = "all_rules",
srcs = glob(["**/*.bzl"]),
Expand Down Expand Up @@ -32,21 +30,3 @@ 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__"],
)
8 changes: 2 additions & 6 deletions go/private/context.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,7 @@ def go_context(ctx, attr = None):

def _go_context_data_impl(ctx):
coverdata = ctx.attr.coverdata[GoArchive]
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]
nogo = ctx.files.nogo[0] if ctx.files.nogo else None
providers = [
GoContextInfo(
coverdata = ctx.attr.coverdata[GoArchive],
Expand All @@ -524,6 +519,7 @@ go_context_data = rule(
providers = [GoConfigInfo],
),
"nogo": attr.label(
mandatory = True,
cfg = "exec",
),
"stdlib": attr.label(
Expand Down
Loading