From a32f3e18ba26f18e5f9449e7925a194591e2f8dd Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Thu, 15 Aug 2024 03:02:10 -0400 Subject: [PATCH] GoSource can store dependent archives instead of full deps (#4041) **What type of PR is this?** Starlark cleanup **What does this PR do? Why is it needed?** Trims the amount of information we are passing in the provider **Which issues(s) does this PR fix?** Fixes # **Other notes for review** --- go/private/actions/archive.bzl | 3 +-- go/private/context.bzl | 31 +++++++++++++++++-------------- go/private/rules/test.bzl | 13 ++++++------- go/providers.rst | 2 +- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl index 8b0d1dfd5..60a5fae45 100644 --- a/go/private/actions/archive.bzl +++ b/go/private/actions/archive.bzl @@ -31,7 +31,6 @@ load( "GoArchive", "GoArchiveData", "effective_importpath_pkgpath", - "get_archive", ) load( "//go/private/actions:compilepkg.bzl", @@ -74,7 +73,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d out_nogo_log = None out_nogo_validation = None - direct = [get_archive(dep) for dep in source.deps] + direct = source.deps files = [] for a in direct: diff --git a/go/private/context.bzl b/go/private/context.bzl index 4e446e2bd..de0522bf7 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -68,6 +68,7 @@ load( "GoSource", "GoStdLib", "INFERRED_PATH", + "get_archive", "get_source", ) @@ -236,28 +237,25 @@ def _merge_embed(source, embed): source["clinkopts"] = source["clinkopts"] or s.clinkopts source["cgo_exports"] = source["cgo_exports"] + s.cgo_exports -def _dedup_deps(deps): - """Returns a list of deps without duplicate import paths. +def _dedup_archives(archives): + """Returns a list of archives without duplicate import paths. - Earlier targets take precedence over later targets. This is intended to + Earlier archives take precedence over later targets. This is intended to allow an embedding library to override the dependencies of its embedded libraries. Args: - deps: an iterable containing either Targets or GoArchives. + archives: an iterable of GoArchives. """ - deduped_deps = [] + deduped_archives = [] importpaths = {} - for dep in deps: - if hasattr(dep, "data") and hasattr(dep.data, "importpath"): - importpath = dep.data.importpath - else: - importpath = dep[GoLibrary].importpath + for arc in archives: + importpath = arc.data.importpath if importpath in importpaths: continue importpaths[importpath] = None - deduped_deps.append(dep) - return deduped_deps + deduped_archives.append(arc) + return deduped_archives def _library_to_source(go, attr, library, coverage_instrumented): #TODO: stop collapsing a depset in this line... @@ -268,6 +266,7 @@ def _library_to_source(go, attr, library, coverage_instrumented): attr_deps = getattr(attr, "deps", []) generated_deps = getattr(library, "deps", []) deps = attr_deps + generated_deps + deps = [get_archive(dep) for dep in deps] source = { "library": library, "mode": go.mode, @@ -295,7 +294,9 @@ def _library_to_source(go, attr, library, coverage_instrumented): for e in getattr(attr, "embed", []): _check_binary_dep(go, e, "embed") _merge_embed(source, e) - source["deps"] = _dedup_deps(source["deps"]) + + source["deps"] = _dedup_archives(source["deps"]) + x_defs = source["x_defs"] for k, v in getattr(attr, "x_defs", {}).items(): v = _expand_location(go, attr, v) @@ -314,7 +315,9 @@ def _library_to_source(go, attr, library, coverage_instrumented): fail("source {} has C/C++ extension, but cgo was not enabled (set 'cgo = True')".format(f.path)) if library.resolve: library.resolve(go, attr, source, _merge_embed) - source["cc_info"] = _collect_cc_infos(source["deps"], source["cdeps"]) + + source["cc_info"] = _collect_cc_infos(attr_deps + generated_deps, source["cdeps"]) + return GoSource(**source) def _collect_runfiles(go, data, deps): diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index ab3cf2eb6..c6cba0586 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -40,7 +40,6 @@ load( "GoLibrary", "GoSource", "INFERRED_PATH", - "get_archive", ) load( "//go/private/rules:binary.bzl", @@ -525,7 +524,7 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe # Build a map from labels to GoArchiveData. # If none of the librares embedded in the internal archive are in the # dependency graph, then nothing needs to be recompiled. - arc_data_list = depset(transitive = [get_archive(dep).transitive for dep in external_source.deps]).to_list() + arc_data_list = depset(transitive = [archive.transitive for archive in external_source.deps]).to_list() label_to_arc_data = {a.label: a for a in arc_data_list} if all([l not in label_to_arc_data for l in library_labels]): return external_source, internal_archive @@ -543,7 +542,7 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe dep_list = [] # stack is a stack of targets to process. We're done when it's empty. - stack = [get_archive(dep).data.label for dep in external_source.deps] + stack = [archive.data.label for archive in external_source.deps] # deps_pushed tracks the status of each target. # DEPS_UNPROCESSED means the target is on the stack, but its dependencies @@ -622,10 +621,10 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe # 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 + for archive in internal_source.deps: + dep_data = archive.data if not need_recompile[dep_data.label]: - internal_deps.append(dep) + internal_deps.append(archive) else: need_recompile_deps.append(dep_data.importpath) @@ -712,5 +711,5 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe # Finally, we need to replace external_source.deps with the recompiled # archives. attrs = structs.to_dict(external_source) - attrs["deps"] = [label_to_archive[get_archive(dep).data.label] for dep in external_source.deps] + attrs["deps"] = [label_to_archive[archive.data.label] for archive in external_source.deps] return GoSource(**attrs), internal_archive diff --git a/go/providers.rst b/go/providers.rst index fe7161031..a289773c2 100644 --- a/go/providers.rst +++ b/go/providers.rst @@ -149,7 +149,7 @@ method. In general, only rules_go should need to build or handle these. +--------------------------------+-----------------------------------------------------------------+ | Map of defines to add to the go link command. | +--------------------------------+-----------------------------------------------------------------+ -| :param:`deps` | :type:`list of Target` | +| :param:`deps` | :type:`list of GoArchive` | +--------------------------------+-----------------------------------------------------------------+ | The direct dependencies needed by this library. | +--------------------------------+-----------------------------------------------------------------+