Skip to content

chore(internal/librariangen): remove the legacy protoc/gRPC plugins #13372

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(internal/librariangen): remove the legacy protoc/gRPC plugins #13372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 0 additions & 13 deletions internal/librariangen/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ FROM marketplace.gcr.io/google/debian12:latest
# Set environment variables for tool versions for easy updates.
ENV GO_VERSION=1.24.0
ENV PROTOC_VERSION=25.7
ENV GO_PROTOC_PLUGIN_V1_VERSION=1.5.4
ENV GO_PROTOC_PLUGIN_V2_VERSION=1.35.2
ENV GO_GRPC_PLUGIN_VERSION=1.3.0
ENV GAPIC_GENERATOR_VERSION=0.55.1
Expand Down Expand Up @@ -97,18 +96,6 @@ RUN wget https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC
rm protoc.zip && \
protoc --version

# Build the "old" Go protoc plugin manually; using go install just redirects to the new one.
# Note that the "old" plugin uses the internal code in the "new" plugin, so we need to use
# "go mod edit" to end up with a consistent version.
# The plugin is built as protoc-gen-go_v1, so that we can use "go_v1" as the name in protoc options.
# This is ghastly, and should all be removed as soon as possible by migrating all packages to go_grpc.
RUN (export GOTOOLCHAIN='auto' && \
git clone --depth 1 --branch=v${GO_PROTOC_PLUGIN_V1_VERSION} https://github.com/golang/protobuf protoc-gen-go-v1 && \
cd protoc-gen-go-v1 && \
go mod edit -require google.golang.org/protobuf@v${GO_PROTOC_PLUGIN_V2_VERSION} && \
go mod tidy && \
go build -o $HOME/go/bin/protoc-gen-go_v1 ./protoc-gen-go)

