Skip to content

Commit

Permalink
rules_go improvement to externalize the nogo fix
Browse files Browse the repository at this point in the history
  • Loading branch information
peng3141 committed Oct 2, 2024
1 parent 106467d commit 47d7bf7
Show file tree
Hide file tree
Showing 19 changed files with 2,341 additions and 263 deletions.
Binary file removed go/.DS_Store
Binary file not shown.
11 changes: 10 additions & 1 deletion go/private/actions/archive.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,18 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
out_facts = go.declare_file(go, name = source.library.name, ext = pre_ext + ".facts")
out_nogo_log = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.log")
out_nogo_validation = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo")

# out_nogo_fix_tmp holds the fixes produced by the RunNogo action, out_nogo_fix holds the fixes produced by the ValidateNogo action.
# They have the same content, but ValidateNogo propagates the fixes and eventually outputs the fixes.
# --run_validations (default=True) ensures nogo fixes (if any) are produced for not only the input targets but also their dependent targets.
# Otherwise, if we put the fixes into the OutputGroupInfo section of the input targets, it will not include the fixes for the dependent targets.
out_nogo_fix_tmp = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.fix.tmp")
out_nogo_fix = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.fix")
else:
out_facts = None
out_nogo_log = None
out_nogo_validation = None
out_nogo_fix_tmp = None
out_nogo_fix = None

