Fix proto_repository for @googleapis (stackb#324)

pcj · web-flow · commit 3d7eec0c990f · 2023-06-12T12:24:59.000-06:00
* Include vendor tree in proto_repository_tools_srcs
* Rename -override_go_googleapis to -reresolve_known_proto_imports
* Add googleapis to CI build
* Fix TestOverrideRule
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -8,6 +8,7 @@ tasks:
     platform: ubuntu1804
     build_targets:
       - "@protoapis//..."
+      - "@googleapis//..."
     test_targets:
       - //example/...
       - //language/...
@@ -23,6 +24,7 @@ tasks:
     platform: macos
     build_targets:
       - "@protoapis//..."
+      - "@googleapis//..."
     test_targets:
       - //example/...
       - //language/...
diff --git a/README.md b/README.md
@@ -30,25 +30,31 @@ Bazel starlark rules for building protocol buffers +/- gRPC :sparkles:.
 
 # Table of Contents
 
+- [`rules_proto (v2)`](#rules_proto-v2)
+- [Table of Contents](#table-of-contents)
 - [Getting Started](#getting-started)
-  - [Workspace Boilerplate](#workspace-boilerplate)
+  - [`WORKSPACE` Boilerplate](#workspace-boilerplate)
   - [Gazelle Setup](#gazelle-setup)
   - [Gazelle Configuration](#gazelle-configuration)
     - [Build Directives](#build-directives)
-    - [YAML Config File](#yaml-configuration)
+    - [YAML Configuration](#yaml-configuration)
   - [Running Gazelle](#running-gazelle)
-- [Build Rules](#build-rules)
-  - [proto_compile](#proto_compile)
-  - [proto_plugin](#proto_plugin)
-  - [proto_compiled_sources](#proto_compiled_sources)
-  - [Deep dive on the mappings attribute](#the-output_mappings-attribute)
-- [Repository Rules](#repository-rules)
-  - [proto_repository](#proto_repository)
-  - [proto_gazelle](#proto_gazelle)
-- [Plugin Implementations](#plugin-implementations)
-- [Rule Implementations](#rule-implementations)
-- [Writing Custom Plugins & Rules](#writing-custom-plugins-and-rules)
-- [History of this repository](#history)
+  - [Build Rules](#build-rules)
+    - [proto\_compile](#proto_compile)
+    - [proto\_plugin](#proto_plugin)
+    - [proto\_compiled\_sources](#proto_compiled_sources)
+    - [proto\_compile\_assets](#proto_compile_assets)
+    - [The `output_mappings` attribute](#the-output_mappings-attribute)
+  - [Repository Rules](#repository-rules)
+  - [proto\_repository](#proto_repository)
+  - [proto\_gazelle](#proto_gazelle)
+  - [golden\_filegroup](#golden_filegroup)
+  - [Plugin Implementations](#plugin-implementations)
+  - [Rule Implementations](#rule-implementations)
+- [Writing Custom Plugins and Rules](#writing-custom-plugins-and-rules)
+  - [+/- of golang implementations](#--of-golang-implementations)
+  - [+/- of starlark implementations](#--of-starlark-implementations)
+- [History](#history)
 
 # Getting Started
 
@@ -390,7 +396,7 @@ potential conflicts with other possible gazelle extensions, using the name
 The core of `stackb/rules_proto` contains two build rules:
 
 | Rule            | Description                                             |
-|-----------------|---------------------------------------------------------|
+| --------------- | ------------------------------------------------------- |
 | `proto_compile` | Executes the `protoc` tool.                             |
 | `proto_plugin`  | Provides static `protoc` plugin-specific configuration. |
 
@@ -590,7 +596,7 @@ proto_repository(
     ],
     build_file_generation = "clean",
     build_file_proto_mode = "file",
-    override_go_googleapis = True,
+    reresolve_known_proto_imports = True,
     proto_language_config_file = "//example:config.yaml",
     strip_prefix = "googleapis-02710fa0ea5312d79d7fb986c9c9823fb41049a9",
     type = "zip",
@@ -615,7 +621,7 @@ Takeaways:
   external workspace.
 - `proto_language_config_file` is an optional label pointing to a valid
   `config.yaml` file to configure this extension.
-- `override_go_googleapis` is a boolean attribute that has special meaning for
+- `reresolve_known_proto_imports` is a boolean attribute that has special meaning for
   the googleapis repository. Due to the fact that the builtin gazelle "proto"
   extension has
   [hardcoded logic](https://github.com/bazelbuild/bazel-gazelle/blob/master/language/proto/known_proto_imports.go)
@@ -762,7 +768,7 @@ The plugin name is an opaque string, but by convention they are maven-esqe
 artifact identifiers that follow a GitHub org/repo/plugin_name convention.
 
 | Plugin                                                                                                                 |
-|------------------------------------------------------------------------------------------------------------------------|
+| ---------------------------------------------------------------------------------------------------------------------- |
 | [builtin:cpp](pkg/plugin/builtin/cpp_plugin.go)                                                                        |
 | [builtin:csharp](pkg/plugin/builtin/csharp_plugin.go)                                                                  |
 | [builtin:java](pkg/plugin/builtin/java_plugin.go)                                                                      |
@@ -798,7 +804,7 @@ The rule name is an opaque string, but by convention they are maven-esqe
 artifact identifiers that follow a GitHub org/repo/rule_name convention.
 
 | Plugin                                                                                            |
-|---------------------------------------------------------------------------------------------------|
+| ------------------------------------------------------------------------------------------------- |
 | [stackb:rules_proto:grpc_cc_library](pkg/rule/rules_cc/grpc_cc_library.go)                        |
 | [stackb:rules_proto:grpc_closure_js_library](pkg/rule/rules_closure/grpc_closure_js_library.go)   |
 | [stackb:rules_proto:grpc_java_library](pkg/rule/rules_java/grpc_java_library.go)                  |
diff --git a/WORKSPACE b/WORKSPACE
@@ -1,7 +1,5 @@
 workspace(name = "build_stack_rules_proto")
 
-load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
-
 # gazelle:repo bazel_gazelle
 
 # ----------------------------------------------------
@@ -99,18 +97,6 @@ go_deps()
 # Core gRPC
 # ----------------------------------------------------
 
-http_archive(
-    name = "com_google_absl",
-    generator_function = "grpc_deps",
-    generator_name = "com_google_absl",
-    sha256 = "9a2b5752d7bfade0bdeee2701de17c9480620f8b237e1964c1b9967c75374906",
-    strip_prefix = "abseil-cpp-20230125.2",
-    urls = [
-        "https://storage.googleapis.com/grpc-bazel-mirror/github.com/abseil/abseil-cpp/archive/20230125.2.tar.gz",
-        "https://github.com/abseil/abseil-cpp/archive/20230125.2.tar.gz",
-    ],
-)
-
 load(
     "@com_github_grpc_grpc//bazel:grpc_deps.bzl",
     "grpc_deps",
diff --git a/docs/proto_repository.md b/docs/proto_repository.md
@@ -89,7 +89,7 @@ proto_repository(
     build_file_generation = "on",
     build_file_proto_mode = "file",
     cfgs = ["//:config.yaml"],
-    override_go_googleapis = True,
+    reresolve_known_proto_imports = True,
     sha256 = "b9dbc65ebc738a486265ef7b708e9449bf361541890091983e946557ee0a4bfc",
     strip_prefix = "googleapis-66759bdf6a5ebb898c2a51c8649aefd1ee0b7ffe",
     type = "zip",
diff --git a/example/golden/testdata/proto_repository/README.md b/example/golden/testdata/proto_repository/README.md
@@ -32,5 +32,5 @@ A few notes about the test:
   to load newer versions of @org_golang_google_grpc before rules_go or gazelle
   try and do that.
 - proto file mode is used.
-- override_go_googleapis = True is needed to rewrite labels that would normally
-  be in the "go_googleapis" external workspace.
+- reresolve_known_proto_imports = True is needed to rewrite labels that would normally
+  be in the "go_googleapis" and "com_google_protobuf" external workspace.
diff --git a/example/golden/testdata/proto_repository/WORKSPACE b/example/golden/testdata/proto_repository/WORKSPACE
@@ -18,7 +18,7 @@ proto_repository(
     build_file_generation = "on",
     build_file_proto_mode = "file",
     cfgs = ["//:config.yaml"],
-    override_go_googleapis = True,
+    reresolve_known_proto_imports = True,
     sha256 = "b9dbc65ebc738a486265ef7b708e9449bf361541890091983e946557ee0a4bfc",
     strip_prefix = "googleapis-66759bdf6a5ebb898c2a51c8649aefd1ee0b7ffe",
     type = "zip",
diff --git a/pkg/language/protobuf/config.go b/pkg/language/protobuf/config.go
@@ -29,9 +29,9 @@ func (pl *protobufLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Co
 	fs.StringVar(&pl.repoName,
 		"proto_repo_name", "",
 		"external name of this repository")
-	fs.BoolVar(&pl.overrideGoGooleapis,
-		"override_go_googleapis", false,
-		"if true, remove hardcoded proto_library deps on go_googleapis")
+	fs.BoolVar(&pl.reresolveKnownProtoImports,
+		"reresolve_known_proto_imports", false,
+		"if true, re-resolve hardcoded proto_library deps on go_googleapis and com_google_protobuf from language/proto from the index")
 	fs.Var(&pl.starlarkRules,
 		"proto_rule",
 		"register custom starlark rule of the form `<file_name>%<rule_name>`")
diff --git a/pkg/language/protobuf/generate.go b/pkg/language/protobuf/generate.go
@@ -91,8 +91,9 @@ func (pl *protobufLang) GenerateRules(args language.GenerateArgs) language.Gener
 
 	rules := pkg.Rules()
 
-	// special case if we want to override go_googleapis deps.
-	if pl.overrideGoGooleapis && len(protoLibraries) > 0 {
+	// special case if we want to override com_google_protobuf or go_googleapis
+	// deps.
+	if pl.reresolveKnownProtoImports && len(protoLibraries) > 0 {
 		rules = append(rules, makeProtoOverrideRule(protoLibraries))
 	}
 
diff --git a/pkg/language/protobuf/lang.go b/pkg/language/protobuf/lang.go
@@ -32,8 +32,8 @@ type protobufLang struct {
 	// importsInFiles is a comma-separated list of files that contains proto
 	// index csv content.
 	importsInFiles string
-	// overrideGoGooleapis performs special processing for go_googleapis deps
-	overrideGoGooleapis bool
+	// reresolveKnownProtoImports performs an additional resolve step for go_googleapis deps
+	reresolveKnownProtoImports bool
 	// the resolver instance used for cross-resolution
 	resolver protoc.ImportResolver
 	// starlarkRules stores custom starlark proto rule names in the form filename%rulename
diff --git a/pkg/language/protobuf/override.go b/pkg/language/protobuf/override.go
@@ -33,7 +33,8 @@ func makeProtoOverrideRule(libs []protoc.ProtoLibrary) *rule.Rule {
 	return overrideRule
 }
 
-func resolveOverrideRule(rel string, overrideRule *rule.Rule, resolver protoc.ImportResolver) {
+func resolveOverrideRule(c *config.Config, rel string, overrideRule *rule.Rule, resolver protoc.ImportResolver) {
+
 	libs := overrideRule.PrivateAttr(protoLibrariesRuleKey).([]protoc.ProtoLibrary)
 	if len(libs) == 0 {
 		return
@@ -42,31 +43,18 @@ func resolveOverrideRule(rel string, overrideRule *rule.Rule, resolver protoc.Im
 	for _, lib := range libs {
 		r := lib.Rule()
 
-		// filter out go_googleapis dependencies and re-resolve them anew.
-		keep := make([]label.Label, 0)
-
-		for _, dep := range r.AttrStrings("deps") {
-			lbl, _ := label.Parse(dep)
-			// log.Printf("override resolve //%s:%s dep %v", rel, r.Name(), lbl)
-			if lbl.Repo == "go_googleapis" {
-				continue
-			}
-			if lbl.Relative {
-				// relative labels will be repopulated via resolution (below)
-				continue
-			}
-			keep = append(keep, lbl)
-		}
+		// re-resolve dependencies.
+		deps := make([]label.Label, 0)
 
 		imports := r.PrivateAttr(config.GazelleImportsKey)
 		if imps, ok := imports.([]string); ok {
 			for _, imp := range imps {
 				result := resolver.Resolve("proto", "proto", imp)
 				if len(result) > 0 {
 					first := result[0]
-					keep = append(keep, first.Label)
+					deps = append(deps, first.Label)
 					if debugOverrides {
-						log.Println("go_googleapis resolve imports HIT", first.Label)
+						log.Println("go_googleapis resolve imports HIT", imp, first.Label)
 					}
 				} else {
 					if debugOverrides {
@@ -76,9 +64,9 @@ func resolveOverrideRule(rel string, overrideRule *rule.Rule, resolver protoc.Im
 			}
 		}
 
-		if len(keep) > 0 {
-			ss := make([]string, len(keep))
-			for i, lbl := range keep {
+		if len(deps) > 0 {
+			ss := make([]string, len(deps))
+			for i, lbl := range deps {
 				ss[i] = lbl.Rel("", rel).String()
 			}
 			r.SetAttr("deps", protoc.DeduplicateAndSort(ss))
diff --git a/pkg/language/protobuf/override_test.go b/pkg/language/protobuf/override_test.go
@@ -26,22 +26,23 @@ func TestOverrideRule(t *testing.T) {
 		deps, imps, want []string
 		known            map[string]label.Label
 	}{
-		// "empty": {
-		// 	rel:  "",
-		// 	want: []string{},
-		// },
-		// "no go_googleapis deps": {
-		// 	rel:  "",
-		// 	deps: []string{"@com_google_protobuf//:any_proto"},
-		// 	want: []string{"@com_google_protobuf//:any_proto"},
-		// },
 		"has go_googleapis dep": {
-			rel:  "",
-			deps: []string{"@com_google_protobuf//:any_proto", "@go_googleapis//google/api:api_proto"},
-			want: []string{"//google/api:http_proto", "@com_google_protobuf//:any_proto"},
-			imps: []string{"google/api/http.proto"},
+			rel: "",
+			deps: []string{
+				"@com_google_protobuf//:any_proto",
+				"@go_googleapis//google/api:api_proto",
+			},
+			want: []string{
+				"//google/api:http_proto",
+				"@protoapis//google/protobuf:any_proto",
+			},
+			imps: []string{
+				"google/protobuf/any.proto",
+				"google/api/http.proto",
+			},
 			known: map[string]label.Label{
-				"google/api/http.proto": label.New("", "google/api", "http_proto"),
+				"google/protobuf/any.proto": label.New("protoapis", "google/protobuf", "any_proto"),
+				"google/api/http.proto":     label.New("", "google/api", "http_proto"),
 			},
 		},
 	} {
@@ -53,10 +54,11 @@ func TestOverrideRule(t *testing.T) {
 			for k, v := range tc.known {
 				resolver.Provide("proto", "proto", k, v)
 			}
+			c := config.New()
 			r := makeProtoLibraryRule("test_proto", tc.deps, tc.imps)
 			lib := makeOtherProtoLibrary(r)
 			overrideRule := makeProtoOverrideRule([]protoc.ProtoLibrary{lib})
-			resolveOverrideRule(tc.rel, overrideRule, resolver)
+			resolveOverrideRule(c, tc.rel, overrideRule, resolver)
 
 			got := r.AttrStrings("deps")
 			if diff := cmp.Diff(tc.want, got); diff != "" {
diff --git a/pkg/language/protobuf/resolve.go b/pkg/language/protobuf/resolve.go
@@ -56,7 +56,7 @@ func (pl *protobufLang) Resolve(
 	from label.Label,
 ) {
 	if r.Kind() == overrideKindName {
-		resolveOverrideRule(from.Pkg, r, protoc.GlobalResolver())
+		resolveOverrideRule(c, from.Pkg, r, protoc.GlobalResolver())
 		return
 	}
 
diff --git a/pkg/protoc/resolver.go b/pkg/protoc/resolver.go
@@ -196,8 +196,12 @@ func (r *resolver) Resolve(lang, impLang, imp string) []resolve.FindResult {
 	}
 	if got, ok := known[imp]; ok {
 		res := make([]resolve.FindResult, len(got))
-		for i, l := range got {
-			res[i] = resolve.FindResult{Label: l}
+		// use last-wins semantics by providing FindResults in reverse to how
+		// they were provided.
+		var index int
+		for i := len(got) - 1; i >= 0; i-- {
+			res[index] = resolve.FindResult{Label: got[i]}
+			index++
 		}
 		return res
 	}
diff --git a/proto_repositories.bzl b/proto_repositories.bzl
@@ -53,8 +53,29 @@ def proto_repositories():
             "google/protobuf/unittest_well_known_types.proto",
             "google/protobuf/compiler/cpp/test_bad_identifiers.proto",
         ],
+        reresolve_known_proto_imports = True,
         strip_prefix = "protobuf-a74f54b724bdc2fe0bfc271f4dc0ceb159805625/src",
         type = "zip",
         # https://github.com/protocolbuffers/protobuf/releases/tag/v23.2
         urls = ["https://codeload.github.com/protocolbuffers/protobuf/zip/a74f54b724bdc2fe0bfc271f4dc0ceb159805625"],
     )
+
+    proto_repository(
+        name = "googleapis",
+        build_directives = [
+            "gazelle:exclude google/ads/googleads/v12/services",
+            "gazelle:exclude google/ads/googleads/v13/services",
+            "gazelle:exclude google/ads/googleads/v14/services",
+            "gazelle:proto_language go enable false",
+            "gazelle:proto_language cpp enable false",
+            "gazelle:proto_language descriptor enable true",
+        ],
+        imports = ["@protoapis//:imports.csv"],
+        build_file_expunge = True,
+        build_file_proto_mode = "file",
+        reresolve_known_proto_imports = True,
+        cfgs = ["//example:config.yaml"],
+        sha256 = "e5b59ae2c0c812e3867158eca8e484fddb96dff03b8e2073bf44242b708fa919",
+        strip_prefix = "googleapis-e115ab1839cb6e1bd953e40337b7e84001291766",
+        urls = ["https://github.com/googleapis/googleapis/archive/e115ab1839cb6e1bd953e40337b7e84001291766.tar.gz"],
+    )
diff --git a/rules/private/list_repository_tools_srcs.go b/rules/private/list_repository_tools_srcs.go
@@ -1,3 +1,4 @@
+//go:build ignore
 // +build ignore
 
 // Copyright 2014 The Bazel Authors. All rights reserved.
@@ -61,7 +62,7 @@ func main() {
 			return err
 		}
 		base := filepath.Base(path)
-		if base == "vendor" || base == "third_party" || base == "testdata" {
+		if base == "third_party" || base == "testdata" {
 			return filepath.SkipDir
 		}
 		if !info.IsDir() &&
diff --git a/rules/private/proto_repository_tools_srcs.bzl b/rules/private/proto_repository_tools_srcs.bzl
diff --git a/rules/proto/proto_repository.bzl b/rules/proto/proto_repository.bzl

Original file line number	Diff line number	Diff line change
`@@ -91,8 +91,9 @@ func (pl *protobufLang) GenerateRules(args language.GenerateArgs) language.Gener`
`91`	`91`
`92`	`92`	`rules := pkg.Rules()`
`93`	`93`
`94`		`- // special case if we want to override go_googleapis deps.`
`95`		`- if pl.overrideGoGooleapis && len(protoLibraries) > 0 {`
	`94`	`+ // special case if we want to override com_google_protobuf or go_googleapis`
	`95`	`+ // deps.`
	`96`	`+ if pl.reresolveKnownProtoImports && len(protoLibraries) > 0 {`
`96`	`97`	`rules = append(rules, makeProtoOverrideRule(protoLibraries))`
`97`	`98`	`}`
`98`	`99`