rules_go improvement to externalize the nogo fix

Author: peng3141

MODULE.bazel

@@ -36,6 +36,7 @@ use_repo(
     "com_github_gogo_protobuf",
     "com_github_golang_mock",
     "com_github_golang_protobuf",
+    "com_github_pmezard_go_difflib",
     "org_golang_google_genproto",
     "org_golang_google_grpc",
     "org_golang_google_grpc_cmd_protoc_gen_go_grpc",

go.sum

@@ -49,7 +49,6 @@ github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
-github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
 github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=

go/.DS_Store

@@ -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
@@ -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,
@@ -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,
@@ -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)

go/private/actions/archive.bzl

@@ -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,
@@ -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]
 
@@ -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,
         )
@@ -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."""
@@ -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")
@@ -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.
@@ -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],

go/private/actions/compilepkg.bzl

@@ -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,
         ),
     ]
 

go/private/rules/binary.bzl

@@ -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,
@@ -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,
         ),
     ]
 

go/private/rules/library.bzl

@@ -75,6 +75,9 @@ 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:
+        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
@@ -95,6 +98,8 @@ def _go_test_impl(ctx):
     external_archive = go.archive(go, external_source, is_external_pkg = True)
     if external_archive.data._validation_output:
         validation_outputs.append(external_archive.data._validation_output)
+    if external_archive.data._nogo_fix_output:
+        validation_outputs.append(external_archive.data._nogo_fix_output)
 
     # now generate the main function
     repo_relative_rundir = ctx.attr.rundir or ctx.label.package or "."

go/private/rules/test.bzl

@@ -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:

go/private/sdk.bzl

@@ -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",
@@ -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",

go/tools/builders/BUILD.bazel

@@ -62,6 +62,6 @@ func main() {
 	log.SetPrefix(verb + ": ")
 
 	if err := action(rest); err != nil {
-		log.Fatalf("\n$$$$$$$$$$$$$$$$$$$$$$$$ fatal: %+v", err)
+		log.Fatal(err)
 	}
 }