Skip to content

Commit

Permalink
De-duplicate dependencies with the same import path (#1785)
Browse files Browse the repository at this point in the history
go_library may depend on a library in deps with the same importpath as
a library in the deps of an embedded library. The embedding
go_library's dependency takes precedence (in other words, the embedder
overrides).

Previously, we could end up with multiple libraries with the same
importpath in the importcfg for the compiler. The compiler does not
report an error and accepts the last entry, which would come from the
embedded library. This has led to package height errors with
go_proto_library.

Fixes #1772
  • Loading branch information
jayconrod authored and Jay Conrod committed Oct 24, 2018
1 parent 4332caf commit 5ae4814
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 0 deletions.
23 changes: 23 additions & 0 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,28 @@ def _merge_embed(source, embed):
fail("multiple libraries with cgo_archives embedded")
source["cgo_archives"] = s.cgo_archives

def _dedup_deps(deps):
"""Returns a list of targets without duplicate import paths.
Earlier targets take precedence over later targets. This is intended to
allow an embedding library to override the dependencies of its
embedded libraries.
"""
deduped_deps = []
importpaths = {}
for dep in deps:
# TODO(#1784): we allow deps to be a list of GoArchive since go_test and
# nogo work this way. We should force deps to be a list of Targets.
if hasattr(dep, "data") and hasattr(dep.data, "importpath"):
importpath = dep.data.importpath
else:
importpath = dep[GoLibrary].importpath
if importpath in importpaths:
continue
importpaths[importpath] = None
deduped_deps.append(dep)
return deduped_deps

def _library_to_source(go, attr, library, coverage_instrumented):
#TODO: stop collapsing a depset in this line...
attr_srcs = [f for t in getattr(attr, "srcs", []) for f in as_iterable(t.files)]
Expand All @@ -158,6 +180,7 @@ def _library_to_source(go, attr, library, coverage_instrumented):
source["cover"] = attr_srcs
for e in getattr(attr, "embed", []):
_merge_embed(source, e)
source["deps"] = _dedup_deps(source["deps"])
x_defs = source["x_defs"]
for k, v in getattr(attr, "x_defs", {}).items():
if "." not in k:
Expand Down
37 changes: 37 additions & 0 deletions tests/core/go_library/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,40 @@ go_library(
],
importpath = "asm_header",
)

go_library(
name = "package_height",
srcs = ["package_height.go"],
importpath = "package_height",
deps = [
":package_height_dep_deep",
":package_height_embedder",
],
)

go_library(
name = "package_height_embedder",
srcs = ["package_height_embedder.go"],
embed = [":package_height_embeddee"],
importpath = "package_height/embed",
deps = [":package_height_dep_deep"],
)

go_library(
name = "package_height_embeddee",
srcs = ["package_height_embeddee.go"],
importpath = "package_height/embed",
deps = [":package_height_dep_shallow"],
)

go_library(
name = "package_height_dep_deep",
srcs = ["package_height_dep_deep.go"],
importpath = "package_height/dep",
)

go_library(
name = "package_height_dep_shallow",
srcs = ["package_height_dep_shallow.go"],
importpath = "package_height/dep",
)
7 changes: 7 additions & 0 deletions tests/core/go_library/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Basic go_library functionality
.. _go_library: /go/core.rst#_go_library
.. #1262: https://github.com/bazelbuild/rules_go/issues/1262
.. #1520: https://github.com/bazelbuild/rules_go/issues/1520
.. #1772: https://github.com/bazelbuild/rules_go/issues/1772
empty
-----
Expand All @@ -22,3 +23,9 @@ asm_header

Checks that assembly files in a `go_library`_ may include ``"go_asm.h"``,
generated by the compiler. Verifies `#1262`_.

package_height
--------------

Checks that when a library embeds another library, the embedder's dependencies
may override the embeddee's dependencies. Verifies `#1772`_.
6 changes: 6 additions & 0 deletions tests/core/go_library/package_height.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package height

import "package_height/embed"
import "package_height/dep"

var X = embed.T{F: dep.T{}}
7 changes: 7 additions & 0 deletions tests/core/go_library/package_height_dep_deep.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package dep

import "os"

type T struct {
F *os.File
}
3 changes: 3 additions & 0 deletions tests/core/go_library/package_height_dep_shallow.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package dep

type T struct{}
7 changes: 7 additions & 0 deletions tests/core/go_library/package_height_embeddee.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package embed

import "package_height/dep"

type T struct {
F dep.T
}
3 changes: 3 additions & 0 deletions tests/core/go_library/package_height_embedder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package embed

var X = T{}

0 comments on commit 5ae4814

Please sign in to comment.