Skip to content

Commit

Permalink
nogo: Create a go_register_nogo wrapper for WORKSPACE users. (#3842)
Browse files Browse the repository at this point in the history
* nogo: Create a go_register_nogo wrapper for WORKSPACE users.

Currently, workspace users register nogo targets by abusing the
go_register_toolchain() function, which calls go_register_nogo().
Instead, we should expose go_register_nogo to workspace users directly.
This will allow workspace users and bzlmod users to use the same
underlying code because go_register_nogo allows users to specify
includes and excludes which you currently can't do with WORKSPACE.

Add a test to verify this functionality works in WORKSPACE too.

* Exclude running on bazel 5.4.0
  • Loading branch information
DolceTriade authored Feb 8, 2024
1 parent fe5c2e2 commit a9b312a
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 23 deletions.
2 changes: 2 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ tasks:
- "-//tests/core/starlark/cgo/..." # Doesn't work before bazel 6
# Bzlmod tests require Bazel 6+
- "-//tests/core/nogo/bzlmod/..."
# Nogo includes/excludes doesn't work before bazel 6
- "-//tests/core/nogo/includes_excludes:includes_exclude_test"
ubuntu2004:
# enable some unflipped incompatible flags on this platform to ensure we don't regress.
shell_commands:
Expand Down
5 changes: 5 additions & 0 deletions go/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,15 @@ load(
_go_register_toolchains = "go_register_toolchains",
_go_wrap_sdk = "go_wrap_sdk",
)
load(
"//go/private:nogo.bzl",
"go_register_nogo_wrapper",
)

go_rules_dependencies = _go_rules_dependencies
go_register_toolchains = _go_register_toolchains
go_download_sdk = _go_download_sdk
go_host_sdk = _go_host_sdk
go_local_sdk = _go_local_sdk
go_wrap_sdk = _go_wrap_sdk
go_register_nogo = go_register_nogo_wrapper
14 changes: 10 additions & 4 deletions go/nogo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,24 @@ want to run.
visibility = ["//visibility:public"],
)
Pass a label for your `nogo`_ target to ``go_register_toolchains`` in your
Pass a label for your `nogo`_ target to ``go_register_nogo`` in your
``WORKSPACE`` file. When using ``MODULE.bazel``, see the Bzlmod_ documentation
instead.

.. code:: bzl
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains")
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_nogo")
go_rules_dependencies()
go_register_toolchains(nogo = "@//:my_nogo") # my_nogo is in the top-level BUILD file of this workspace
go_register_toolchains(version = "1.20.7")
go_register_nogo(
nogo = "@//:my_nogo" # my_nogo is in the top-level BUILD file of this workspace
includes = ["@//:__subpackages__"], # Labels to lint. By default only lints code in workspace.
excludes = ["@//generated:__subpackages__"], # Labels to exclude.
)
**NOTE**: You must include ``"@//"`` prefix when referring to targets in the local
workspace.
workspace. Also note that you cannot use this to refer to bzlmod repos, as the labels
don't go though repo mapping.

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 Down
13 changes: 5 additions & 8 deletions go/private/extensions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

load("@io_bazel_rules_go_bazel_features//:features.bzl", "bazel_features")
load("//go/private:sdk.bzl", "detect_host_platform", "go_download_sdk_rule", "go_host_sdk_rule", "go_multiple_toolchains")
load("//go/private:nogo.bzl", "DEFAULT_NOGO", "go_register_nogo")
load("//go/private:nogo.bzl", "DEFAULT_NOGO", "NOGO_DEFAULT_EXCLUDES", "NOGO_DEFAULT_INCLUDES", "go_register_nogo")

def host_compatible_toolchain_impl(ctx):
ctx.file("BUILD.bazel")
Expand Down Expand Up @@ -67,16 +67,13 @@ _host_tag = tag_class(
},
)

_NOGO_DEFAULT_INCLUDES = ["@@//:__subpackages__"]
_NOGO_DEFAULT_EXCLUDES = []

