Extract nogo facts into a separate file

go/private/actions/archive.bzl

@@ -58,9 +58,13 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
         pre_ext += _recompile_suffix
     out_lib = go.declare_file(go, name = source.library.name, ext = pre_ext + ".a")
 
-    # store __.PKGDEF and nogo facts in .x
+    # store export information for compiling dependent packages separately
     out_export = go.declare_file(go, name = source.library.name, ext = pre_ext + ".x")
     out_cgo_export_h = None  # set if cgo used in c-shared or c-archive mode
+    out_facts = None
+    nogo = go.get_nogo(go)
+    if nogo:
+        out_facts = go.declare_file(go, name = source.library.name, ext = pre_ext + ".facts")
 
     direct = [get_archive(dep) for dep in source.deps]
     runfiles = source.runfiles
@@ -105,6 +109,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
             archives = direct,
             out_lib = out_lib,
             out_export = out_export,
+            out_facts = out_facts,
+            nogo = nogo,
             out_cgo_export_h = out_cgo_export_h,
             gc_goopts = source.gc_goopts,
             cgo = True,
@@ -129,6 +135,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
             archives = direct,
             out_lib = out_lib,
             out_export = out_export,
+            out_facts = out_facts,
+            nogo = nogo,
             gc_goopts = source.gc_goopts,
             cgo = False,
             testfilter = testfilter,
@@ -173,6 +181,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
         # Information needed by dependents
         file = out_lib,
         export_file = out_export,
+        facts_file = out_facts,
         data_files = as_tuple(data_files),
         _cgo_deps = as_tuple(cgo_deps),
     )

go/private/actions/compilepkg.bzl

@@ -28,6 +28,18 @@ def _archive(v):
         v.data.export_file.path if v.data.export_file else v.data.file.path,
     )
 
+def _facts(v):
+    facts_file = v.data.facts_file
+    if not facts_file:
+        return None
+    importpaths = [v.data.importpath]
+    importpaths.extend(v.data.importpath_aliases)
+    return "{}={}={}".format(
+        ":".join(importpaths),
+        v.data.importmap,
+        facts_file.path,
+    )
+
 def _embedroot_arg(src):
     return src.root.path
 
