From d1da1bb5fda93fb220c0793bad496cc0ce1e4ef3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 27 Sep 2023 08:15:25 +0200 Subject: [PATCH] Infer importpath if not set explicitly (#3705) * Infer importpath if not set explicitely * Add test with explicit and implicit importpath --------- Co-authored-by: Zhongpeng Lin --- go/private/context.bzl | 10 +++--- go/private/rules/library.bzl | 3 -- proto/def.bzl | 16 ++++----- .../go_proto_library_importpath/BUILD.bazel | 36 +++++++++++++++++++ .../go_proto_library_importpath/bar.proto | 9 +++++ .../go_proto_library_importpath/foo.proto | 7 ++++ .../importpath_test.go | 22 ++++++++++++ 7 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 tests/core/go_proto_library_importpath/BUILD.bazel create mode 100644 tests/core/go_proto_library_importpath/bar.proto create mode 100644 tests/core/go_proto_library_importpath/foo.proto create mode 100644 tests/core/go_proto_library_importpath/importpath_test.go diff --git a/go/private/context.bzl b/go/private/context.bzl index 3b494dedd0..b1699cf996 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -357,17 +357,17 @@ def _check_importpaths(ctx): if ":" in p: fail("import path '%s' contains invalid character :" % p) -def _infer_importpath(ctx): +def _infer_importpath(ctx, attr): DEFAULT_LIB = "go_default_library" VENDOR_PREFIX = "/vendor/" # Check if paths were explicitly set, either in this rule or in an # embedded rule. - attr_importpath = getattr(ctx.attr, "importpath", "") - attr_importmap = getattr(ctx.attr, "importmap", "") + attr_importpath = getattr(attr, "importpath", "") + attr_importmap = getattr(attr, "importmap", "") embed_importpath = "" embed_importmap = "" - for embed in getattr(ctx.attr, "embed", []): + for embed in getattr(attr, "embed", []): if GoLibrary not in embed: continue lib = embed[GoLibrary] @@ -504,7 +504,7 @@ def go_context(ctx, attr = None): toolchain.sdk.tools) _check_importpaths(ctx) - importpath, importmap, pathtype = _infer_importpath(ctx) + importpath, importmap, pathtype = _infer_importpath(ctx, attr) importpath_aliases = tuple(getattr(attr, "importpath_aliases", ())) return struct( diff --git a/go/private/rules/library.bzl b/go/private/rules/library.bzl index 6f96c0845a..eeb7952442 100644 --- a/go/private/rules/library.bzl +++ b/go/private/rules/library.bzl @@ -29,7 +29,6 @@ load( load( "//go/private:providers.bzl", "GoLibrary", - "INFERRED_PATH", ) load( "//go/private/rules:transition.bzl", @@ -39,8 +38,6 @@ load( def _go_library_impl(ctx): """Implements the go_library() rule.""" go = go_context(ctx) - if go.pathtype == INFERRED_PATH: - fail("importpath must be specified in this library or one of its embedded libraries") library = go.new_library(go) source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented()) archive = go.archive(go, source) diff --git a/proto/def.bzl b/proto/def.bzl index 381078ec98..073af3afe1 100644 --- a/proto/def.bzl +++ b/proto/def.bzl @@ -31,10 +31,6 @@ load( "//go/private:go_toolchain.bzl", "GO_TOOLCHAIN", ) -load( - "//go/private:providers.bzl", - "INFERRED_PATH", -) load( "//go/private/rules:transition.bzl", "non_go_tool_transition", @@ -46,7 +42,7 @@ load( GoProtoImports = provider() -def get_imports(attr): +def get_imports(attr, importpath): proto_deps = [] # ctx.attr.proto is a one-element array since there is a Starlark transition attached to it. @@ -60,7 +56,7 @@ def get_imports(attr): direct = dict() for dep in proto_deps: for src in dep[ProtoInfo].check_deps_sources.to_list(): - direct["{}={}".format(proto_path(src, dep[ProtoInfo]), attr.importpath)] = True + direct["{}={}".format(proto_path(src, dep[ProtoInfo]), importpath)] = True deps = getattr(attr, "deps", []) + getattr(attr, "embed", []) transitive = [ @@ -71,7 +67,8 @@ def get_imports(attr): return depset(direct = direct.keys(), transitive = transitive) def _go_proto_aspect_impl(_target, ctx): - imports = get_imports(ctx.rule.attr) + go = go_context(ctx, ctx.rule.attr) + imports = get_imports(ctx.rule.attr, go.importpath) return [GoProtoImports(imports = imports)] _go_proto_aspect = aspect( @@ -80,6 +77,7 @@ _go_proto_aspect = aspect( "deps", "embed", ], + toolchains = [GO_TOOLCHAIN], ) def _proto_library_to_source(_go, attr, source, merge): @@ -93,8 +91,6 @@ def _proto_library_to_source(_go, attr, source, merge): def _go_proto_library_impl(ctx): go = go_context(ctx) - if go.pathtype == INFERRED_PATH: - fail("importpath must be specified in this library or one of its embedded libraries") if ctx.attr.compiler: #TODO: print("DEPRECATED: compiler attribute on {}, use compilers instead".format(ctx.label)) compilers = [ctx.attr.compiler] @@ -124,7 +120,7 @@ def _go_proto_library_impl(ctx): go, compiler = compiler, protos = [d[ProtoInfo] for d in proto_deps], - imports = get_imports(ctx.attr), + imports = get_imports(ctx.attr, go.importpath), importpath = go.importpath, )) library = go.new_library( diff --git a/tests/core/go_proto_library_importpath/BUILD.bazel b/tests/core/go_proto_library_importpath/BUILD.bazel new file mode 100644 index 0000000000..90fdbfc7f6 --- /dev/null +++ b/tests/core/go_proto_library_importpath/BUILD.bazel @@ -0,0 +1,36 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") +load("@rules_proto//proto:defs.bzl", "proto_library") + +# Common rules +proto_library( + name = "foo_proto", + srcs = ["foo.proto"], +) + +go_proto_library( + name = "foo_go_proto", + importpath = "path/to/foo_go", + proto = ":foo_proto", +) + +proto_library( + name = "bar_proto", + srcs = ["bar.proto"], + deps = [":foo_proto"], +) + +go_proto_library( + name = "bar_go_proto", + proto = ":bar_proto", + deps = [":foo_go_proto"], +) + +go_test( + name = "importpath_test", + srcs = ["importpath_test.go"], + deps = [ + ":bar_go_proto", + ":foo_go_proto", + ], +) diff --git a/tests/core/go_proto_library_importpath/bar.proto b/tests/core/go_proto_library_importpath/bar.proto new file mode 100644 index 0000000000..373f237219 --- /dev/null +++ b/tests/core/go_proto_library_importpath/bar.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package tests.core.go_proto_library_importpath.bar; + +import "tests/core/go_proto_library_importpath/foo.proto"; + +message Bar { + foo.Foo value = 1; +} diff --git a/tests/core/go_proto_library_importpath/foo.proto b/tests/core/go_proto_library_importpath/foo.proto new file mode 100644 index 0000000000..bc354cacc4 --- /dev/null +++ b/tests/core/go_proto_library_importpath/foo.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package tests.core.go_proto_library_importpath.foo; + +message Foo { + int64 value = 1; +} diff --git a/tests/core/go_proto_library_importpath/importpath_test.go b/tests/core/go_proto_library_importpath/importpath_test.go new file mode 100644 index 0000000000..3de6569c22 --- /dev/null +++ b/tests/core/go_proto_library_importpath/importpath_test.go @@ -0,0 +1,22 @@ +package importpath_test + +import ( + "fmt" + "testing" + + bar_proto "tests/core/go_proto_library_importpath/bar_go_proto" + foo_proto "path/to/foo_go" +) + +func Test(t *testing.T) { + bar := &bar_proto.Bar{} + bar.Value = &foo_proto.Foo{} + bar.Value.Value = 5 + + var expected int64 = 5 + if bar.Value.Value != expected { + t.Errorf(fmt.Sprintf("Not equal: \n"+ + "expected: %s\n"+ + "actual : %s", expected, bar.Value.Value)) + } +}