diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl index 42323f1579..ef60b6a811 100644 --- a/go/private/actions/archive.bzl +++ b/go/private/actions/archive.bzl @@ -39,7 +39,8 @@ load( "emit_compilepkg", ) -def emit_archive(go, source = None, _recompile_suffix = "", extra_archive_datas = []): + +def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_deps = None, extra_archive_datas = []): """See go/toolchains.rst#archive for full documentation.""" if source == None: @@ -132,6 +133,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", extra_archive_datas gc_goopts = source.gc_goopts, cgo = False, testfilter = testfilter, + recompile_internal_deps = recompile_internal_deps, ) data = GoArchiveData( diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl index f4e5a0c93f..28c32123f5 100644 --- a/go/private/actions/compilepkg.bzl +++ b/go/private/actions/compilepkg.bzl @@ -59,7 +59,8 @@ def emit_compilepkg( out_export = None, out_cgo_export_h = None, gc_goopts = [], - testfilter = None): # TODO: remove when test action compiles packages + testfilter = None, # TODO: remove when test action compiles packages + recompile_internal_deps = []): """Compiles a complete Go package.""" if sources == None: fail("sources is a required parameter") @@ -99,6 +100,8 @@ def emit_compilepkg( args.add("-cover_format", go.cover_format) args.add_all(cover, before_each = "-cover") args.add_all(archives, before_each = "-arc", map_each = _archive) + if recompile_internal_deps: + args.add_all(recompile_internal_deps, before_each = "-recompile_internal_deps") if importpath: args.add("-importpath", importpath) else: diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index 92193caf84..8017bba067 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -591,14 +591,26 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe # is shared between the internal and external archive. The internal archive # can't import anything that imports itself. internal_source = internal_archive.source - internal_deps = [dep for dep in internal_source.deps if not need_recompile[get_archive(dep).data.label]] + + internal_deps = [] + + # Pass internal dependencies that need to be recompiled down to the builder to check if the internal archive + # tries to import any of the dependencies. If there is, that means that there is a dependency cycle. + need_recompile_deps = [] + for dep in internal_source.deps: + dep_data = get_archive(dep).data + if not need_recompile[dep_data.label]: + internal_deps.append(dep) + else: + need_recompile_deps.append(dep_data.importpath) + x_defs = dict(internal_source.x_defs) x_defs.update(internal_archive.x_defs) attrs = structs.to_dict(internal_source) attrs["deps"] = internal_deps attrs["x_defs"] = x_defs internal_source = GoSource(**attrs) - internal_archive = go.archive(go, internal_source, _recompile_suffix = ".recompileinternal") + internal_archive = go.archive(go, internal_source, _recompile_suffix = ".recompileinternal", recompile_internal_deps = need_recompile_deps) # Build a map from labels to possibly recompiled GoArchives. label_to_archive = {} diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index b788f97203..a88178c89a 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -49,7 +49,7 @@ func compilePkg(args []string) error { fs := flag.NewFlagSet("GoCompilePkg", flag.ExitOnError) goenv := envFlags(fs) - var unfilteredSrcs, coverSrcs, embedSrcs, embedLookupDirs, embedRoots multiFlag + var unfilteredSrcs, coverSrcs, embedSrcs, embedLookupDirs, embedRoots, recompileInternalDeps multiFlag var deps archiveMultiFlag var importPath, packagePath, nogoPath, packageListPath, coverMode string var outPath, outFactsPath, cgoExportHPath string @@ -82,6 +82,7 @@ func compilePkg(args []string) error { 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") + fs.Var(&recompileInternalDeps, "recompile_internal_deps", "The import path of the direct dependencies that needs to be recompiled.") if err := fs.Parse(args); err != nil { return err } @@ -162,7 +163,8 @@ func compilePkg(args []string) error { outPath, outFactsPath, cgoExportHPath, - coverFormat) + coverFormat, + recompileInternalDeps) } func compileArchive( @@ -192,6 +194,7 @@ func compileArchive( outXPath string, cgoExportHPath string, coverFormat string, + recompileInternalDeps []string, ) error { workDir, cleanup, err := goenv.workDir() if err != nil { @@ -363,7 +366,7 @@ func compileArchive( // Check that the filtered sources don't import anything outside of // the standard library and the direct dependencies. - imports, err := checkImports(srcs.goSrcs, deps, packageListPath) + imports, err := checkImports(srcs.goSrcs, deps, packageListPath, importPath, recompileInternalDeps) if err != nil { return err } diff --git a/go/tools/builders/importcfg.go b/go/tools/builders/importcfg.go index 72dedad0c1..9fe55b420f 100644 --- a/go/tools/builders/importcfg.go +++ b/go/tools/builders/importcfg.go @@ -33,11 +33,11 @@ type archive struct { } // checkImports verifies that each import in files refers to a -// direct dependendency in archives or to a standard library package +// direct dependency in archives or to a standard library package // listed in the file at stdPackageListPath. checkImports returns // a map from source import paths to elements of archives or to nil // for standard library packages. -func checkImports(files []fileInfo, archives []archive, stdPackageListPath string) (map[string]*archive, error) { +func checkImports(files []fileInfo, archives []archive, stdPackageListPath string, importPath string, recompileInternalDeps []string) (map[string]*archive, error) { // Read the standard package list. packagesTxt, err := ioutil.ReadFile(stdPackageListPath) if err != nil { @@ -71,7 +71,11 @@ func checkImports(files []fileInfo, archives []archive, stdPackageListPath strin importAliasToArchive[imp] = arc } } - + // Construct recompileInternalDeps as a map to check if there are imports that are disallowed. + recompileInternalDepMap := make(map[string]struct{}) + for _, dep := range recompileInternalDeps { + recompileInternalDepMap[dep] = struct{}{} + } // Build the import map. imports := make(map[string]*archive) var derr depsError @@ -83,6 +87,9 @@ func checkImports(files []fileInfo, archives []archive, stdPackageListPath strin // errors for them here, but they will probably break something else. continue } + if _, ok := recompileInternalDepMap[path]; ok { + return nil, fmt.Errorf("dependency cycle detected between %q and %q in file %q", importPath, path, f.filename) + } if stdPkgs[path] { imports[path] = nil } else if arc := importToArchive[path]; arc != nil {