# Install required Go tools for protoc and the post-processor.
RUN (export GOTOOLCHAIN='auto' && \
go install google.golang.org/protobuf/cmd/protoc-gen-go@v${GO_PROTOC_PLUGIN_V2_VERSION} && \
Expand Down
12 changes: 3 additions & 9 deletions internal/librariangen/bazel/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ type Config struct {

// Whether this library has a GAPIC rule at all.
hasGAPIC bool

// Whether the go_proto_library rule uses @io_bazel_rules_go//proto:go_grpc
hasLegacyGRPC bool
}

// HasGAPIC indicates whether the GAPIC generator should be run.
Expand Down Expand Up @@ -104,11 +101,6 @@ func (c *Config) HasRESTNumericEnums() bool { return c.restNumericEnums }
// used. This is trending toward typically true.
func (c *Config) HasGoGRPC() bool { return c.hasGoGRPC }

// HasLegacyGRPC indicates whether a go_proto_library rule uses
// @io_bazel_rules_go//proto:go_grpc to generate gRPC code. If so,
// the "plugins=grpc" option is passed to the legacy Go plugin.
func (c *Config) HasLegacyGRPC() bool { return c.hasLegacyGRPC }

// Validate ensures that the configuration is valid.
func (c *Config) Validate() error {
if c.hasGAPIC {
Expand Down Expand Up @@ -167,7 +159,9 @@ func Parse(dir string) (*Config, error) {
if c.hasGoGRPC {
return nil, fmt.Errorf("librariangen: misconfiguration in BUILD.bazel file, only one of go_grpc_library and go_proto_library rules should be present: %s", fp)
}
c.hasLegacyGRPC = strings.Contains(goProtoLibraryBlock, "@io_bazel_rules_go//proto:go_grpc")
if strings.Contains(goProtoLibraryBlock, "@io_bazel_rules_go//proto:go_grpc") {
return nil, fmt.Errorf("librariangen: BUILD.bazel requires legacy gRPC plugin: %s", fp)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message requires legacy gRPC plugin could be misinterpreted as a missing dependency. To improve clarity for users who encounter this error, consider rephrasing it to state that the legacy plugin is no longer supported.

Suggested change
return nil, fmt.Errorf("librariangen: BUILD.bazel requires legacy gRPC plugin: %s", fp)
return nil, fmt.Errorf("librariangen: BUILD.bazel uses legacy gRPC plugin (@io_bazel_rules_go//proto:go_grpc) which is no longer supported: %s", fp)

}
}
if err := c.Validate(); err != nil {
return nil, fmt.Errorf("librariangen: invalid bazel config in %s: %w", dir, err)
Expand Down
21 changes: 5 additions & 16 deletions internal/librariangen/bazel/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ go_grpc_library(
}
}

func TestParse_legacyProtocPlugin_noGrpc(t *testing.T) {
func TestParse_goProtoLibrary_noGrpc(t *testing.T) {
content := `
go_proto_library(
name = "asset_go_proto",
Expand Down Expand Up @@ -242,9 +242,6 @@ go_gapic_library(
if got.HasGoGRPC() {
t.Error("HasGoGRPC() = true; want false")
}
if got.HasLegacyGRPC() {
t.Error("HasLegacyGRPC() = true; want false")
}
}

func TestParse_legacyProtocPlugin_withGrpc(t *testing.T) {
Expand Down Expand Up @@ -275,17 +272,9 @@ go_gapic_library(
t.Fatalf("failed to write test file: %v", err)
}

got, err := Parse(tmpDir)
if err != nil {
t.Fatalf("Parse() failed: %v", err)
}

// Only test the bits related to protoc plugins
if got.HasGoGRPC() {
t.Error("HasGoGRPC() = true; want false")
}
if !got.HasLegacyGRPC() {
t.Error("HasLegacyGRPC() = false; want true")
_, err := Parse(tmpDir)
if err == nil {
t.Error("Parse() succeeded; want error")
}
Comment on lines +275 to 278
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make this test more robust, it's a good practice to assert not just that an error occurred, but that the correct error occurred. You can check if the error message contains a specific substring related to the legacy plugin. Also, consider using t.Fatal when an error is expected but not found, as it stops the test execution immediately, which is appropriate here.

Suggested change
_, err := Parse(tmpDir)
if err == nil {
t.Error("Parse() succeeded; want error")
}
_, err := Parse(tmpDir)
if err == nil {
t.Fatal("Parse() succeeded; want error")
}
if !strings.Contains(err.Error(), "legacy gRPC plugin") {
t.Errorf("Parse() returned unexpected error: got %v, want substring 'legacy gRPC plugin'", err)
}

}

Expand All @@ -302,6 +291,6 @@ func TestDisableGAPIC(t *testing.T) {
}
cfg.DisableGAPIC()
if cfg.HasGAPIC() {
t.Error("HasLegacyGRPC() = true; want false")
t.Error("HasGAPIC() = true; want false")
}
}
13 changes: 2 additions & 11 deletions internal/librariangen/protoc/protoc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type ConfigProvider interface {
HasRESTNumericEnums() bool
HasGoGRPC() bool
HasGAPIC() bool
HasLegacyGRPC() bool
}

// Build constructs the full protoc command arguments for a given API.
Expand Down Expand Up @@ -95,17 +94,9 @@ func Build(lib *request.Library, api *request.API, config ConfigProvider, source
"--experimental_allow_proto3_optional",
}
// All generated files are written to the /output directory.
// Which plugin(s) we use depends on whether the Bazel rule was go_grpc_library
// or go_proto_library:
// - If we're using go_rpc, we use the newer go plugin and the go-grpc plugin
// - Otherwise, use the "old" plugin (built explicitly in the Dockerfile)
args = append(args, "--go_out="+outputDir)
if config.HasGoGRPC() {
args = append(args, "--go_out="+outputDir, "--go-grpc_out="+outputDir, "--go-grpc_opt=require_unimplemented_servers=false")
} else {
args = append(args, "--go_v1_out="+outputDir)
if config.HasLegacyGRPC() {
args = append(args, "--go_v1_opt=plugins=grpc")
}
args = append(args, "--go-grpc_out="+outputDir, "--go-grpc_opt=require_unimplemented_servers=false")
}
if config.HasGAPIC() {
args = append(args, "--go_gapic_out="+outputDir)
Expand Down
39 changes: 2 additions & 37 deletions internal/librariangen/protoc/protoc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type mockConfigProvider struct {
restNumericEnums bool
hasGoGRPC bool
hasGAPIC bool
hasLegacyGRPC bool
}

func (m *mockConfigProvider) GAPICImportPath() string { return m.gapicImportPath }
Expand All @@ -53,7 +52,6 @@ func (m *mockConfigProvider) HasDiregapic() bool { return m.diregapic }
func (m *mockConfigProvider) HasRESTNumericEnums() bool { return m.restNumericEnums }
func (m *mockConfigProvider) HasGoGRPC() bool { return m.hasGoGRPC }
func (m *mockConfigProvider) HasGAPIC() bool { return m.hasGAPIC }
func (m *mockConfigProvider) HasLegacyGRPC() bool { return m.hasLegacyGRPC }

func TestBuild(t *testing.T) {
// The testdata directory is a curated version of a valid protoc
Expand Down Expand Up @@ -107,39 +105,6 @@ func TestBuild(t *testing.T) {
filepath.Join(sourceDir, "google/cloud/workflows/v1/nested/y.proto"),
},
},
{
name: "go_proto_library rule with legacy gRPC",
apiPath: "google/cloud/secretmanager/v1beta2",
reqID: "secretmanager",
config: mockConfigProvider{
gapicImportPath: "cloud.google.com/go/secretmanager/apiv1beta2;secretmanager",
transport: "grpc",
grpcServiceConfig: "secretmanager_grpc_service_config.json",
serviceYAML: "secretmanager_v1beta2.yaml",
releaseLevel: "ga",
metadata: true,
restNumericEnums: true,
hasGoGRPC: false,
hasGAPIC: true,
hasLegacyGRPC: true,
},
want: []string{
"protoc",
"--experimental_allow_proto3_optional",
"--go_v1_out=/output",
"--go_v1_opt=plugins=grpc",
"--go_gapic_out=/output",
"--go_gapic_opt=go-gapic-package=cloud.google.com/go/secretmanager/apiv1beta2;secretmanager",
"--go_gapic_opt=api-service-config=" + filepath.Join(sourceDir, "google/cloud/secretmanager/v1beta2/secretmanager_v1beta2.yaml"),
"--go_gapic_opt=grpc-service-config=" + filepath.Join(sourceDir, "google/cloud/secretmanager/v1beta2/secretmanager_grpc_service_config.json"),
"--go_gapic_opt=transport=grpc",
"--go_gapic_opt=release-level=ga",
"--go_gapic_opt=metadata",
"--go_gapic_opt=rest-numeric-enums",
"-I=" + sourceDir,
filepath.Join(sourceDir, "google/cloud/secretmanager/v1beta2/secretmanager.proto"),
},
},
{
name: "go_proto_library rule without legacy gRPC",
apiPath: "google/cloud/secretmanager/v1beta2",
Expand All @@ -158,7 +123,7 @@ func TestBuild(t *testing.T) {
want: []string{
"protoc",
"--experimental_allow_proto3_optional",
"--go_v1_out=/output",
"--go_out=/output",
"--go_gapic_out=/output",
"--go_gapic_opt=go-gapic-package=cloud.google.com/go/secretmanager/apiv1beta2;secretmanager",
"--go_gapic_opt=api-service-config=" + filepath.Join(sourceDir, "google/cloud/secretmanager/v1beta2/secretmanager_v1beta2.yaml"),
Expand All @@ -184,7 +149,7 @@ func TestBuild(t *testing.T) {
want: []string{
"protoc",
"--experimental_allow_proto3_optional",
"--go_v1_out=/output",
"--go_out=/output",
"-I=" + sourceDir,
filepath.Join(sourceDir, "google/cloud/secretmanager/v1beta2/secretmanager.proto"),
},
Expand Down