Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer go_grpc_library #1711

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions cmd/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,6 @@ proto_library(

go_proto_library(
name = "repo_go_proto",
compilers = ["@io_bazel_rules_go//proto:go_grpc"],
importpath = "example.com/repo",
proto = ":repo_proto",
visibility = ["//visibility:public"],
Expand Down Expand Up @@ -1790,12 +1789,11 @@ message Bar {};
Path: "service/BUILD.bazel",
Content: `
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_proto_library(
go_grpc_library(
name = "service_go_proto",
compilers = ["@io_bazel_rules_go//proto:go_grpc"],
importpath = "example.com/repo/service",
proto = ":service_proto",
visibility = ["//visibility:public"],
Expand Down Expand Up @@ -1867,10 +1865,10 @@ go_library(
Path: "service/BUILD.bazel",
Content: `
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_proto_library(
go_grpc_library(
name = "service_go_proto",
compilers = ["//foo"],
importpath = "example.com/repo/service",
Expand Down
4 changes: 2 additions & 2 deletions language/go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ const (
)

var (
defaultGoProtoCompilers = []string{"@io_bazel_rules_go//proto:go_proto"}
defaultGoGrpcCompilers = []string{"@io_bazel_rules_go//proto:go_grpc"}
defaultGoProtoCompilers = []string{}
defaultGoGrpcCompilers = []string{}
)

func (m testMode) String() string {
Expand Down
6 changes: 3 additions & 3 deletions language/go/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ const (
// mode for libraries that contained .pb.go files and .proto files.
legacyProtoFilegroupName = "go_default_library_protos"

// grpcCompilerLabel is the label for the gRPC compiler plugin, used in the
// "compilers" attribute of go_proto_library rules.
grpcCompilerLabel = "@io_bazel_rules_go//proto:go_grpc"
// oldGrpcCompilerLabel is the label for the old gRPC compiler plugin, used
// in the "compilers" attribute of go_proto_library rules.
oldGrpcCompilerLabel = "@io_bazel_rules_go//proto:go_grpc"

// goProtoSuffix is the suffix applied to the labels of all generated
// go_proto_library targets.
Expand Down
13 changes: 0 additions & 13 deletions language/go/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

func (*goLang) Fix(c *config.Config, f *rule.File) {
migrateLibraryEmbed(c, f)
migrateGrpcCompilers(c, f)
flattenSrcs(c, f)
squashCgoLibrary(c, f)
squashXtest(c, f)
Expand Down Expand Up @@ -171,18 +170,6 @@ func migrateLibraryEmbed(c *config.Config, f *rule.File) {
}
}

// migrateGrpcCompilers converts "go_grpc_library" rules into "go_proto_library"
// rules with a "compilers" attribute.
func migrateGrpcCompilers(c *config.Config, f *rule.File) {
for _, r := range f.Rules {
if r.Kind() != "go_grpc_library" || r.ShouldKeep() || r.Attr("compilers") != nil {
continue
}
r.SetKind("go_proto_library")
r.SetAttr("compilers", []string{grpcCompilerLabel})
}
}

// squashCgoLibrary removes cgo_library rules with the default name and
// merges their attributes with go_library with the default name. If no
// go_library rule exists, a new one will be created.
Expand Down
35 changes: 0 additions & 35 deletions language/go/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,41 +551,6 @@ gomock(
package = "main",
source = "stripe.go",
)
`,
},
// migrateGrpcCompilers tests
{
desc: "go_grpc_library migrated to compilers",
old: `load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "foo_proto",
srcs = ["foo.proto"],
visibility = ["//visibility:public"],
)

go_grpc_library(
name = "foo_go_proto",
importpath = "example.com/repo",
proto = ":foo_proto",
visibility = ["//visibility:public"],
)
`,
want: `load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "foo_proto",
srcs = ["foo.proto"],
visibility = ["//visibility:public"],
)

go_proto_library(
name = "foo_go_proto",
compilers = ["@io_bazel_rules_go//proto:go_grpc"],
importpath = "example.com/repo",
proto = ":foo_proto",
visibility = ["//visibility:public"],
)
`,
},
// flattenSrcs tests
Expand Down
20 changes: 13 additions & 7 deletions language/go/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
protoPackages := make(map[string]proto.Package)
protoFileInfo := make(map[string]proto.FileInfo)
for _, r := range args.OtherGen {
if r.Kind() == "go_proto_library" {
if r.Kind() == "go_proto_library" || r.Kind() == "go_grpc_library" {
if proto := r.AttrString("proto"); proto != "" {
goProtoRules[proto] = struct{}{}
}
Expand Down Expand Up @@ -454,14 +454,20 @@ func (g *generator) generateProto(mode proto.Mode, target protoTarget, importPat
}
}

goProtoLibrary := rule.NewRule("go_proto_library", goProtoName)
var goProtoLibrary *rule.Rule
if !target.hasServices {
goProtoLibrary = rule.NewRule("go_proto_library", goProtoName)
if gc.goProtoCompilersSet {
goProtoLibrary.SetAttr("compilers", gc.goProtoCompilers)
}
} else {
goProtoLibrary = rule.NewRule("go_grpc_library", goProtoName)
if gc.goGrpcCompilersSet {
goProtoLibrary.SetAttr("compilers", gc.goGrpcCompilers)
}
}
goProtoLibrary.SetAttr("proto", ":"+protoName)
g.setImportAttrs(goProtoLibrary, importPath)
if target.hasServices {
goProtoLibrary.SetAttr("compilers", gc.goGrpcCompilers)
} else if gc.goProtoCompilersSet {
goProtoLibrary.SetAttr("compilers", gc.goProtoCompilers)
}
if g.shouldSetVisibility {
goProtoLibrary.SetAttr("visibility", visibility)
}
Expand Down
4 changes: 3 additions & 1 deletion language/go/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ func (gl *goLang) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Remo
switch r.Kind() {
case "go_proto_library":
resolve = resolveProto
case "go_grpc_library":
resolve = resolveProto
default:
resolve = ResolveGo
}
Expand All @@ -95,7 +97,7 @@ func (gl *goLang) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Remo
log.Print(err)
}
if !deps.IsEmpty() {
if r.Kind() == "go_proto_library" {
if isGoProtoLibrary(r.Kind()) {
// protos may import the same library multiple times by different names,
// so we need to de-duplicate them. Protos are not platform-specific,
// so it's safe to just flatten them.
Expand Down
5 changes: 2 additions & 3 deletions language/go/testdata/service/BUILD.want
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "service_proto",
Expand All @@ -12,13 +12,12 @@ proto_library(
visibility = ["//visibility:public"],
)

go_proto_library(
go_grpc_library(
name = "service_go_proto",
_gazelle_imports = [
"google/protobuf/any.proto",
"service/sub/sub.proto",
],
compilers = ["@io_bazel_rules_go//proto:go_grpc"],
importpath = "example.com/repo/service",
proto = ":service_proto",
visibility = ["//visibility:public"],
Expand Down
4 changes: 2 additions & 2 deletions language/go/testdata/service_gogo/BUILD.want
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "service_gogo_proto",
Expand All @@ -12,7 +12,7 @@ proto_library(
visibility = ["//visibility:public"],
)

go_proto_library(
go_grpc_library(
name = "service_gogo_go_proto",
_gazelle_imports = [
"google/protobuf/any.proto",
Expand Down
4 changes: 2 additions & 2 deletions language/go/testdata/service_gogo_subdir_reset/BUILD.want
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "protos_gogo_proto",
Expand All @@ -9,7 +9,7 @@ proto_library(
visibility = ["//visibility:public"],
)

go_proto_library(
go_grpc_library(
name = "protos_gogo_go_proto",
_gazelle_imports = [],
compilers = [
Expand Down
5 changes: 2 additions & 3 deletions language/go/testdata/service_gogo_subdir_reset/sub/BUILD.want
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
name = "protos_gogo_proto",
Expand All @@ -9,10 +9,9 @@ proto_library(
visibility = ["//visibility:public"],
)

go_proto_library(
go_grpc_library(
name = "protos_gogo_go_proto",
_gazelle_imports = [],
compilers = ["@io_bazel_rules_go//proto:go_grpc"],
importpath = "example.com/repo/protos_gogo",
proto = ":protos_gogo_proto",
visibility = ["//visibility:public"],
Expand Down