Skip to content

Commit

Permalink
nogo: Create a go_register_nogo wrapper for WORKSPACE users.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DolceTriade committed Jan 23, 2024
1 parent 30099a6 commit 7fa8f78
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 22 deletions.
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
10 changes: 7 additions & 3 deletions go/nogo.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,19 @@ 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_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.
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,
)
43 changes: 39 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,32 @@ go_wrap_sdk(
root_file = "@local_go_sdk//:ROOT",
)
go_register_toolchains({{if .Nogo}}nogo = "{{.Nogo}}"{{end}})
# Required for tests to allow updating the toolchain.
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 7fa8f78

Please sign in to comment.