Skip to content

Commit 045eb9c

Browse files
authored
Generate facts for targets not in nogo scope (#4268)
**What type of PR is this?** Bug fix **What does this PR do? Why is it needed?** Nogo analyzers may need facts from their (transitive) dependencies to operate correctly. We thus need to run nogo on all targets, even those not in scope, but should only register the validation action for the targets that are in scope to have them fail the build on nogo findings. **Which issues(s) does this PR fix?** Fixes #4241 **Other notes for review**
1 parent c0bf665 commit 045eb9c

File tree

7 files changed

+36
-39
lines changed

7 files changed

+36
-39
lines changed

go/private/actions/archive.bzl

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
load(
1616
"//go/private:context.bzl",
17-
"get_nogo",
17+
"validate_nogo",
1818
)
1919
load(
2020
"//go/private:mode.bzl",
@@ -59,19 +59,22 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
5959
out_export = go.declare_file(go, name = source.name, ext = pre_ext + ".x")
6060
out_cgo_export_h = None # set if cgo used in c-shared or c-archive mode
6161

62-
nogo = get_nogo(go)
62+
nogo = go.nogo
6363

6464
# nogo is a FilesToRunProvider and some targets don't have it, some have it but no executable.
6565
if nogo != None and nogo.executable != None:
6666
out_facts = go.declare_file(go, name = source.name, ext = pre_ext + ".facts")
6767
out_nogo_log = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo.log")
68-
out_nogo_validation = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo")
6968
out_nogo_fix = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo.patch")
69+
if validate_nogo(go):
70+
out_nogo_validation = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo")
71+
else:
72+
out_nogo_validation = None
7073
else:
7174
out_facts = None
7275
out_nogo_log = None
73-
out_nogo_validation = None
7476
out_nogo_fix = None
77+
out_nogo_validation = None
7578

7679
direct = source.deps
7780

@@ -116,8 +119,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
116119
out_export = out_export,
117120
out_facts = out_facts,
118121
out_nogo_log = out_nogo_log,
119-
out_nogo_validation = out_nogo_validation,
120122
out_nogo_fix = out_nogo_fix,
123+
out_nogo_validation = out_nogo_validation,
121124
nogo = nogo,
122125
out_cgo_export_h = out_cgo_export_h,
123126
gc_goopts = source.gc_goopts,

go/private/actions/compilepkg.bzl

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ def emit_compilepkg(
6969
out_export = None,
7070
out_facts = None,
7171
out_nogo_log = None,
72-
out_nogo_validation = None,
7372
out_nogo_fix = None,
73+
out_nogo_validation = None,
7474
nogo = None,
7575
out_cgo_export_h = None,
7676
gc_goopts = [],
@@ -88,8 +88,6 @@ def emit_compilepkg(
8888
fail("nogo must be specified if and only if out_facts is specified", nogo)
8989
if have_nogo != (out_nogo_log != None):
9090
fail("nogo must be specified if and only if out_nogo_log is specified")
91-
if have_nogo != (out_nogo_validation != None):
92-
fail("nogo must be specified if and only if out_nogo_validation is specified")
9391
if have_nogo != (out_nogo_fix != None):
9492
fail("nogo must be specified if and only if out_nogo_fix is specified")
9593

@@ -222,8 +220,8 @@ def emit_compilepkg(
222220
archives = archives,
223221
out_facts = out_facts,
224222
out_log = out_nogo_log,
225-
out_validation = out_nogo_validation,
226223
out_fix = out_nogo_fix,
224+
out_validation = out_nogo_validation,
227225
nogo = nogo,
228226
)
229227

@@ -279,21 +277,22 @@ def _run_nogo(
279277
progress_message = "Running nogo on %{label}",
280278
)
281279

282-
# This is a separate action that produces the validation output registered with Bazel. It
283-
# prints any nogo findings and, crucially, fails if there are any findings. This is necessary
284-
# to actually fail the build on nogo findings, which RunNogo doesn't do.
285-
validation_args = go.actions.args()
286-
validation_args.add("nogovalidation")
287-
validation_args.add(out_validation)
288-
validation_args.add(out_log)
289-
validation_args.add(out_fix)
280+
if out_validation:
281+
# This is a separate action that produces the validation output registered with Bazel. It
282+
# prints any nogo findings and, crucially, fails if there are any findings. This is necessary
283+
# to actually fail the build on nogo findings, which RunNogo doesn't do.
284+
validation_args = go.actions.args()
285+
validation_args.add("nogovalidation")
286+
validation_args.add(out_validation)
287+
validation_args.add(out_log)
288+
validation_args.add(out_fix)
290289

291-
go.actions.run(
292-
inputs = [out_log, out_fix],
293-
outputs = [out_validation],
294-
mnemonic = "ValidateNogo",
295-
executable = go.toolchain._builder,
296-
arguments = [validation_args],
297-
execution_requirements = SUPPORTS_PATH_MAPPING_REQUIREMENT,
298-
progress_message = "Validating nogo output for %{label}",
299-
)
290+
go.actions.run(
291+
inputs = [out_log, out_fix],
292+
outputs = [out_validation],
293+
mnemonic = "ValidateNogo",
294+
executable = go.toolchain._builder,
295+
arguments = [validation_args],
296+
execution_requirements = SUPPORTS_PATH_MAPPING_REQUIREMENT,
297+
progress_message = "Validating nogo output for %{label}",
298+
)

go/private/context.bzl

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -429,13 +429,11 @@ def _matches_scopes(label, scopes):
429429
return True
430430
return False
431431

432-
def get_nogo(go):
433-
"""Returns the nogo file for this target, if enabled and in scope."""
432+
def validate_nogo(go):
433+
"""Whether nogo should be run as a validation action rather than just to generate fact files for the current
434+
target."""
434435
label = go.label
435-
if _matches_scopes(label, NOGO_INCLUDES) and not _matches_scopes(label, NOGO_EXCLUDES):
436-
return go.nogo
437-
else:
438-
return None
436+
return _matches_scopes(label, NOGO_INCLUDES) and not _matches_scopes(label, NOGO_EXCLUDES)
439437

440438
default_go_config_info = GoConfigInfo(
441439
static = False,

go/private/rules/library.bzl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ go_library = rule(
115115
with the same path (for example, from different vendor directories).
116116
""",
117117
),
118-
"importpath_aliases": attr.string_list(
119-
), # experimental, undocumented
118+
"importpath_aliases": attr.string_list(), # experimental, undocumented
120119
"embed": attr.label_list(
121120
providers = [GoInfo],
122121
doc = """

tests/core/cgo/BUILD.bazel

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,23 +99,21 @@ go_library(
9999
tags = ["manual"],
100100
)
101101

102-
103102
genrule(
104103
name = "generate_imported_dylib_linux",
105104
srcs = ["imported.c"],
106-
tools = ["generate_imported_dylib.sh"],
107105
outs = [
108106
"libimported.so",
109107
"libversioned.so.2",
110108
],
111109
cmd = "$(location generate_imported_dylib.sh) $(location imported.c) $(@D)",
112110
target_compatible_with = ["@platforms//os:linux"],
111+
tools = ["generate_imported_dylib.sh"],
113112
)
114113

115114
genrule(
116115
name = "generate_imported_dylib_darwin",
117116
srcs = ["imported.c"],
118-
tools = ["generate_imported_dylib.sh"],
119117
outs = [
120118
"libimported.dylib",
121119
"libversioned.2.dylib",
@@ -124,6 +122,7 @@ genrule(
124122
],
125123
cmd = "$(location generate_imported_dylib.sh) $(location imported.c) $(@D)",
126124
target_compatible_with = ["@platforms//os:macos"],
125+
tools = ["generate_imported_dylib.sh"],
127126
)
128127

129128
cc_import(

tests/core/go_library/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ genrule(
186186
go_source(
187187
name = "use_syso_srcs",
188188
srcs = [
189-
":sysolib",
190189
"wrap_imported_amd64.s",
191190
"wrap_imported_arm64.s",
191+
":sysolib",
192192
],
193193
visibility = [
194194
"//tests/core/go_binary:__pkg__",

tests/core/go_proto_aspect/codegen.bzl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ def _go_generated_library_impl(ctx):
2727
),
2828
]
2929

30-
3130
go_generated_library = rule(
3231
implementation = _go_generated_library_impl,
3332
attrs = {

0 commit comments

Comments
 (0)