[go] Enable declaration of recursive package in GN
This change is effectively a no-op from a build perspective. It adds a
'/...' suffix to some package definitions in GN and change build/go/build.py to
tolerate these.
The '/...' Go package suffix is a well known and documented format in Go to
declare recursivity.
The end goal is to change the go package naming convention:
- A GN Go package ending with '/...' means that the package includes all
children packages.
- A GN Go package without this suffix means that no children packages is
included.
The status quo at the moment is that all children Go packages are included.
This causes all sorts of issues and surprises. This caused the proliferation of
"lib" pseudo-packages under //tools and the anti-pattern of the relative
directory not matching the relative Go package name.
Switching the logic requires updating a lot of packages so this will be
done over the course for several CLs.
All third_parties are effectively used as recursive dependencies so
update third_party/golibs/BUILD.gn right away, to show how it'll look
like in the end state for recursive packages.
Bug: 55387
Change-Id: Ib25e65230594d5c0469642a2e36f266499a9c85a
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/404216
Commit-Queue: Marc-Antoine Ruel <maruel@google.com>
Reviewed-by: Shai Barack <shayba@google.com>
Reviewed-by: Petr Hosek <phosek@google.com>
Testability-Review: Shai Barack <shayba@google.com>
diff --git a/build/go/build.py b/build/go/build.py
index 29db351..4177862 100755
--- a/build/go/build.py
+++ b/build/go/build.py
@@ -126,33 +126,55 @@
project_path = os.path.join(
args.root_out_dir, 'gen', 'gopaths', args.binname)
- # Clean up any old project path to avoid leaking old dependencies
- shutil.rmtree(os.path.join(project_path, 'src'), ignore_errors=True)
- os.makedirs(os.path.join(project_path, 'src'))
+ # Clean up any old project path to avoid leaking old dependencies.
+ gopath_src = os.path.join(project_path, 'src')
+ if os.path.exists(gopath_src):
+ shutil.rmtree(gopath_src)
+ os.makedirs(gopath_src)
if args.go_dep_files:
- # Create a gopath for the packages dependency tree
- for dst, src in list(get_sources(args.go_dep_files).items()):
- dstdir = os.path.join(project_path, 'src', os.path.dirname(dst))
- try:
+ # Create a GOPATH for the packages dependency tree.
+ for dst, src in sorted(get_sources(args.go_dep_files).items()):
+ # Determine if the package should be mapped recursively or only the
+ # package source itself should be mapped.
+ #
+ # There are cases (e.g for fidl generated source) where a single
+ # source is directly mapped instead of a formal Go package path.
+ # This is incorrect but happened to work, so people started to
+ # depend on that. In this case, we use the recurse mode, which
+ # happens to do what is needed; map the file directly as a symlink.
+ recurse = dst.endswith('.go')
+ # - src can have a '/...' suffix like with
+ # 'github.com/google/go-cmp/...'.
+ # - dst have the suffix when defining a package.
+ # - src can only have the suffix if dst has it too.
+ assert dst.endswith('/...') >= src.endswith('/...'), (dst, src)
+ if dst.endswith('/...'):
+ dst = dst[:-4]
+ recurse = True
+ if src.endswith('/...'):
+ src = src[:-4]
+ dstdir = os.path.join(gopath_src, dst)
+
+ # TODO(fxb/55387): Enable non-recursive mode.
+ recurse = True
+
+ if recurse:
+ # Map the whole directory, which implicitly makes Go subpackages
+ # (subdirectories) available.
+ parent = os.path.dirname(dstdir)
+ if not os.path.exists(parent):
+ os.makedirs(parent)
+ os.symlink(src, dstdir)
+ else:
+ # Map individual files since the dependency is only on the
+ # package itself, not Go subpackages. The only exception is
+ # 'testdata'.
os.makedirs(dstdir)
- except OSError as e:
- # EEXIST occurs if two gopath entries share the same parent name
- if e.errno != errno.EEXIST:
- raise
- # TODO(BLD-228): the following check might not be necessary anymore.
- tgt = os.path.join(dstdir, os.path.basename(dst))
- # The source tree is effectively read-only once the build begins.
- # Therefore it is an error if tgt is in the source tree. At first
- # glance this may seem impossible, but it can happen if dst is foo/bar
- # and foo is a symlink back to the source tree.
- canon_root_out_dir = os.path.realpath(args.root_out_dir)
- canon_tgt = os.path.realpath(tgt)
- if not canon_tgt.startswith(canon_root_out_dir):
- raise ValueError(
- "Dependency destination not in --root-out-dir: provided=%s, path=%s, realpath=%s"
- % (dst, tgt, canon_tgt))
- os.symlink(os.path.relpath(src, os.path.dirname(tgt)), tgt)
+ for filename in os.listdir(src):
+ src_file = os.path.join(src, filename)
+ if filename == 'testdata' or os.path.isfile(src_file):
+ os.symlink(src_file, os.path.join(dstdir, filename))
cflags = []
if args.sysroot:
@@ -222,7 +244,7 @@
cmd += ['-ldflags=' + ' '.join(args.ldflag)]
cmd += [
'-pkgdir',
- os.path.join(project_path, 'pkg'), '-o', args.output_path, args.package
+ os.path.join(project_path, 'pkg'), '-o', args.output_path, args.package,
]
retcode = subprocess.call(cmd, env=env)
diff --git a/build/go/gen_library_metadata.py b/build/go/gen_library_metadata.py
index 98353fc..d818a1f 100755
--- a/build/go/gen_library_metadata.py
+++ b/build/go/gen_library_metadata.py
@@ -48,7 +48,7 @@
print(' - %s (%s)' % (src.path, src.file))
raise Exception('Could not aggregate sources')
- return dict([(s.name, s.path) for s in sources])
+ return {s.name: s.path for s in sources}
def main():
diff --git a/third_party/golibs/BUILD.gn b/third_party/golibs/BUILD.gn
index d94d38d..282e5d9 100644
--- a/third_party/golibs/BUILD.gn
+++ b/third_party/golibs/BUILD.gn
@@ -6,7 +6,7 @@
template("golib") {
go_library(target_name) {
- name = target_name
+ name = target_name + "/..."
source_dir = "//third_party/golibs/$name"
forward_variables_from(invoker, [ "deps" ])
}
@@ -16,9 +16,11 @@
### cloud.google.com/*
go_library("cloud.google.com/go/compute/metadata") {
+ name = "cloud.google.com/go/compute/metadata/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-cloud-go/compute/metadata"
}
go_library("cloud.google.com/go/iam") {
+ name = "cloud.google.com/go/iam/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-cloud-go/iam"
deps = [
":cloud.google.com/go/internal",
@@ -31,6 +33,7 @@
# Actually aggregates the dependencies of other cloud.google.com/go/internal/*
# packages as well. Use this as a convenience target for any such package.
go_library("cloud.google.com/go/internal") {
+ name = "cloud.google.com/go/internal/..."
source_dir =
"//third_party/golibs/github.com/googleapis/google-cloud-go/internal"
deps = [
@@ -44,6 +47,7 @@
]
}
go_library("cloud.google.com/go/storage") {
+ name = "cloud.google.com/go/storage/..."
source_dir =
"//third_party/golibs/github.com/googleapis/google-cloud-go/storage"
deps = [
@@ -132,6 +136,7 @@
# Actually aggregates the dependencies of other go.opencensus.io/*
# packages as well. Use this as a convenience target for any such package.
go_library("go.opencensus.io") {
+ name = "go.opencensus.io/..."
source_dir =
"//third_party/golibs/github.com/census-instrumentation/opencensus-go"
deps = [
@@ -165,15 +170,18 @@
golib("golang.org/x/sys") {
}
go_library("golang.org/x/text") {
+ name = "golang.org/x/text/..."
source_dir = "//third_party/golibs/github.com/golang/text"
}
### google.golang.org/*
go_library("google.golang.org/genproto/googleapis/api/annotations") {
+ name = "google.golang.org/genproto/googleapis/api/annotations/..."
source_dir = "//third_party/golibs/github.com/googleapis/go-genproto/googleapis/api/annotations"
deps = [ ":github.com/golang/protobuf" ]
}
go_library("google.golang.org/genproto/googleapis/iam/v1") {
+ name = "google.golang.org/genproto/googleapis/iam/v1/..."
source_dir =
"//third_party/golibs/github.com/googleapis/go-genproto/googleapis/iam/v1"
deps = [
@@ -184,14 +192,17 @@
]
}
go_library("google.golang.org/genproto/googleapis/rpc/code") {
+ name = "google.golang.org/genproto/googleapis/rpc/code/..."
source_dir = "//third_party/golibs/github.com/googleapis/go-genproto/googleapis/rpc/code"
deps = [ ":github.com/golang/protobuf" ]
}
go_library("google.golang.org/genproto/googleapis/rpc/status") {
+ name = "google.golang.org/genproto/googleapis/rpc/status/..."
source_dir = "//third_party/golibs/github.com/googleapis/go-genproto/googleapis/rpc/status"
deps = [ ":github.com/golang/protobuf" ]
}
go_library("google.golang.org/genproto/googleapis/type/expr") {
+ name = "google.golang.org/genproto/googleapis/type/expr/..."
source_dir = "//third_party/golibs/github.com/googleapis/go-genproto/googleapis/type/expr"
deps = [ ":github.com/golang/protobuf" ]
}
@@ -199,6 +210,7 @@
# Actually aggregates the dependencies of other google.golang.org/grpc/*
# packages as well. Use this as a convenience target for any such package.
go_library("google.golang.org/grpc") {
+ name = "google.golang.org/grpc/..."
source_dir = "//third_party/golibs/github.com/grpc/grpc-go"
deps = [
":golang.org/x/net",
@@ -207,13 +219,16 @@
]
}
go_library("google.golang.org/api/gensupport") {
+ name = "google.golang.org/api/gensupport/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-api-go-client/gensupport"
deps = [ ":google.golang.org/api/googleapi" ]
}
go_library("google.golang.org/api/googleapi") {
+ name = "google.golang.org/api/googleapi/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-api-go-client/googleapi"
}
go_library("google.golang.org/api/internal") {
+ name = "google.golang.org/api/internal/..."
source_dir =
"//third_party/golibs/github.com/googleapis/google-api-go-client/internal"
deps = [
@@ -222,10 +237,12 @@
]
}
go_library("google.golang.org/api/iterator") {
+ name = "google.golang.org/api/iterator/..."
source_dir =
"//third_party/golibs/github.com/googleapis/google-api-go-client/iterator"
}
go_library("google.golang.org/api/option") {
+ name = "google.golang.org/api/option/..."
source_dir =
"//third_party/golibs/github.com/googleapis/google-api-go-client/option"
deps = [
@@ -235,6 +252,7 @@
]
}
go_library("google.golang.org/api/storage/v1") {
+ name = "google.golang.org/api/storage/v1/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-api-go-client/storage/v1"
deps = [
":google.golang.org/api/gensupport",
@@ -244,6 +262,7 @@
]
}
go_library("google.golang.org/api/transport/http") {
+ name = "google.golang.org/api/transport/http/..."
source_dir = "//third_party/golibs/github.com/googleapis/google-api-go-client/transport/http"
deps = [
":go.opencensus.io",
@@ -255,7 +274,7 @@
### gopkg.in/*
go_library("gopkg.in/check.v1") {
- name = "gopkg.in/check.v1"
+ name = "gopkg.in/check.v1/..."
source_dir = "//third_party/golibs/github.com/go-check/check"
deps = [ ":github.com/kr/pretty" ]
}
