From d63f8d4dcfe262f77ae03d2a0fe1da2507b5ec54 Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Thu, 27 Apr 2023 12:52:54 -0700 Subject: [PATCH] Revert "[CSHARP] Add base_namespace experimental option to C# plugin" (#32957) Reverts grpc/grpc#32636 ``` src/compiler/csharp_generator_helpers.h:25:7: error: no member named 'compiler' in namespace ... src/compiler/csharp_generator_helpers.h:25:25: error: no member named 'csharp' in namespace 'compiler' ... ``` --- src/compiler/csharp_generator_helpers.h | 23 +---- src/compiler/csharp_plugin.cc | 11 +-- src/csharp/BUILD-INTEGRATION.md | 31 +----- test/csharp/codegen/BUILD | 19 ---- .../basenamespace/proto/namespacetest.proto | 37 ------- .../csharp_codegen_base_namespace_test.sh | 97 ------------------- 6 files changed, 10 insertions(+), 208 deletions(-) delete mode 100644 test/csharp/codegen/basenamespace/proto/namespacetest.proto delete mode 100755 test/csharp/codegen/csharp_codegen_base_namespace_test.sh diff --git a/src/compiler/csharp_generator_helpers.h b/src/compiler/csharp_generator_helpers.h index 31c6aad6eaa..466c645859a 100644 --- a/src/compiler/csharp_generator_helpers.h +++ b/src/compiler/csharp_generator_helpers.h @@ -22,30 +22,13 @@ #include "src/compiler/config.h" #include "src/compiler/generator_helpers.h" -using google::protobuf::compiler::csharp::GetOutputFile; - namespace grpc_csharp_generator { inline bool ServicesFilename(const grpc::protobuf::FileDescriptor* file, const std::string& file_suffix, - const std::string& base_namespace, - std::string& out_file, std::string* error) { - // Support for base_namespace option is **experimental**. - // - // If base_namespace is provided then slightly different name mangling - // is used to generate the service file name. This is because this - // uses common code with protoc. For most file names this will not - // make a difference (only files with punctuation or numbers in the - // name.) - // Otherwise the behavior remains the same as before. - if (base_namespace.empty()) { - out_file = grpc_generator::FileNameInUpperCamel(file, false) + file_suffix; - } else { - out_file = GetOutputFile(file, file_suffix, true, base_namespace, error); - if (out_file.empty()) { - return false; - } - } + std::string& out_file_name_or_error) { + out_file_name_or_error = + grpc_generator::FileNameInUpperCamel(file, false) + file_suffix; return true; } diff --git a/src/compiler/csharp_plugin.cc b/src/compiler/csharp_plugin.cc index 0136f4bea48..f63bb4de555 100644 --- a/src/compiler/csharp_plugin.cc +++ b/src/compiler/csharp_plugin.cc @@ -43,8 +43,6 @@ class CSharpGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { bool generate_client = true; bool generate_server = true; bool internal_access = false; - std::string base_namespace = ""; - // the suffix that will get appended to the name generated from the name // of the original .proto file std::string file_suffix = "Grpc.cs"; @@ -57,11 +55,6 @@ class CSharpGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { internal_access = true; } else if (options[i].first == "file_suffix") { file_suffix = options[i].second; - } else if (options[i].first == "base_namespace") { - // Support for base_namespace option in this plugin is experimental. - // The option may be removed or file names generated may change - // in the future. - base_namespace = options[i].second; } else { *error = "Unknown generator option: " + options[i].first; return false; @@ -76,8 +69,8 @@ class CSharpGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { // Get output file name. std::string file_name; - if (!grpc_csharp_generator::ServicesFilename( - file, file_suffix, base_namespace, file_name, error)) { + if (!grpc_csharp_generator::ServicesFilename(file, file_suffix, + file_name)) { return false; } std::unique_ptr output( diff --git a/src/csharp/BUILD-INTEGRATION.md b/src/csharp/BUILD-INTEGRATION.md index 644adf38767..4f2f57cfcdc 100644 --- a/src/csharp/BUILD-INTEGRATION.md +++ b/src/csharp/BUILD-INTEGRATION.md @@ -173,33 +173,12 @@ to perform code generation. Here is an overview of the available `grpc_csharp_pl | no_client | off | Don't generate the client stub | | no_server | off | Don't generate the server-side stub | | internal_access | off | Generate classes with "internal" visibility | -| file_suffix | Grpc.cs | The suffix that will get appended to the name of the generated file. **Can only be used on the command line.** | -| base_namespace | none | *Experimental - may change or be removed.* Same as `base_namespace` for `protoc` [C# options](https://protobuf.dev/reference/csharp/csharp-generated/#compiler_options) . **Can only be used on the command line.** | -To use these options with `Grpc.Tools` specify them in the __GrpcOutputOptions__ -metadata in the `` item. - -Notes: -- `file_suffix` and `base_namespace` should not be used with `Grpc.Tools`. Using them will break the build. - -- using `base_namespace` changes the algorithm for the generated file names to align it with the algorithm used by `protoc`. - - This only affects files with punctuation or numbers in the name. E.g. `hello.world2d.proto` now generates file `HelloWorld2DGrpc.cs` instead of `Hello.world2dGrpc.cs` - -To use these options on the command line specify them with the `--grpc_opt` -option. - -Code generated by `protoc` is independent of the plugin and you may also need to specify C# options for this with `--csharp_opt`. -These are [documented here](https://protobuf.dev/reference/csharp/csharp-generated/#compiler_options). - - e.g.: - -```bash -protoc --plugin=protoc-gen-grpc=grpc_csharp_plugin \ - --csharp_out=OUT_DIR \ - --csharp_opt=base_namespace=Example \ - --grpc_out=OUT_DIR \ - --grpc_opt=no_server,base_namespace=Example \ +Note that the protocol buffer compiler has a special commandline syntax for plugin options. +Example: +``` +protoc --plugin=protoc-gen-grpc=grpc_csharp_plugin --csharp_out=OUT_DIR \ + --grpc_out=OUT_DIR --grpc_opt=lite_client,no_server \ -I INCLUDE_DIR foo.proto ``` ## Environment Variables diff --git a/test/csharp/codegen/BUILD b/test/csharp/codegen/BUILD index 44ff93f7745..378c20464e7 100644 --- a/test/csharp/codegen/BUILD +++ b/test/csharp/codegen/BUILD @@ -56,22 +56,3 @@ grpc_sh_test( ], uses_polling = False, ) - -grpc_sh_test( - name = "csharp_codegen_base_namespace_test", - size = "small", - srcs = ["csharp_codegen_base_namespace_test.sh"], - data = [ - "basenamespace/proto/namespacetest.proto", - "//src/compiler:grpc_csharp_plugin", - "@com_google_protobuf//:protoc", - ], - tags = [ - "no_windows", - "noasan", - "nomsan", - "notsan", - "noubsan", - ], - uses_polling = False, -) diff --git a/test/csharp/codegen/basenamespace/proto/namespacetest.proto b/test/csharp/codegen/basenamespace/proto/namespacetest.proto deleted file mode 100644 index dd02efb0c2f..00000000000 --- a/test/csharp/codegen/basenamespace/proto/namespacetest.proto +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2023 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -syntax = "proto3"; - -package test.csharp.codegen.basenamespace.test; - -option csharp_namespace = "Example.V1.CodegenTest"; - -// The greeting service definition. -service Greeter { - // Sends a greeting - rpc SayHello (HelloRequest) returns (HelloReply) {} - - rpc SayHelloStreamReply (HelloRequest) returns (stream HelloReply) {} -} - -// The request message containing the user's name. -message HelloRequest { - string name = 1; -} - -// The response message containing the greetings -message HelloReply { - string message = 1; -} diff --git a/test/csharp/codegen/csharp_codegen_base_namespace_test.sh b/test/csharp/codegen/csharp_codegen_base_namespace_test.sh deleted file mode 100755 index f0f85877ee4..00000000000 --- a/test/csharp/codegen/csharp_codegen_base_namespace_test.sh +++ /dev/null @@ -1,97 +0,0 @@ -#!/bin/bash -# Copyright 2023 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Run this script via bazel test -# It expects that protoc and grpc_csharp_plugin have already been built. - -# Simple test - compare generated output to expected files - -set -x - -TESTNAME=basenamespace - -# protoc and grpc_csharp_plugin binaries are supplied as "data" in bazel -PROTOC=./external/com_google_protobuf/protoc -PLUGIN=./src/compiler/grpc_csharp_plugin - -# where to find the test data -DATA_DIR=./test/csharp/codegen/${TESTNAME} - -# output directory for the generated files -PROTO_OUT=./proto_out -rm -rf ${PROTO_OUT} -mkdir -p ${PROTO_OUT} - -# run protoc and the plugin specifying the base_namespace options -$PROTOC \ - --plugin=protoc-gen-grpc-csharp=$PLUGIN \ - --csharp_out=${PROTO_OUT} \ - --grpc-csharp_out=${PROTO_OUT} \ - --csharp_opt=base_namespace=Example \ - --grpc-csharp_opt=base_namespace=Example \ - -I ${DATA_DIR}/proto \ - ${DATA_DIR}/proto/namespacetest.proto - -# log the files generated -ls -lR ./proto_out - -# Verify the output files exist in the right location. -# The base_namespace option does not change the generated code just -# the location of the files. Contents are not checked in this test. - -# The C# namespace option in the proto file of "Example.V1.CodegenTest" -# combined with the command line options above should mean the generated files -# are created in the output directory "V1/CodegenTest" - -# First file is generated by protoc. -[ -e ${PROTO_OUT}/V1/CodegenTest/Namespacetest.cs ] || { - echo >&2 "missing generated output, expecting V1/CodegenTest/Namespacetest.cs" - exit 1 -} - -# Second file is generated by the plugin. -[ -e ${PROTO_OUT}/V1/CodegenTest/NamespacetestGrpc.cs ] || { - echo >&2 "missing generated output, expecting V1/CodegenTest/NamespacetestGrpc.cs" - exit 1 -} - -# Run again without the base_namespace options to check the files are created -# in the root of the output directory - -rm -rf ${PROTO_OUT} -mkdir -p ${PROTO_OUT} - -$PROTOC \ - --plugin=protoc-gen-grpc-csharp=$PLUGIN \ - --csharp_out=${PROTO_OUT} \ - --grpc-csharp_out=${PROTO_OUT} \ - -I ${DATA_DIR}/proto \ - ${DATA_DIR}/proto/namespacetest.proto - -ls -lR ./proto_out - -[ -e ${PROTO_OUT}/Namespacetest.cs ] || { - echo >&2 "missing generated output, expecting Namespacetest.cs" - exit 1 -} - -[ -e ${PROTO_OUT}/NamespacetestGrpc.cs ] || { - echo >&2 "missing generated output, expecting NamespacetestGrpc.cs" - exit 1 -} - -# Run one extra command to clear $? before exiting the script to prevent -# failing even when tests pass. -echo "Plugin test: ${TESTNAME}: passed."