From 36b1741aa01cf8645a5023ef95d1729d75fafcc1 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 25 Feb 2024 17:37:06 +0100 Subject: [PATCH] proto: Add Bzlmod support for well-known types (#1741) * proto: Add Bzlmod support for well-known types * Update .bazelrc * Update .bazelrc * Update .bazelrc --- .bazelci/presubmit.yml | 11 ++++++----- .bcr/presubmit.yml | 10 ++++++---- cmd/gazelle/BUILD.bazel | 1 - cmd/gazelle/fix-update.go | 15 ++------------- cmd/gazelle/update-repos.go | 18 ++++++------------ config/BUILD.bazel | 1 + config/config.go | 10 ++++++++++ language/proto/constants.go | 6 ++++++ language/proto/proto.csv | 24 ++++++++++++------------ language/proto/resolve.go | 11 +++++++++++ tests/bcr/.bazelrc | 3 +++ tests/bcr/MODULE.bazel | 3 +++ tests/bcr/go.mod | 2 +- tests/bcr/go.sum | 2 ++ tests/bcr/proto/BUILD.bazel | 31 +++++++++++++++++++++++++++++++ tests/bcr/proto/foo.proto | 12 ++++++++++++ tests/bcr/proto/foo_test.go | 23 +++++++++++++++++++++++ 17 files changed, 135 insertions(+), 48 deletions(-) create mode 100644 tests/bcr/proto/BUILD.bazel create mode 100644 tests/bcr/proto/foo.proto create mode 100644 tests/bcr/proto/foo_test.go diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index b8ccbfd53..97f66c112 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -38,17 +38,18 @@ tasks: platform: ${{ platform }} working_directory: tests/bcr shell_commands: - # Regenerate the BUILD file for the test module using Gazelle. + # Regenerate the BUILD files for the test module using Gazelle. # Also verify -repo_config are generated correctly in gazelle.bash - - rm pkg/BUILD.bazel - - bazel run //:gazelle -- update pkg - - bazel run //:gazelle -- pkg + - rm pkg/BUILD.bazel proto/BUILD.bazel + - bazel run //:gazelle -- update pkg proto + - bazel run //:gazelle -- pkg proto build_targets: - "//..." - "//:gazelle" test_targets: - # Specify this target explicitly to verify that Gazelle generated it correctly. + # Specify these targets explicitly to verify that Gazelle generates them correctly. - "//pkg:pkg_test" + - "//proto:proto_test" - "//..." - "@test_dep//..." bcr_test_windows: diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index d95652b26..89eaacfe3 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -11,14 +11,16 @@ bcr_test_module: name: Run test module platform: ${{ platform }} shell_commands: - # Regenerate the BUILD file for the test module using Gazelle. - - rm pkg/BUILD.bazel - - bazel run //:gazelle + # Regenerate the BUILD files for the test module using Gazelle. + - rm pkg/BUILD.bazel proto/BUILD.bazel + - bazel run //:gazelle -- update pkg proto + - bazel run //:gazelle -- pkg proto build_targets: - //... - //:gazelle test_targets: - # Specify this target explicitly to verify that Gazelle generated it correctly. + # Specify these targets explicitly to verify that Gazelle generates them correctly. - "//pkg:pkg_test" + - "//proto:proto_test" - "//..." - "@test_dep//..." diff --git a/cmd/gazelle/BUILD.bazel b/cmd/gazelle/BUILD.bazel index e14a1af47..c1d5ef861 100644 --- a/cmd/gazelle/BUILD.bazel +++ b/cmd/gazelle/BUILD.bazel @@ -26,7 +26,6 @@ go_library( deps = [ "//config", "//flag", - "//internal/module", "//internal/wspace", "//label", "//language", diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go index fe8fee57f..db35b5c50 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -30,7 +30,6 @@ import ( "github.com/bazelbuild/bazel-gazelle/config" gzflag "github.com/bazelbuild/bazel-gazelle/flag" - "github.com/bazelbuild/bazel-gazelle/internal/module" "github.com/bazelbuild/bazel-gazelle/internal/wspace" "github.com/bazelbuild/bazel-gazelle/label" "github.com/bazelbuild/bazel-gazelle/language" @@ -168,15 +167,10 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro }) } - moduleToApparentName, err := module.ExtractModuleToApparentNameMapping(c.RepoRoot) - if err != nil { - return err - } - for _, r := range c.Repos { if r.Kind() == "go_repository" { var name string - if apparentName := moduleToApparentName(r.AttrString("module_name")); apparentName != "" { + if apparentName := c.ModuleToApparentName(r.AttrString("module_name")); apparentName != "" { name = apparentName } else { name = r.Name() @@ -276,11 +270,6 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { return err } - moduleToApparentName, err := module.ExtractModuleToApparentNameMapping(c.RepoRoot) - if err != nil { - return err - } - mrslv := newMetaResolver() kinds := make(map[string]rule.KindInfo) loads := genericLoads @@ -291,7 +280,7 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { kinds[kind] = info } if moduleAwareLang, ok := lang.(language.ModuleAwareLanguage); ok { - loads = append(loads, moduleAwareLang.ApparentLoads(moduleToApparentName)...) + loads = append(loads, moduleAwareLang.ApparentLoads(c.ModuleToApparentName)...) } else { loads = append(loads, lang.Loads()...) } diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go index bac5f3f22..9d33977a1 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -26,7 +26,6 @@ import ( "strings" "github.com/bazelbuild/bazel-gazelle/config" - "github.com/bazelbuild/bazel-gazelle/internal/module" "github.com/bazelbuild/bazel-gazelle/internal/wspace" "github.com/bazelbuild/bazel-gazelle/label" "github.com/bazelbuild/bazel-gazelle/language" @@ -108,11 +107,11 @@ func (*updateReposConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) err workspacePath := wspace.FindWORKSPACEFile(c.RepoRoot) uc.workspace, err = rule.LoadWorkspaceFile(workspacePath, "") if err != nil { - if c.Bzlmod { - return nil - } else { - return fmt.Errorf("loading WORKSPACE file: %v", err) - } + if c.Bzlmod { + return nil + } else { + return fmt.Errorf("loading WORKSPACE file: %v", err) + } } c.Repos, uc.repoFileMap, err = repo.ListRepositories(uc.workspace) if err != nil { @@ -141,16 +140,11 @@ func updateRepos(wd string, args []string) (err error) { } uc := getUpdateReposConfig(c) - moduleToApparentName, err := module.ExtractModuleToApparentNameMapping(c.RepoRoot) - if err != nil { - return err - } - kinds := make(map[string]rule.KindInfo) loads := []rule.LoadInfo{} for _, lang := range languages { if moduleAwareLang, ok := lang.(language.ModuleAwareLanguage); ok { - loads = append(loads, moduleAwareLang.ApparentLoads(moduleToApparentName)...) + loads = append(loads, moduleAwareLang.ApparentLoads(c.ModuleToApparentName)...) } else { loads = append(loads, lang.Loads()...) } diff --git a/config/BUILD.bazel b/config/BUILD.bazel index cc301bf3f..2d5718174 100644 --- a/config/BUILD.bazel +++ b/config/BUILD.bazel @@ -9,6 +9,7 @@ go_library( importpath = "github.com/bazelbuild/bazel-gazelle/config", visibility = ["//visibility:public"], deps = [ + "//internal/module", "//internal/wspace", "//rule", ], diff --git a/config/config.go b/config/config.go index 1748ab8fa..57c3d4481 100644 --- a/config/config.go +++ b/config/config.go @@ -34,6 +34,7 @@ import ( "path/filepath" "strings" + "github.com/bazelbuild/bazel-gazelle/internal/module" "github.com/bazelbuild/bazel-gazelle/internal/wspace" "github.com/bazelbuild/bazel-gazelle/rule" ) @@ -107,6 +108,11 @@ type Config struct { // Whether Gazelle is loaded as a Bzlmod 'bazel_dep'. Bzlmod bool + + // ModuleToApparentName is a function that maps the name of a Bazel module + // to the apparent name (repo_name) specified in the MODULE.bazel file. It + // returns the empty string if the module is not found. + ModuleToApparentName func(string) string } // MappedKind describes a replacement to use for a built-in kind. @@ -250,6 +256,10 @@ func (cc *CommonConfigurer) CheckFlags(fs *flag.FlagSet, c *Config) error { c.Langs = strings.Split(cc.langCsv, ",") } c.Bzlmod = cc.bzlmod + c.ModuleToApparentName, err = module.ExtractModuleToApparentNameMapping(c.RepoRoot) + if err != nil { + return fmt.Errorf("failed to parse MODULE.bazel: %v", err) + } return nil } diff --git a/language/proto/constants.go b/language/proto/constants.go index be6bb4c8d..525f5f8a6 100644 --- a/language/proto/constants.go +++ b/language/proto/constants.go @@ -25,3 +25,9 @@ const ( // pre-generated code for the Well Known Types. wellKnownTypesGoPrefix = "github.com/golang/protobuf" ) + +// bazelModuleRepos maps the names of Bazel modules to their "well-known" +// WORKSPACE repository name +var bazelModuleRepos = map[string]string{ + "protobuf": "com_google_protobuf", +} diff --git a/language/proto/proto.csv b/language/proto/proto.csv index 04ca3693a..0279d8c8c 100644 --- a/language/proto/proto.csv +++ b/language/proto/proto.csv @@ -1,15 +1,15 @@ # This file lists special protos that Gazelle knows how to import. This is used to generate # code for proto and Go resolvers. # proto name,proto label,go import path,go proto label -google/protobuf/any.proto,@com_google_protobuf//:any_proto,github.com/golang/protobuf/ptypes/any,@com_github_golang_protobuf//ptypes/any -google/protobuf/api.proto,@com_google_protobuf//:api_proto,google.golang.org/genproto/protobuf/api,@org_golang_google_genproto//protobuf/api -google/protobuf/compiler/plugin.proto,@com_google_protobuf//:compiler_plugin_proto,github.com/golang/protobuf/protoc-gen-go/plugin,@com_github_golang_protobuf//protoc-gen-go/plugin -google/protobuf/descriptor.proto,@com_google_protobuf//:descriptor_proto,github.com/golang/protobuf/protoc-gen-go/descriptor,@com_github_golang_protobuf//protoc-gen-go/descriptor -google/protobuf/duration.proto,@com_google_protobuf//:duration_proto,github.com/golang/protobuf/ptypes/duration,@com_github_golang_protobuf//ptypes/duration -google/protobuf/empty.proto,@com_google_protobuf//:empty_proto,github.com/golang/protobuf/ptypes/empty,@com_github_golang_protobuf//ptypes/empty -google/protobuf/field_mask.proto,@com_google_protobuf//:field_mask_proto,google.golang.org/genproto/protobuf/field_mask,@org_golang_google_genproto//protobuf/field_mask -google/protobuf/source_context.proto,@com_google_protobuf//:source_context_proto,google.golang.org/genproto/protobuf/source_context,@org_golang_google_genproto//protobuf/source_context -google/protobuf/struct.proto,@com_google_protobuf//:struct_proto,github.com/golang/protobuf/ptypes/struct,@com_github_golang_protobuf//ptypes/struct -google/protobuf/timestamp.proto,@com_google_protobuf//:timestamp_proto,github.com/golang/protobuf/ptypes/timestamp,@com_github_golang_protobuf//ptypes/timestamp -google/protobuf/type.proto,@com_google_protobuf//:type_proto,google.golang.org/genproto/protobuf/ptype,@org_golang_google_genproto//protobuf/ptype -google/protobuf/wrappers.proto,@com_google_protobuf//:wrappers_proto,github.com/golang/protobuf/ptypes/wrappers,@com_github_golang_protobuf//ptypes/wrappers +google/protobuf/any.proto,@protobuf//:any_proto,github.com/golang/protobuf/ptypes/any,@com_github_golang_protobuf//ptypes/any +google/protobuf/api.proto,@protobuf//:api_proto,google.golang.org/genproto/protobuf/api,@org_golang_google_genproto//protobuf/api +google/protobuf/compiler/plugin.proto,@protobuf//:compiler_plugin_proto,github.com/golang/protobuf/protoc-gen-go/plugin,@com_github_golang_protobuf//protoc-gen-go/plugin +google/protobuf/descriptor.proto,@protobuf//:descriptor_proto,github.com/golang/protobuf/protoc-gen-go/descriptor,@com_github_golang_protobuf//protoc-gen-go/descriptor +google/protobuf/duration.proto,@protobuf//:duration_proto,github.com/golang/protobuf/ptypes/duration,@com_github_golang_protobuf//ptypes/duration +google/protobuf/empty.proto,@protobuf//:empty_proto,github.com/golang/protobuf/ptypes/empty,@com_github_golang_protobuf//ptypes/empty +google/protobuf/field_mask.proto,@protobuf//:field_mask_proto,google.golang.org/genproto/protobuf/field_mask,@org_golang_google_genproto//protobuf/field_mask +google/protobuf/source_context.proto,@protobuf//:source_context_proto,google.golang.org/genproto/protobuf/source_context,@org_golang_google_genproto//protobuf/source_context +google/protobuf/struct.proto,@protobuf//:struct_proto,github.com/golang/protobuf/ptypes/struct,@com_github_golang_protobuf//ptypes/struct +google/protobuf/timestamp.proto,@protobuf//:timestamp_proto,github.com/golang/protobuf/ptypes/timestamp,@com_github_golang_protobuf//ptypes/timestamp +google/protobuf/type.proto,@protobuf//:type_proto,google.golang.org/genproto/protobuf/ptype,@org_golang_google_genproto//protobuf/ptype +google/protobuf/wrappers.proto,@protobuf//:wrappers_proto,github.com/golang/protobuf/ptypes/wrappers,@com_github_golang_protobuf//ptypes/wrappers diff --git a/language/proto/resolve.go b/language/proto/resolve.go index 6458a5638..bf27dc150 100644 --- a/language/proto/resolve.go +++ b/language/proto/resolve.go @@ -122,6 +122,17 @@ func resolveProto(c *config.Config, ix *resolve.RuleIndex, r *rule.Rule, imp str if l.Equal(from) { return label.NoLabel, errSkipImport } else { + if workspaceName, isModule := bazelModuleRepos[l.Repo]; isModule { + apparentRepoName := c.ModuleToApparentName(l.Repo) + if apparentRepoName == "" { + // The user doesn't have a bazel_dep for the module containing this known + // target. + // TODO: Fail here instead when not using WORKSPACE anymore. + l.Repo = workspaceName + } else { + l.Repo = apparentRepoName + } + } return l, nil } } diff --git a/tests/bcr/.bazelrc b/tests/bcr/.bazelrc index 6bc4e8eff..7ccbf79fb 100644 --- a/tests/bcr/.bazelrc +++ b/tests/bcr/.bazelrc @@ -1,3 +1,6 @@ common --enable_bzlmod common --experimental_isolated_extension_usages common --lockfile_mode=update +common --enable_platform_specific_config +# Required by abseil-cpp on macOS with Bazel 6. +common:macos --host_cxxopt=-std=c++14 diff --git a/tests/bcr/MODULE.bazel b/tests/bcr/MODULE.bazel index 046f8d6c7..cc0026e03 100644 --- a/tests/bcr/MODULE.bazel +++ b/tests/bcr/MODULE.bazel @@ -14,7 +14,9 @@ local_path_override( path = "test_dep", ) +bazel_dep(name = "protobuf", version = "23.1", repo_name = "my_protobuf") bazel_dep(name = "rules_go", version = "0.42.0", repo_name = "my_rules_go") +bazel_dep(name = "rules_proto", version = "6.0.0-rc2", repo_name = "my_rules_proto") # This bazel_dep provides the Go dependency github.com/cloudflare/circl, which requires custom # patches beyond what Gazelle can generate. @@ -123,6 +125,7 @@ use_repo( "com_github_fmeum_dep_on_gazelle", "com_github_google_safetext", "com_github_stretchr_testify", + "org_golang_google_protobuf", "org_golang_x_sys", # It is not necessary to list transitive dependencies here, only direct ones. # "in_gopkg_yaml_v3", diff --git a/tests/bcr/go.mod b/tests/bcr/go.mod index 405742d91..d4f1957f5 100644 --- a/tests/bcr/go.mod +++ b/tests/bcr/go.mod @@ -15,11 +15,11 @@ require ( github.com/fmeum/dep_on_gazelle v1.0.0 github.com/google/safetext v0.0.0-20220905092116-b49f7bc46da2 golang.org/x/sys v0.15.0 + google.golang.org/protobuf v1.32.0 ) require ( github.com/bazelbuild/bazel-gazelle v0.30.0 // indirect github.com/kr/text v0.2.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - google.golang.org/protobuf v1.30.0 // indirect ) diff --git a/tests/bcr/go.sum b/tests/bcr/go.sum index a82fbd9fd..ee03e8091 100644 --- a/tests/bcr/go.sum +++ b/tests/bcr/go.sum @@ -45,6 +45,8 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= +google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I= +google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/tests/bcr/proto/BUILD.bazel b/tests/bcr/proto/BUILD.bazel new file mode 100644 index 000000000..d1621cc20 --- /dev/null +++ b/tests/bcr/proto/BUILD.bazel @@ -0,0 +1,31 @@ +load("@my_rules_proto//proto:defs.bzl", "proto_library") +load("@my_rules_go//go:def.bzl", "go_test") +load("@my_rules_go//proto:def.bzl", "go_proto_library") + +proto_library( + name = "foo_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], + deps = [ + "@my_protobuf//:timestamp_proto", + "@my_protobuf//:type_proto", + ], +) + +go_proto_library( + name = "foo_go_proto", + importpath = "github.com/bazelbuild/bazel-gazelle/tests/bcr/proto/foo", + proto = ":foo_proto", + visibility = ["//visibility:public"], +) + +go_test( + name = "proto_test", + srcs = ["foo_test.go"], + deps = [ + ":foo_go_proto", + "@org_golang_google_protobuf//types/known/sourcecontextpb", + "@org_golang_google_protobuf//types/known/timestamppb", + "@org_golang_google_protobuf//types/known/typepb", + ], +) diff --git a/tests/bcr/proto/foo.proto b/tests/bcr/proto/foo.proto new file mode 100644 index 000000000..b4674e4f7 --- /dev/null +++ b/tests/bcr/proto/foo.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +option go_package = "github.com/bazelbuild/bazel-gazelle/tests/bcr/proto/foo"; + +import "google/protobuf/timestamp.proto"; +import "google/protobuf/type.proto"; + +message Foo { + string name = 1; + google.protobuf.Timestamp last_updated = 2; + google.protobuf.Type type = 3; +} diff --git a/tests/bcr/proto/foo_test.go b/tests/bcr/proto/foo_test.go new file mode 100644 index 000000000..ee5bd456b --- /dev/null +++ b/tests/bcr/proto/foo_test.go @@ -0,0 +1,23 @@ +package proto + +import ( + "testing" + + "github.com/bazelbuild/bazel-gazelle/tests/bcr/proto/foo" + "google.golang.org/protobuf/types/known/sourcecontextpb" + "google.golang.org/protobuf/types/known/timestamppb" + "google.golang.org/protobuf/types/known/typepb" +) + +func TestWellKnownTypes(t *testing.T) { + var foo foo.Foo + foo.Name = "foo" + foo.Type = &typepb.Type{ + Name: "my_type", + SourceContext: &sourcecontextpb.SourceContext{}, + } + foo.LastUpdated = ×tamppb.Timestamp{ + Seconds: 12345, + Nanos: 67890, + } +}