diff --git a/go/private/context.bzl b/go/private/context.bzl index 708405a4ec..b6766716c9 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -365,12 +365,13 @@ def go_context(ctx, attr = None): coverdata = None nogo = None if hasattr(attr, "_go_context_data"): - if CgoContextInfo in attr._go_context_data: - cgo_context_info = attr._go_context_data[CgoContextInfo] - go_config_info = attr._go_context_data[GoConfigInfo] - stdlib = attr._go_context_data[GoStdLib] - coverdata = attr._go_context_data[GoContextInfo].coverdata - nogo = attr._go_context_data[GoContextInfo].nogo + go_context_data = _flatten(attr._go_context_data) + if CgoContextInfo in go_context_data: + cgo_context_info = go_context_data[CgoContextInfo] + go_config_info = go_context_data[GoConfigInfo] + stdlib = go_context_data[GoStdLib] + coverdata = go_context_data[GoContextInfo].coverdata + nogo = go_context_data[GoContextInfo].nogo if getattr(attr, "_cgo_context_data", None) and CgoContextInfo in attr._cgo_context_data: cgo_context_info = attr._cgo_context_data[CgoContextInfo] if getattr(attr, "cgo_context_data", None) and CgoContextInfo in attr.cgo_context_data: @@ -378,7 +379,7 @@ def go_context(ctx, attr = None): if hasattr(attr, "_go_config"): go_config_info = attr._go_config[GoConfigInfo] if hasattr(attr, "_stdlib"): - stdlib = attr._stdlib[GoStdLib] + stdlib = _flatten(attr._stdlib)[GoStdLib] mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info) tags = mode.tags @@ -837,3 +838,15 @@ go_config = rule( def _expand_opts(go, attribute_name, opts): return [go._ctx.expand_make_variables(attribute_name, opt, {}) for opt in opts] + +# Used to get attribute values which may have been transitioned. +# Transitioned attributes end up as lists. +# We never use split-transitions, so we always expect exactly one element in those lists. +# But if the attribute wasn't transitioned, it won't be a list. +def _flatten(maybe_list): + if type(maybe_list) == "list": + if len(maybe_list) == 1: + return maybe_list[0] + else: + fail("Expected exactly one element in list but got {}".format(maybe_list)) + return maybe_list diff --git a/go/private/mode.bzl b/go/private/mode.bzl index b755b2fd95..e9b094b0ee 100644 --- a/go/private/mode.bzl +++ b/go/private/mode.bzl @@ -80,8 +80,8 @@ def get_mode(ctx, go_toolchain, cgo_context_info, go_config_info): debug = go_config_info.debug if go_config_info else False linkmode = go_config_info.linkmode if go_config_info else LINKMODE_NORMAL cover_format = go_config_info and go_config_info.cover_format - goos = go_toolchain.default_goos - goarch = go_toolchain.default_goarch + goos = go_toolchain.default_goos if getattr(ctx.attr, "goos", "auto") == "auto" else ctx.attr.goos + goarch = go_toolchain.default_goarch if getattr(ctx.attr, "goarch", "auto") == "auto" else ctx.attr.goarch # TODO(jayconrod): check for more invalid and contradictory settings. if pure and race: diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl index f13aedde4d..02b50f4384 100644 --- a/go/private/rules/binary.bzl +++ b/go/private/rules/binary.bzl @@ -33,7 +33,7 @@ load( ) load( "//go/private/rules:transition.bzl", - "go_transition_rule", + "go_transition", "non_go_transition", ) load( @@ -213,6 +213,7 @@ _go_binary_kwargs = { doc = """List of Go libraries this package imports directly. These may be `go_library` rules or compatible rules with the [GoLibrary] provider. """, + cfg = go_transition, ), "embed": attr.label_list( providers = [GoLibrary], @@ -225,6 +226,7 @@ _go_binary_kwargs = { embedding binary may not also have `cgo = True`. See [Embedding] for more information. """, + cfg = go_transition, ), "embedsrcs": attr.label_list( allow_files = True, @@ -390,7 +392,7 @@ _go_binary_kwargs = { """, ), - "_go_context_data": attr.label(default = "//:go_context_data"), + "_go_context_data": attr.label(default = "//:go_context_data", cfg = go_transition), "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", ), @@ -410,8 +412,7 @@ _go_binary_kwargs = { } go_binary = rule(executable = True, **_go_binary_kwargs) -go_transition_binary = go_transition_rule(executable = True, **_go_binary_kwargs) -go_non_executable_transition_binary = go_transition_rule(executable = False, **_go_binary_kwargs) +go_non_executable_binary = rule(executable = False, **_go_binary_kwargs) def _go_tool_binary_impl(ctx): sdk = ctx.attr.sdk[GoSDK] diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index f8f2630359..4eae434201 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -43,7 +43,7 @@ load( ) load( "//go/private/rules:transition.bzl", - "go_transition_rule", + "go_transition", "non_go_transition", ) load( @@ -220,6 +220,7 @@ _go_test_kwargs = { doc = """List of Go libraries this test imports directly. These may be go_library rules or compatible rules with the [GoLibrary] provider. """, + cfg = go_transition, ), "embed": attr.label_list( providers = [GoLibrary], @@ -231,6 +232,7 @@ _go_test_kwargs = { and the embedding library may not also have `cgo = True`. See [Embedding] for more information. """, + cfg = go_transition, ), "embedsrcs": attr.label_list( allow_files = True, @@ -402,10 +404,11 @@ _go_test_kwargs = { See [Cross compilation] for more information. """, ), - "_go_context_data": attr.label(default = "//:go_context_data"), + "_go_context_data": attr.label(default = "//:go_context_data", cfg = go_transition), "_testmain_additional_deps": attr.label_list( providers = [GoLibrary], default = ["//go/tools/bzltestutil"], + cfg = go_transition, ), # Workaround for bazelbuild/bazel#6293. See comment in lcov_merger.sh. "_lcov_merger": attr.label( @@ -453,7 +456,6 @@ _go_test_kwargs = { } go_test = rule(**_go_test_kwargs) -go_transition_test = go_transition_rule(**_go_test_kwargs) def _recompile_external_deps(go, external_source, internal_archive, library_labels): """Recompiles some archives in order to split internal and external tests. diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl index 6499caecfc..f567028398 100644 --- a/go/private/rules/transition.bzl +++ b/go/private/rules/transition.bzl @@ -80,62 +80,6 @@ _SETTING_KEY_TO_ORIGINAL_SETTING_KEY = { for setting in TRANSITIONED_GO_SETTING_KEYS } -def go_transition_wrapper(kind, transition_kind, name, **kwargs): - """Wrapper for rules that may use transitions. - - This is used in place of instantiating go_binary or go_transition_binary - directly. If one of the transition attributes is set explicitly, it - instantiates the rule with a transition. Otherwise, it instantiates the - regular rule. This prevents targets from being rebuilt for an alternative - configuration identical to the default configuration. - """ - transition_keys = ("goos", "goarch", "pure", "static", "msan", "race", "gotags", "linkmode") - need_transition = any([key in kwargs for key in transition_keys]) - if need_transition: - transition_kind(name = name, **kwargs) - else: - kind(name = name, **kwargs) - -def go_transition_rule(**kwargs): - """Like "rule", but adds a transition and mode attributes.""" - kwargs = dict(kwargs) - kwargs["attrs"].update({ - "goos": attr.string( - default = "auto", - values = ["auto"] + {goos: None for goos, _ in GOOS_GOARCH}.keys(), - ), - "goarch": attr.string( - default = "auto", - values = ["auto"] + {goarch: None for _, goarch in GOOS_GOARCH}.keys(), - ), - "pure": attr.string( - default = "auto", - values = ["auto", "on", "off"], - ), - "static": attr.string( - default = "auto", - values = ["auto", "on", "off"], - ), - "msan": attr.string( - default = "auto", - values = ["auto", "on", "off"], - ), - "race": attr.string( - default = "auto", - values = ["auto", "on", "off"], - ), - "gotags": attr.string_list(default = []), - "linkmode": attr.string( - default = "auto", - values = ["auto"] + LINKMODES, - ), - "_whitelist_function_transition": attr.label( - default = "@bazel_tools//tools/whitelists/function_transition_whitelist", - ), - }) - kwargs["cfg"] = go_transition - return rule(**kwargs) - def _go_transition_impl(settings, attr): # NOTE: Keep the list of rules_go settings set by this transition in sync # with POTENTIALLY_TRANSITIONED_SETTINGS. diff --git a/go/private/rules/wrappers.bzl b/go/private/rules/wrappers.bzl index df7ccec78f..7c4993d608 100644 --- a/go/private/rules/wrappers.bzl +++ b/go/private/rules/wrappers.bzl @@ -26,17 +26,11 @@ load( load( "//go/private/rules:binary.bzl", "go_binary", - "go_non_executable_transition_binary", - "go_transition_binary", + "go_non_executable_binary", ) load( "//go/private/rules:test.bzl", "go_test", - "go_transition_test", -) -load( - "//go/private/rules:transition.bzl", - "go_transition_wrapper", ) def _cgo(name, kwargs): @@ -51,12 +45,24 @@ def go_library_macro(name, **kwargs): def go_binary_macro(name, **kwargs): """See docs/go/core/rules.md#go_binary for full documentation.""" _cgo(name, kwargs) + + if kwargs.get("goos") != None or kwargs.get("goarch") != None: + for key, value in kwargs.items(): + if type(value) == "select": + # In the long term, we should replace goos/goarch with Bazel-native platform + # support, but while we have the mechanisms, we try to avoid people trying to use + # _both_ goos/goarch _and_ native platform support. + # + # It's unclear to users whether the select should happen before or after the + # goos/goarch is reconfigured, and we can't interpose code to force either + # behaviour, so we forbid this. + fail("Cannot use select for go_binary with goos/goarch set, but {} was a select".format(key)) + if kwargs.get("linkmode", default = LINKMODE_NORMAL) in LINKMODES_EXECUTABLE: - go_transition_wrapper(go_binary, go_transition_binary, name = name, **kwargs) + go_binary(name = name, **kwargs) else: - # A non-normal link mode implies the use of transitions, so we don't have to define a - # non-executable version of the untransitioned go_binary. - go_transition_wrapper(None, go_non_executable_transition_binary, name = name, **kwargs) + go_non_executable_binary(name = name, **kwargs) + if kwargs.get("linkmode") in (LINKMODE_C_ARCHIVE, LINKMODE_C_SHARED): # Create an alias to tell users of the `.cc` rule that it is deprecated. native.alias( @@ -70,4 +76,4 @@ def go_binary_macro(name, **kwargs): def go_test_macro(name, **kwargs): """See docs/go/core/rules.md#go_test for full documentation.""" _cgo(name, kwargs) - go_transition_wrapper(go_test, go_transition_test, name = name, **kwargs) + go_test(name = name, **kwargs) diff --git a/tests/core/go_binary/BUILD.bazel b/tests/core/go_binary/BUILD.bazel index 930425a528..99a2d78492 100644 --- a/tests/core/go_binary/BUILD.bazel +++ b/tests/core/go_binary/BUILD.bazel @@ -4,6 +4,16 @@ load(":many_deps.bzl", "many_deps") test_suite(name = "go_binary") +go_bazel_test( + name = "configurable_attribute_bad_test", + srcs = ["configurable_attribute_bad_test.go"], +) + +go_bazel_test( + name = "configurable_attribute_good_test", + srcs = ["configurable_attribute_good_test.go"], +) + go_binary( name = "hello", srcs = ["hello.go"], diff --git a/tests/core/go_binary/configurable_attribute_bad_test.go b/tests/core/go_binary/configurable_attribute_bad_test.go new file mode 100644 index 0000000000..e043275d65 --- /dev/null +++ b/tests/core/go_binary/configurable_attribute_bad_test.go @@ -0,0 +1,69 @@ +// Copyright 2022 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 configurable_attribute_bad_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{ + Main: ` +-- BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_binary") + +go_binary( + name = "main", + srcs = [ + "main.go", + ], + goos = "darwin", + goarch = "amd64", + gotags = select({ + "@io_bazel_rules_go//go/platform:linux": ["penguins"], + "//conditions:default": ["nopenguins"], + }), +) + +-- main.go -- +package main + +import "fmt" + +func main() { + fmt.Println("Howdy") +} +`, + }) +} + +func TestConfigurableGotagsAttribute(t *testing.T) { + _, err := bazel_testing.BazelOutput("build", "//:main") + if err == nil { + t.Fatal("Want error") + } + eErr, ok := err.(*bazel_testing.StderrExitError) + if !ok { + t.Fatalf("Want StderrExitError but got %v", err) + } + stderr := eErr.Error() + want := "Cannot use select for go_binary with goos/goarch set, but gotags was a select" + if !strings.Contains(stderr, want) { + t.Fatalf("Want error message containing %q but got %v", want, stderr) + } +} diff --git a/tests/core/go_binary/configurable_attribute_good_test.go b/tests/core/go_binary/configurable_attribute_good_test.go new file mode 100644 index 0000000000..a1fe195442 --- /dev/null +++ b/tests/core/go_binary/configurable_attribute_good_test.go @@ -0,0 +1,92 @@ +// Copyright 2022 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 configurable_attribute_good_test + +import ( + "runtime" + "strings" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/bazel_testing" +) + +func TestMain(m *testing.M) { + bazel_testing.TestMain(m, bazel_testing.Args{ + Main: ` +-- BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_binary") + +go_binary( + name = "main", + srcs = [ + "main.go", + "lib_nopenguins.go", + "lib_penguins.go", + ], + gotags = select({ + "@io_bazel_rules_go//go/platform:linux": ["penguins"], + "//conditions:default": ["nopenguins"], + }), +) + +-- main.go -- +package main + +import "fmt" + +func main() { + fmt.Println(message()) +} + +-- lib_penguins.go -- +// +build penguins + +package main + +func message() string { + return "Penguins are great" +} + + +-- lib_nopenguins.go -- +// +build !penguins + +package main + +func message() string { + return "Penguins smell fishy'" +} +`, + }) +} + +func TestConfigurableGotagsAttribute(t *testing.T) { + outBytes, err := bazel_testing.BazelOutput("run", "//:main") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + out := string(outBytes) + os := runtime.GOOS + switch os { + case "linux": + if !strings.Contains(out, "Penguins are great") { + t.Fatalf("Wanted penguin executable, but output was: %s", out) + } + default: + if !strings.Contains(out, "Penguins smell fishy") { + t.Fatalf("Wanted nopenguin executable, but output was: %s", out) + } + } +}