Skip to content

Commit 2d22666

Browse files
dzbarskyfmeum
andauthored
Remove GoSource/GoLibrary from test/binary rules (#4044)
**What type of PR is this?** Starlark cleanup **What does this PR do? Why is it needed?** We don't want go_binary or go_test targets to be included in `deps` or `embed`. We have a check for it today, but we can push this into the starlark type system by ensuring they don't have these providers. **Which issues(s) does this PR fix?** Fixes # **Other notes for review** --------- Co-authored-by: Fabian Meumertzheim <[email protected]>
1 parent a01ba7c commit 2d22666

File tree

5 files changed

+73
-28
lines changed

5 files changed

+73
-28
lines changed

go/private/context.bzl

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,7 @@ def _library_to_source(go, attr, library, coverage_instrumented):
284284
"pgoprofile": getattr(attr, "pgoprofile", None),
285285
}
286286

287-
for dep in source["deps"]:
288-
_check_binary_dep(go, dep, "deps")
289287
for e in getattr(attr, "embed", []):
290-
_check_binary_dep(go, e, "embed")
291288
_merge_embed(source, e)
292289

293290
source["deps"] = _dedup_archives(source["deps"])
@@ -324,24 +321,6 @@ def _collect_runfiles(go, data, deps):
324321
[get_source(t).runfiles for t in deps],
325322
)
326323

327-
def _check_binary_dep(go, dep, edge):
328-
"""Checks that this rule doesn't depend on a go_binary or go_test.
329-
330-
go_binary and go_test may return providers with useful information for other
331-
rules (like go_path), but go_binary and go_test may not depend on other
332-
go_binary and go_test targets. Their dependencies may be built in
333-
different modes, resulting in conflicts and opaque errors.
334-
"""
335-
if (type(dep) == "Target" and
336-
DefaultInfo in dep and
337-
getattr(dep[DefaultInfo], "files_to_run", None) and
338-
dep[DefaultInfo].files_to_run.executable):
339-
fail("rule {rule} depends on executable {dep} via {edge}. This is not safe for cross-compilation. Depend on go_library instead.".format(
340-
rule = str(go.label),
341-
dep = str(dep.label),
342-
edge = edge,
343-
))
344-
345324
def _check_importpaths(ctx):
346325
paths = []
347326
p = getattr(ctx.attr, "importpath", "")

go/private/rules/binary.bzl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,6 @@ def _go_binary_impl(ctx):
141141
validation_output = archive.data._validation_output
142142

143143
providers = [
144-
library,
145-
source,
146144
archive,
147145
OutputGroupInfo(
148146
cgo_exports = archive.cgo_exports,

go/private/rules/cross.bzl

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
load(
1616
"//go/private:providers.bzl",
1717
"GoArchive",
18-
"GoLibrary",
19-
"GoSource",
2018
)
2119
load(
2220
"//go/private/rules:transition.bzl",
@@ -84,8 +82,6 @@ def _go_cross_impl(ctx):
8482
providers = [
8583
ctx.attr.target[provider]
8684
for provider in [
87-
GoLibrary,
88-
GoSource,
8985
GoArchive,
9086
OutputGroupInfo,
9187
CcInfo,
@@ -100,7 +96,7 @@ _go_cross_kwargs = {
10096
"target": attr.label(
10197
doc = """Go binary target to transition to the given platform and/or sdk_version.
10298
""",
103-
providers = [GoLibrary, GoSource, GoArchive],
99+
providers = [GoArchive],
104100
mandatory = True,
105101
),
106102
"platform": attr.label(

tests/core/starlark/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
load(":common_tests.bzl", "common_test_suite")
22
load(":context_tests.bzl", "context_test_suite")
3+
load(":provider_tests.bzl", "provider_test_suite")
34
load(":sdk_tests.bzl", "sdk_test_suite")
45

56
common_test_suite()
67

78
context_test_suite()
89

10+
provider_test_suite()
11+
912
sdk_test_suite()
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
2+
load("//go:def.bzl", "go_binary", "go_library", "go_test")
3+
4+
# go_binary and go_test targets must not be used as deps/embed attributes;
5+
# their dependencies may be built in different modes, resulting in conflicts and opaque errors.
6+
def _providers_test_impl(ctx):
7+
env = analysistest.begin(ctx)
8+
asserts.expect_failure(env, "does not have mandatory providers")
9+
return analysistest.end(env)
10+
11+
providers_test = analysistest.make(
12+
_providers_test_impl,
13+
expect_failure = True,
14+
)
15+
16+
def provider_test_suite():
17+
go_binary(
18+
name = "go_binary",
19+
tags = ["manual"],
20+
)
21+
22+
go_library(
23+
name = "lib_binary_deps",
24+
deps = [":go_binary"],
25+
tags = ["manual"],
26+
)
27+
28+
providers_test(
29+
name = "go_binary_deps_test",
30+
target_under_test = ":lib_binary_deps",
31+
)
32+
33+
go_library(
34+
name = "lib_binary_embed",
35+
embed = [":go_binary"],
36+
tags = ["manual"],
37+
)
38+
39+
providers_test(
40+
name = "go_binary_embed_test",
41+
target_under_test = ":lib_binary_embed",
42+
)
43+
44+
go_test(
45+
name = "go_test",
46+
tags = ["manual"],
47+
)
48+
49+
go_library(
50+
name = "lib_test_deps",
51+
deps = [":go_test"],
52+
tags = ["manual"],
53+
)
54+
55+
providers_test(
56+
name = "go_test_deps_test",
57+
target_under_test = ":lib_test_deps",
58+
)
59+
60+
go_library(
61+
name = "lib_embed_test",
62+
embed = [":go_test"],
63+
tags = ["manual"],
64+
)
65+
66+
providers_test(
67+
name = "go_test_embed_test",
68+
target_under_test = ":lib_embed_test",
69+
)

0 commit comments

Comments
 (0)