Skip to content

Commit

Permalink
GoSource can store dependent archives instead of full deps (#4041)
Browse files Browse the repository at this point in the history
**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**
  • Loading branch information
dzbarsky authored Aug 15, 2024
1 parent 02587bf commit a32f3e1
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 24 deletions.
3 changes: 1 addition & 2 deletions go/private/actions/archive.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ load(
"GoArchive",
"GoArchiveData",
"effective_importpath_pkgpath",
"get_archive",
)
load(
"//go/private/actions:compilepkg.bzl",
Expand Down Expand Up @@ -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:
Expand Down
31 changes: 17 additions & 14 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ load(
"GoSource",
"GoStdLib",
"INFERRED_PATH",
"get_archive",
"get_source",
)

Expand Down Expand Up @@ -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...
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down
13 changes: 6 additions & 7 deletions go/private/rules/test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ load(
"GoLibrary",
"GoSource",
"INFERRED_PATH",
"get_archive",
)
load(
"//go/private/rules:binary.bzl",
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion go/providers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
+--------------------------------+-----------------------------------------------------------------+
Expand Down

0 comments on commit a32f3e1

Please sign in to comment.