@@ -55,6 +67,8 @@ def emit_compilepkg(
         clinkopts = [],
         out_lib = None,
         out_export = None,
+        out_facts = None,
+        nogo = None,
         out_cgo_export_h = None,
         gc_goopts = [],
         testfilter = None,  # TODO: remove when test action compiles packages
@@ -64,6 +78,8 @@ def emit_compilepkg(
         fail("sources is a required parameter")
     if out_lib == None:
         fail("out_lib is a required parameter")
+    if bool(nogo) != bool(out_facts):
+        fail("nogo must be specified if and only if out_facts is specified")
 
     inputs = (sources + embedsrcs + [go.package_list] +
               [archive.data.export_file for archive in archives] +
@@ -110,8 +126,11 @@ def emit_compilepkg(
 
     args.add("-o", out_lib)
     args.add("-x", out_export)
-    nogo = go.get_nogo(go)
     if nogo:
+        args.add_all(archives, before_each = "-facts", map_each = _facts)
+        inputs.extend([archive.data.facts_file for archive in archives if archive.data.facts_file])
+        args.add("-out_facts", out_facts)
+        outputs.append(out_facts)
         args.add("-nogo", nogo)
         inputs.append(nogo)
     if out_cgo_export_h:

go/providers.rst

@@ -260,7 +260,15 @@ rule.  Instead, it's referenced in the ``data`` field of GoArchive_.
 +--------------------------------+-----------------------------------------------------------------+
 | :param:`file`                  | :type:`File`                                                    |
 +--------------------------------+-----------------------------------------------------------------+
-| The archive file produced when this library is compiled.                                         |
+| The archive file for the linker produced when this library is compiled.                          |
++--------------------------------+-----------------------------------------------------------------+
+| :param:`export_file`           | :type:`File`                                                    |
++--------------------------------+-----------------------------------------------------------------+
+| The archive file for compilation of dependent libraries produced when this library is compiled.  |
++--------------------------------+-----------------------------------------------------------------+
+| :param:`facts_file`            | :type:`File`                                                    |
++--------------------------------+-----------------------------------------------------------------+
+| The serialized facts for this library produced when nogo ran for this library.                   |
 +--------------------------------+-----------------------------------------------------------------+
 | :param:`srcs`                  | :type:`tuple of File`                                           |
 +--------------------------------+-----------------------------------------------------------------+

go/tools/builders/BUILD.bazel

@@ -76,11 +76,11 @@ filegroup(
         "generate_test_main.go",
         "importcfg.go",
         "link.go",
-        "pack.go",
         "read.go",
         "replicate.go",
         "stdlib.go",
         "stdliblist.go",
+        "utils.go",
     ] + select({
         "@bazel_tools//src/conditions:windows": ["path_windows.go"],
         "//conditions:default": ["path.go"],
@@ -97,7 +97,6 @@ go_source(
         "nogo_typeparams_go117.go",
         "nogo_typeparams_go118.go",
         "nolint.go",
-        "pack.go",
     ],
     # //go/tools/builders:nogo_srcs is considered a different target by
     # Bazel's visibility check than

go/tools/builders/ar.go

@@ -23,6 +23,18 @@ import (
 	"strings"
 )
 
+const (
+	// arHeader appears at the beginning of archives created by "ar" and
+	// "go tool pack" on all platforms.
+	arHeader = "!<arch>\n"
+
+	// entryLength is the size in bytes of the metadata preceding each file
+	// in an archive.
+	entryLength = 60
+)
+
+var zeroBytes = []byte("0                    ")
+
 type header struct {
 	NameRaw     [16]byte
 	ModTimeRaw  [12]byte

go/tools/builders/compilepkg.go

@@ -50,9 +50,9 @@ func compilePkg(args []string) error {
 	fs := flag.NewFlagSet("GoCompilePkg", flag.ExitOnError)
 	goenv := envFlags(fs)
 	var unfilteredSrcs, coverSrcs, embedSrcs, embedLookupDirs, embedRoots, recompileInternalDeps multiFlag
-	var deps archiveMultiFlag
+	var deps, facts archiveMultiFlag
 	var importPath, packagePath, nogoPath, packageListPath, coverMode string
-	var outPath, outXPath, cgoExportHPath string
+	var outPath, outXPath, outFactsPath, cgoExportHPath string
 	var testFilter string
 	var gcFlags, asmFlags, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags quoteMultiFlag
 	var coverFormat string
@@ -63,6 +63,7 @@ func compilePkg(args []string) error {
 	fs.Var(&embedLookupDirs, "embedlookupdir", "Root-relative paths to directories relative to which //go:embed directives are resolved")
 	fs.Var(&embedRoots, "embedroot", "Bazel output root under which a file passed via -embedsrc resides")
 	fs.Var(&deps, "arc", "Import path, package path, and file name of a direct dependency, separated by '='")
+	fs.Var(&facts, "facts", "Import path, package path, and file name of a direct dependency's nogo facts file, separated by '='")
 	fs.StringVar(&importPath, "importpath", "", "The import path of the package being compiled. Not passed to the compiler, but may be displayed in debug data.")
 	fs.StringVar(&packagePath, "p", "", "The package path (importmap) of the package being compiled")
 	fs.Var(&gcFlags, "gcflags", "Go compiler flags")
@@ -77,7 +78,8 @@ func compilePkg(args []string) error {
 	fs.StringVar(&packageListPath, "package_list", "", "The file containing the list of standard library packages")
 	fs.StringVar(&coverMode, "cover_mode", "", "The coverage mode to use. Empty if coverage instrumentation should not be added.")
 	fs.StringVar(&outPath, "o", "", "The full output archive file required by the linker")
-	fs.StringVar(&outXPath, "x", "", "The export-only output archive required to compile dependent packages (may includes nogo facts)")
+	fs.StringVar(&outXPath, "x", "", "The export-only output archive required to compile dependent packages")
+	fs.StringVar(&outFactsPath, "out_facts", "", "The file to emit serialized nogo facts to (must be set if -nogo is set")
 	fs.StringVar(&cgoExportHPath, "cgoexport", "", "The _cgo_exports.h file to write")
 	fs.StringVar(&testFilter, "testfilter", "off", "Controls test package filtering")
 	fs.StringVar(&coverFormat, "cover_format", "", "Emit source file paths in coverage instrumentation suitable for the specified coverage format")
@@ -142,6 +144,7 @@ func compilePkg(args []string) error {
 		packagePath,
 		srcs,
 		deps,
+		facts,
 		coverMode,
 		coverSrcs,
 		embedSrcs,
@@ -161,6 +164,7 @@ func compilePkg(args []string) error {
 		packageListPath,
 		outPath,
 		outXPath,
+		outFactsPath,
 		cgoExportHPath,
 		coverFormat,
 		recompileInternalDeps,
@@ -173,6 +177,7 @@ func compileArchive(
 	packagePath string,
 	srcs archiveSrcs,
 	deps []archive,
+	facts []archive,
 	coverMode string,
 	coverSrcs []string,
 	embedSrcs []string,
@@ -192,6 +197,7 @@ func compileArchive(
 	packageListPath string,
 	outPath string,
 	outXPath string,
+	outFactsPath string,
 	cgoExportHPath string,
 	coverFormat string,
 	recompileInternalDeps []string,
@@ -428,7 +434,6 @@ func compileArchive(
 
 	// Run nogo concurrently.
 	var nogoChan chan error
-	outFactsPath := filepath.Join(workDir, nogoFact)
 	nogoSrcs := make([]string, 0, len(goSrcs))
 	for _, goSrc := range goSrcs {
 		// If source is found in the origin map, that means it's likely to be a generated source file
@@ -449,11 +454,11 @@ func compileArchive(
 		// Add unknown origin source files into the mix.
 		nogoSrcs = append(nogoSrcs, goSrc)
 	}
-	if nogoPath != "" && len(nogoSrcs) > 0 {
+	if nogoPath != "" {
 		ctx, cancel := context.WithCancel(context.Background())
 		nogoChan = make(chan error)
 		go func() {
-			nogoChan <- runNogo(ctx, workDir, nogoPath, nogoSrcs, deps, packagePath, importcfgPath, outFactsPath)
+			nogoChan <- runNogo(ctx, workDir, nogoPath, nogoSrcs, facts, packagePath, importcfgPath, outFactsPath)
 		}()
 		defer func() {
 			if nogoChan != nil {
@@ -517,27 +522,21 @@ func compileArchive(
 	// Pack .o files into the archive. These may come from cgo generated code,
 	// cgo dependencies (cdeps), or assembly.
 	if len(objFiles) > 0 {
-		if err := appendFiles(goenv, outPath, objFiles...); err != nil {
+		if err := appendToArchive(goenv, outPath, objFiles); err != nil {
 			return err
 		}
 	}
 
 	// Check results from nogo.
-	nogoStatus := nogoNotRun
 	if nogoChan != nil {
 		err := <-nogoChan
 		nogoChan = nil // no cancellation needed
 		if err != nil {
-			nogoStatus = nogoFailed
-			// TODO: should we still create the .x file without nogo facts in this case?
+			// TODO: Move nogo into a separate action so we don't fail the compilation here.
 			return err
 		}
-		nogoStatus = nogoSucceeded
 	}
 
-	if nogoStatus == nogoSucceeded {
-		return appendFiles(goenv, outXPath, outFactsPath)
-	}
 	return nil
 }
 
@@ -565,12 +564,16 @@ func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPa
 	return goenv.runCommand(args)
 }
 
-func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string, deps []archive, packagePath, importcfgPath, outFactsPath string) error {
+func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string, facts []archive, packagePath, importcfgPath, outFactsPath string) error {
+	if len(srcs) == 0 {
+		// emit_compilepkg expects a nogo facts file, even if it's empty.
+		return os.WriteFile(outFactsPath, nil, 0o666)
+	}
 	args := []string{nogoPath}
 	args = append(args, "-p", packagePath)
 	args = append(args, "-importcfg", importcfgPath)
-	for _, dep := range deps {
-		args = append(args, "-fact", fmt.Sprintf("%s=%s", dep.importPath, dep.file))
+	for _, fact := range facts {
+		args = append(args, "-fact", fmt.Sprintf("%s=%s", fact.importPath, fact.file))
 	}
 	args = append(args, "-x", outFactsPath)
 	args = append(args, srcs...)
@@ -600,6 +603,15 @@ func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string
 	return nil
 }
 
+func appendToArchive(goenv *env, outPath string, objFiles []string) error {
+	// Use abs to work around long path issues on Windows.
+	// TODO(jayconrod): copy cmd/internal/archive and use that instead of
+	// shelling out to cmd/pack.
+	args := goenv.goTool("pack", "r", abs(outPath))
+	args = append(args, objFiles...)
+	return goenv.runCommand(args)
+}
+
 func createTrimPath(gcFlags []string, path string) string {
 	for _, flag := range gcFlags {
 		if strings.HasPrefix(flag, "-trimpath=") {

go/tools/builders/nogo_main.go

@@ -610,8 +610,8 @@ func (i *importer) Import(path string) (*types.Package, error) {
 }
 
 func (i *importer) readFacts(pkgPath string) ([]byte, error) {
-	archive := i.factMap[pkgPath]
-	if archive == "" {
+	facts := i.factMap[pkgPath]
+	if facts == "" {
 		// Packages that were not built with the nogo toolchain will not be
 		// analyzed, so there's no opportunity to store facts. This includes
 		// packages in the standard library and packages built with go_tool_library,
@@ -621,18 +621,7 @@ func (i *importer) readFacts(pkgPath string) ([]byte, error) {
 		// fmt.Printf accepts a format string.
 		return nil, nil
 	}
-	factReader, err := readFileInArchive(nogoFact, archive)
-	if os.IsNotExist(err) {
-		// Packages that were not built with the nogo toolchain will not be
-		// analyzed, so there's no opportunity to store facts. This includes
-		// packages in the standard library and packages built with go_tool_library,
-		// such as coverdata.
-		return nil, nil
-	} else if err != nil {
-		return nil, err
-	}
-	defer factReader.Close()
-	return ioutil.ReadAll(factReader)
+	return os.ReadFile(facts)
 }
 
 type factMultiFlag map[string]string