bazel-contrib / bazel-contrib/rules_go

`linkmode = "c-archive"` gets propagated across `cfg = exec` (so even to tools)

Open
#4,544 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.5k
Forks
760
Avg merge
1d 11h
Merged PRs (30d)
12

Description

## Summary of the error

When Bazel analyses a `go_binary` target X which has `linkmode = "c-archive"`, and a standalone executable (`linkmode = "normal"`) `go_binary` target Y is reachable (even via tools) from X, then Bazel analysis fails with

```
ERROR: ... The rule '...' is executable. It needs to create an executable File and pass it as the 'executable' parameter to the DefaultInfo it returns.
```

## Simplistic repro

Let's first consider this (somewhat contrived) example:

```starlark
load("@io_bazel_rules_go//go:def.bzl", "go_binary")

go_binary(
name = "standalone_binary",
srcs = ["binary.go"],
)

go_binary(
name = "static_c_library",
srcs = ["library.go"],
cgo = True,
linkmode = "c-archive",
deps = [":standalone_binary"], # WHOOPS
)
```

When running `bazel build --nobuild :static_c_library`, we end up with a somewhat unclear error:

```
ERROR: ... The rule 'standalone_binary' is executable. It needs to create an executable File and pass it as the 'executable' parameter to the DefaultInfo it returns.
```

This is caused by `linkmode = "c-archive"` getting forced by `go_transition` onto a rule which is already defined with `executable = True`. The [rule implementation](https://github.com/bazel-contrib/rules_go/blob/v0.59.0/go/private/rules/binary.bzl#L201) does not provide `executable` in `DefaultInfo` provided, causing analysis to fail.

Note that setting `linkmode = "normal"` explicitly in `standalone_binary` doesn't fix the problem, it just makes the error slightly less unclear:

```
(16:21:29) ERROR: Traceback (most recent call last):
File ".../external/io_bazel_rules_go/go/private/rules/transition.bzl", line 132, column 21, in _go_transition_impl
fail("go_transition can't be nested")
Error in fail: go_transition can't be nested
(16:21:29) ERROR: ...: Errors encountered while applying Starlark transition
```

All in all, it's pretty reasonable to assume that it's forbidden for a C static library to depend on a standalone executable.

It's also unlikely that anyone would want to actually depend from library to executable (not the other way round) in such a direct way.

## More realistic repro

Things get more difficult when we consider that the standalone executable can be used in an aspect that is applied to the library.

With a simple aspect like that:

```starlark
def _demo_aspect_impl(target, ctx):
return []

demo_aspect = aspect(
implementation = _demo_aspect_impl,
attrs = {
"_tool": attr.label(
default = "//:tool_binary",
cfg = "exec",
executable = True,
),
},
)
```

even when the problematic `deps = [":standalone_binary"]` is removed from `static_c_library`, we still get the same error.

## Possible workaround

To solve the problem without patching rules_go, I've made sure that `go_tool_transition` is applied (`cfg = go_tool_transition`) on my `tool_binary` before it's referenced from the aspect. Also, I needed to apply `cfg = "exec"` - hence I needed to create a passthrough rule like that:

```starlark
load("@io_bazel_rules_go//go/private/rules:transition.bzl", "go_tool_transition")

def _go_tool_wrapper_impl(ctx):
"""Pass-through wrapper that applies go_tool_transition to reset Go settings.
This rule exists solely to apply go_tool_transition to its dependency,
ensuring the underlying Go binary is built with default Go settings.
"""
# Custom transitions require attr.label_list (even for 1-to-1 transitions).
# Bazel's transition API treats all custom transitions as potentially split,
# so the attribute is always a list. We take the first element since
# go_tool_transition is guaranteed to be 1-to-1.
if len(ctx.attr.actual_executable) != 1:
fail("actual_executable must contain exactly one target")

actual_executable = ctx.attr.actual_executable[0]
source_executable = actual_executable[DefaultInfo].files_to_run.executable
runfiles = actual_executable[DefaultInfo].default_runfiles

# Create a symlink to the actual executable
wrapper_executable = ctx.actions.declare_file(ctx.label.name)
ctx.actions.symlink(
output = wrapper_executable,
target_file = source_executable,
is_executable = True,
)

return [
DefaultInfo(
executable = wrapper_executable,
runfiles = runfiles,
),
]

_go_tool_wrapper = rule(
implementation = _go_tool_wrapper_impl,
executable = True,
attrs = {
"actual_executable": attr.label_list(
mandatory = True,
cfg = go_tool_transition,
),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
)
```

since Bazel doesn't allow for composing transitions with anything like `cfg = ["exec", go_tool_transition]`.

Still, this feels very hackish - I believe there must be a cleaner way to solve it in Bazel (as the problem doesn't seem specific to rules_go; all rules that define their own build settings may be affected to some degree).

Contributor guide

Open the contributing guide

Research direction

Start with go/private/rules/transition.bzl, especially the go transition implementation, and go/private/rules/binary.bzl around the linked rule implementation. Reproduce the direct dependency and aspect tool cases, then define a fix that prevents c-archive settings from propagating incorrectly while avoiding nested transitions; done means both analyses succeed with the intended link modes.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
build-system, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.