direct = source.deps
Expand Down Expand Up @@ -115,6 +122,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
out_facts = out_facts,
out_nogo_log = out_nogo_log,
out_nogo_validation = out_nogo_validation,
out_nogo_fix_tmp = out_nogo_fix_tmp,
out_nogo_fix = out_nogo_fix,
nogo = nogo,
out_cgo_export_h = out_cgo_export_h,
Expand Down Expand Up @@ -145,6 +153,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
out_facts = out_facts,
out_nogo_log = out_nogo_log,
out_nogo_validation = out_nogo_validation,
out_nogo_fix_tmp = out_nogo_fix_tmp,
out_nogo_fix = out_nogo_fix,
nogo = nogo,
gc_goopts = source.gc_goopts,
Expand Down Expand Up @@ -191,7 +200,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
facts_file = out_facts,
runfiles = source.runfiles,
_validation_output = out_nogo_validation,
_out_nogo_fix = out_nogo_fix,
_nogo_fix_output = out_nogo_fix,
_cgo_deps = cgo_deps,
)
x_defs = dict(source.x_defs)
Expand Down
17 changes: 13 additions & 4 deletions go/private/actions/compilepkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def emit_compilepkg(
out_facts = None,
out_nogo_log = None,
out_nogo_validation = None,
out_nogo_fix_tmp = None,
out_nogo_fix = None,
nogo = None,
out_cgo_export_h = None,
Expand All @@ -88,8 +89,11 @@ def emit_compilepkg(
fail("nogo must be specified if and only if out_nogo_log is specified")
if bool(nogo) != bool(out_nogo_validation):
fail("nogo must be specified if and only if out_nogo_validation is specified")
if bool(nogo) != bool(out_nogo_fix_tmp):
fail("nogo must be specified if and only if out_nogo_fix_tmp is specified")
if bool(nogo) != bool(out_nogo_fix):
fail("nogo must be specified if and only if out_nogo_fix is specified")

if cover and go.coverdata:
archives = archives + [go.coverdata]

Expand Down Expand Up @@ -223,6 +227,7 @@ def emit_compilepkg(
out_facts = out_facts,
out_log = out_nogo_log,
out_validation = out_nogo_validation,
out_nogo_fix_tmp = out_nogo_fix_tmp,
out_nogo_fix = out_nogo_fix,
nogo = nogo,
)
Expand All @@ -241,6 +246,7 @@ def _run_nogo(
out_facts,
out_log,
out_validation,
out_nogo_fix_tmp,
out_nogo_fix,
nogo):
"""Runs nogo on Go source files, including those generated by cgo."""
Expand All @@ -250,7 +256,7 @@ def _run_nogo(
[archive.data.facts_file for archive in archives if archive.data.facts_file] +
[archive.data.export_file for archive in archives])
inputs_transitive = [sdk.tools, sdk.headers, go.stdlib.libs]
outputs = [out_facts, out_log, out_nogo_fix]
outputs = [out_facts, out_log, out_nogo_fix_tmp]

args = go.builder_args(go, "nogo", use_path_mapping = True)
args.add_all(sources, before_each = "-src")
Expand All @@ -275,7 +281,7 @@ def _run_nogo(
args.add_all(archives, before_each = "-facts", map_each = _facts)
args.add("-out_facts", out_facts)
args.add("-out_log", out_log)
args.add("-fixpath", out_nogo_fix)
args.add("-out_fix", out_nogo_fix_tmp)
args.add("-nogo", nogo)

# This action runs nogo and produces the facts files for downstream nogo actions.
Expand Down Expand Up @@ -304,9 +310,12 @@ def _run_nogo(
validation_args.add("nogovalidation")
validation_args.add(out_validation)
validation_args.add(out_log)
validation_args.add(out_nogo_fix_tmp)
validation_args.add(out_nogo_fix)

go.actions.run(
inputs = [out_log],
outputs = [out_validation],
inputs = [out_log, out_nogo_fix_tmp],
outputs = [out_validation, out_nogo_fix],
mnemonic = "ValidateNogo",
executable = go.toolchain._builder,
arguments = [validation_args],
Expand Down
9 changes: 8 additions & 1 deletion go/private/rules/binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,20 @@ def _go_binary_impl(ctx):
executable = executable,
)
validation_output = archive.data._validation_output
nogo_fix_output = archive.data._nogo_fix_output

nogo_validation_outputs = []
if validation_output:
nogo_validation_outputs.append(validation_output)
if nogo_fix_output:
nogo_validation_outputs.append(nogo_fix_output)

providers = [
archive,
OutputGroupInfo(
cgo_exports = archive.cgo_exports,
compilation_outputs = [archive.data.file],
_validation = [validation_output] if validation_output else [],
_validation = nogo_validation_outputs,
),
]

Expand Down
11 changes: 8 additions & 3 deletions go/private/rules/library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@ def _go_library_impl(ctx):
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
archive = go.archive(go, source)
validation_output = archive.data._validation_output
nogo_fix_output = archive.data._out_nogo_fix
nogo_fix_output = archive.data._nogo_fix_output

nogo_validation_outputs = []
if validation_output:
nogo_validation_outputs.append(validation_output)
if nogo_fix_output:
nogo_validation_outputs.append(nogo_fix_output)

return [
library,
Expand All @@ -66,8 +72,7 @@ def _go_library_impl(ctx):
OutputGroupInfo(
cgo_exports = archive.cgo_exports,
compilation_outputs = [archive.data.file],
out_nogo_fix = [nogo_fix_output] if nogo_fix_output else [],
_validation = [validation_output] if validation_output else [],
_validation = nogo_validation_outputs,
),
]

Expand Down
5 changes: 5 additions & 0 deletions go/private/rules/test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ def _go_test_impl(ctx):
internal_archive = go.archive(go, internal_source)
if internal_archive.data._validation_output:
validation_outputs.append(internal_archive.data._validation_output)
if internal_archive.data._nogo_fix_output:
# We do not include those from external_archive that corresponds to a separate package
# since that package would be built separately, during which the nogo fixes are produced already.
validation_outputs.append(internal_archive.data._nogo_fix_output)

go_srcs = [src for src in internal_source.srcs if src.extension == "go"]

# Compile the library with the external black box tests
Expand Down
1 change: 1 addition & 0 deletions go/private/sdk.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def _go_download_sdk_impl(ctx):
)

data = ctx.read("versions.json")
ctx.delete("versions.json")
sdks_by_version = _parse_versions_json(data)

if not version:
Expand Down
17 changes: 16 additions & 1 deletion go/tools/builders/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ go_test(
],
)

go_test(
name = "nogo_change_test",
size = "small",
srcs = [
"difflib.go",
"nogo_change.go",
"nogo_change_serialization.go",
"nogo_change_serialization_test.go",
"nogo_change_test.go",
],
deps = [
"@org_golang_x_tools//go/analysis",
],
)

go_test(
name = "stdliblist_test",
size = "small",
Expand Down Expand Up @@ -95,11 +110,11 @@ go_source(
name = "nogo_srcs",
srcs = [
"constants.go",
"difflib.go",
"env.go",
"flags.go",
"nogo_change.go",
"nogo_change_serialization.go",
"nogo_edit.go",
"nogo_main.go",
"nogo_typeparams_go117.go",
"nogo_typeparams_go118.go",
Expand Down
2 changes: 1 addition & 1 deletion go/tools/builders/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ func main() {
log.SetPrefix(verb + ": ")

if err := action(rest); err != nil {
log.Fatalf("\n$$$$$$$$$$$$$$$$$$$$$$$$ fatal: %+v", err)
log.Fatal(err)
}
}
Loading

0 comments on commit 47d7bf7

Please sign in to comment.