From 7718a1bc0d69d060af934029de74591d0781c6bd Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 9 May 2022 21:33:18 +0200 Subject: [PATCH] Let go_binary's executable bit depend on `linkmode` (#3143) If a go_binary's `linkmode` is neither `normal` nor `pie`, the resulting file can't be executed. In this case, `executable` should be set to `False` on the go_binary rule so that Bazel doesn't create a runfiles tree for the targets and does not include e.g. a resulting static library in the runfiles of dependants. --- go/private/mode.bzl | 3 ++ go/private/rules/binary.bzl | 6 +-- go/private/rules/wrappers.bzl | 10 +++- tests/core/go_binary/BUILD.bazel | 5 ++ tests/core/go_binary/non_executable_test.go | 51 +++++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/core/go_binary/non_executable_test.go diff --git a/go/private/mode.bzl b/go/private/mode.bzl index 9ca4525047..b755b2fd95 100644 --- a/go/private/mode.bzl +++ b/go/private/mode.bzl @@ -28,6 +28,9 @@ LINKMODE_C_ARCHIVE = "c-archive" LINKMODES = [LINKMODE_NORMAL, LINKMODE_PLUGIN, LINKMODE_C_SHARED, LINKMODE_C_ARCHIVE, LINKMODE_PIE] +# All link modes that produce executables to be run with bazel run. +LINKMODES_EXECUTABLE = [LINKMODE_NORMAL, LINKMODE_PIE] + def mode_string(mode): result = [mode.goos, mode.goarch] if mode.static: diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl index 3481f1fe43..388445c164 100644 --- a/go/private/rules/binary.bzl +++ b/go/private/rules/binary.bzl @@ -372,7 +372,6 @@ _go_binary_kwargs = { ), "_go_context_data": attr.label(default = "//:go_context_data"), }, - "executable": True, "toolchains": ["@io_bazel_rules_go//go:toolchain"], "doc": """This builds an executable from a set of source files, which must all be in the `main` package. You can run the binary with @@ -387,8 +386,9 @@ _go_binary_kwargs = { """, } -go_binary = rule(**_go_binary_kwargs) -go_transition_binary = go_transition_rule(**_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) def _go_tool_binary_impl(ctx): sdk = ctx.attr.sdk[GoSDK] diff --git a/go/private/rules/wrappers.bzl b/go/private/rules/wrappers.bzl index c7ef6ed97a..df7ccec78f 100644 --- a/go/private/rules/wrappers.bzl +++ b/go/private/rules/wrappers.bzl @@ -14,8 +14,10 @@ load( "//go/private:mode.bzl", + "LINKMODES_EXECUTABLE", "LINKMODE_C_ARCHIVE", "LINKMODE_C_SHARED", + "LINKMODE_NORMAL", ) load( "//go/private/rules:library.bzl", @@ -24,6 +26,7 @@ load( load( "//go/private/rules:binary.bzl", "go_binary", + "go_non_executable_transition_binary", "go_transition_binary", ) load( @@ -48,7 +51,12 @@ 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) - go_transition_wrapper(go_binary, go_transition_binary, name = name, **kwargs) + if kwargs.get("linkmode", default = LINKMODE_NORMAL) in LINKMODES_EXECUTABLE: + go_transition_wrapper(go_binary, go_transition_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) 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( diff --git a/tests/core/go_binary/BUILD.bazel b/tests/core/go_binary/BUILD.bazel index dfa3de252e..930425a528 100644 --- a/tests/core/go_binary/BUILD.bazel +++ b/tests/core/go_binary/BUILD.bazel @@ -176,3 +176,8 @@ go_binary( name = "prefix", embed = ["//tests/core/go_binary/prefix"], ) + +go_bazel_test( + name = "non_executable_test", + srcs = ["non_executable_test.go"], +) diff --git a/tests/core/go_binary/non_executable_test.go b/tests/core/go_binary/non_executable_test.go new file mode 100644 index 0000000000..e1a352426a --- /dev/null +++ b/tests/core/go_binary/non_executable_test.go @@ -0,0 +1,51 @@ +// 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 non_executable_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: ` +-- src/BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_binary") + +go_binary( + name = "archive", + srcs = ["archive.go"], + linkmode = "c-archive", +) +-- src/archive.go -- +package main + +func main() {} +`, + }) +} + +func TestNonExecutableGoBinary(t *testing.T) { + if err := bazel_testing.RunBazel("build", "//src:archive"); err != nil { + t.Fatal(err) + } + err := bazel_testing.RunBazel("run", "//src:archive") + if err == nil || !strings.Contains(err.Error(), "ERROR: Cannot run target //src:archive: Not executable") { + t.Errorf("Expected bazel run to fail due to //src:archive not being executable") + } +}