_nogo_tag = tag_class(
attrs = {
"nogo": attr.label(
doc = "The nogo target to use when this module is the root module.",
),
"includes": attr.label_list(
default = _NOGO_DEFAULT_INCLUDES,
default = NOGO_DEFAULT_INCLUDES,
# The special include "all" is undocumented on purpose: With it, adding a new transitive
# dependency to a Go module can cause a build failure if the new dependency has lint
# issues.
Expand All @@ -90,7 +87,7 @@ Uses the same format as 'visibility', i.e., every entry must be a label that end
""",
),
"excludes": attr.label_list(
default = _NOGO_DEFAULT_EXCLUDES,
default = NOGO_DEFAULT_EXCLUDES,
doc = "See 'includes'.",
),
},
Expand All @@ -116,8 +113,8 @@ _TOOLCHAIN_INDEX_PAD_LENGTH = len(str(_MAX_NUM_TOOLCHAINS))
def _go_sdk_impl(ctx):
nogo_tag = struct(
nogo = DEFAULT_NOGO,
includes = _NOGO_DEFAULT_INCLUDES,
excludes = _NOGO_DEFAULT_EXCLUDES,
includes = NOGO_DEFAULT_INCLUDES,
excludes = NOGO_DEFAULT_EXCLUDES,
)
for module in ctx.modules:
if not module.is_root or not module.tags.nogo:
Expand Down
11 changes: 11 additions & 0 deletions go/private/nogo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# limitations under the License.

DEFAULT_NOGO = "@io_bazel_rules_go//:default_nogo"
NOGO_DEFAULT_INCLUDES = ["@@//:__subpackages__"]
NOGO_DEFAULT_EXCLUDES = []

# repr(Label(...)) does not emit a canonical label literal.
def _label_repr(label):
Expand Down Expand Up @@ -59,3 +61,12 @@ go_register_nogo = repository_rule(
"excludes": attr.string_list(),
},
)

def go_register_nogo_wrapper(nogo, includes = NOGO_DEFAULT_INCLUDES, excludes = NOGO_DEFAULT_EXCLUDES):
"""See go/nogo.rst"""
go_register_nogo(
name = "io_bazel_rules_nogo",
nogo = nogo,
includes = includes,
excludes = excludes,
)
42 changes: 38 additions & 4 deletions go/tools/bazel_testing/bazel_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ type Args struct {
// nogo is not used.
Nogo string

// NogoIncludes is the list of targets to include for Nogo linting.
NogoIncludes []string

// NogoExcludes is the list of targets to include for Nogo linting.
NogoExcludes []string

// WorkspaceSuffix is a string that should be appended to the end
// of the default generated WORKSPACE file.
WorkspaceSuffix string
Expand Down Expand Up @@ -401,8 +407,10 @@ func setupWorkspace(args Args, files []string) (dir string, cleanup func() error
}
}()
info := workspaceTemplateInfo{
Suffix: args.WorkspaceSuffix,
Nogo: args.Nogo,
Suffix: args.WorkspaceSuffix,
Nogo: args.Nogo,
NogoIncludes: args.NogoIncludes,
NogoExcludes: args.NogoExcludes,
}
for name := range workspaceNames {
info.WorkspaceNames = append(info.WorkspaceNames, name)
Expand Down Expand Up @@ -526,6 +534,8 @@ type workspaceTemplateInfo struct {
WorkspaceNames []string
GoSDKPath string
Nogo string
NogoIncludes []string
NogoExcludes []string
Suffix string
}

Expand All @@ -549,7 +559,7 @@ local_repository(
path = "{{.GoSDKPath}}",
)
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains", "go_wrap_sdk")
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains", "go_wrap_sdk", "go_register_nogo")
go_rules_dependencies()
Expand All @@ -558,7 +568,31 @@ go_wrap_sdk(
root_file = "@local_go_sdk//:ROOT",
)
go_register_toolchains({{if .Nogo}}nogo = "{{.Nogo}}"{{end}})
go_register_toolchains()
{{if .Nogo}}
go_register_nogo(
nogo = "{{.Nogo}}",
{{ if .NogoIncludes }}
includes = [
{{range .NogoIncludes }}
"{{ . }}",
{{ end }}
],
{{ else }}
includes = ["all"],
{{ end}}
{{ if .NogoExcludes }}
excludes = [
{{range .NogoExcludes }}
"{{ . }}",
{{ end }}
],
{{ else }}
excludes = None,
{{ end}}
)
{{end}}
{{end}}
{{.Suffix}}
`))
Expand Down
6 changes: 6 additions & 0 deletions tests/core/nogo/includes_excludes/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test")

go_bazel_test(
name = "includes_exclude_test",
srcs = ["includes_excludes_test.go"],
)
14 changes: 14 additions & 0 deletions tests/core/nogo/includes_excludes/README.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Nogo excludes-includes configuration
==================

.. _nogo: /go/nogo.rst

Tests that verify nogo_ `includes` and `excludes` works when configured from ``WORKSPACE.bazel``.

.. contents::

includes_excludes_test
-----------

Verifies that `go_library`_ targets can be built in default configurations with
nogo with includes and excludes being honored.
120 changes: 120 additions & 0 deletions tests/core/nogo/includes_excludes/includes_excludes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Copyright 2019 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 includes_excludes_test

import (
"strings"
"testing"

"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
)

func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Nogo: "@//:my_nogo",
NogoIncludes: []string{"@//go:__subpackages__"},
NogoExcludes: []string{"@//go/third_party:__subpackages__"},
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo", "TOOLS_NOGO")
nogo(
name = "my_nogo",
visibility = ["//visibility:public"],
deps = TOOLS_NOGO,
)
go_library(
name = "lib",
srcs = ["lib.go"],
importpath = "example.com/lib",
)
-- lib.go --
package lib
func shadowed() string {
foo := "original"
if foo == "original" {
foo := "shadow"
return foo
}
return foo
}
-- go/BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "lib",
srcs = ["lib.go"],
importpath = "example.com/go/lib",
)
-- go/lib.go --
package lib
func shadowed() string {
foo := "original"
if foo == "original" {
foo := "shadow"
return foo
}
return foo
}
-- go/third_party/BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "lib",
srcs = ["lib.go"],
importpath = "example.com/go/third_party/lib",
)
-- go/third_party/lib.go --
package lib
func shadowed() string {
foo := "original"
if foo == "original" {
foo := "shadow"
return foo
}
return foo
}
`,
})
}

func TestNotIncluded(t *testing.T) {
if err := bazel_testing.RunBazel("build", "//:lib"); err != nil {
t.Fatal(err)
}
}

func TestIncluded(t *testing.T) {
if err := bazel_testing.RunBazel("build", "//go:lib"); err == nil {
t.Fatal("Expected build to fail")
} else if !strings.Contains(err.Error(), "lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)") {
t.Fatalf("Expected error to contain \"lib.go:6:3: declaration of \"foo\" shadows declaration at line 4 (shadow)\", got %s", err)
}
}

func TestExcluded(t *testing.T) {
if err := bazel_testing.RunBazel("build", "//go/third_party:lib"); err != nil {
t.Fatal(err)
}
}
6 changes: 1 addition & 5 deletions tests/core/nogo/nolint/nolint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Nogo: "@//:nogo",
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_tool_library", "nogo")
Expand Down Expand Up @@ -156,11 +157,6 @@ var V = struct {
}

func Test(t *testing.T) {
customRegister := `go_register_toolchains(nogo = "@//:nogo")`
if err := replaceInFile("WORKSPACE", "go_register_toolchains()", customRegister); err != nil {
t.Fatal(err)
}

tests := []struct {
Name string
Target string
Expand Down
5 changes: 3 additions & 2 deletions tests/core/nogo/vet/vet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Nogo: "@io_bazel_rules_go//:default_nogo",
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_tool_library", "nogo")
Expand Down Expand Up @@ -134,8 +135,8 @@ func Test(t *testing.T) {
} {
t.Run(test.desc, func(t *testing.T) {
if test.nogo != "" {
origRegister := "go_register_toolchains()"
customRegister := fmt.Sprintf("go_register_toolchains(nogo = %q)", test.nogo)
origRegister := `nogo = "@io_bazel_rules_go//:default_nogo",`
customRegister := fmt.Sprintf("nogo = %q,", test.nogo)
if err := replaceInFile("WORKSPACE", origRegister, customRegister); err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit a9b312a

Please sign in to comment.