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

Grpc.Tools causes a build error when .proto file name contains dot #17661

Closed
P-tan opened this issue Jan 8, 2019 · 9 comments · Fixed by #18470
Closed

Grpc.Tools causes a build error when .proto file name contains dot #17661

P-tan opened this issue Jan 8, 2019 · 9 comments · Fixed by #18470

Comments

@P-tan
Copy link

P-tan commented Jan 8, 2019

What version of gRPC and what language are you using?

Grpc 1.17.1
Grpc.Core 1.17.1
Grpc.Tools 1.17.1

Language: C#

What operating system (Linux, Windows, …) and version?

Windows 10 Pro 1803 64bit

What runtime / compiler are you using (e.g. python version or version of gcc)

.NET Core SDK 2.1.502

What did you do?

I tried to build .proto which file name contains dot(e.g. hello.world.proto) by Grpc.Tools.

source files: Greeter.zip

What did you expect to see?

Build succeeds.

What did you see instead?

Following build errors:

obj\Debug\netstandard1.5\Hello.worldGrpc.cs(18,57,18,69): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(19,57,19,67): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(21,53,21,65): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(21,86,21,96): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(43,117,43,129): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(43,77,43,87): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(81,80,81,92): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(81,41,81,51): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(91,80,91,92): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(91,41,91,51): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(103,107,103,119): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(103,62,103,72): error CS0234: The type or namespace name 'HelloReply' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
obj\Debug\netstandard1.5\Hello.worldGrpc.cs(113,107,113,119): error CS0234: The type or namespace name 'HelloRequest' does not exist in the namespace 'Helloworld' (are you missing an assembly reference?)
@jtattermusch
Copy link
Contributor

CC @kkm000

@kkm000
Copy link
Contributor

kkm000 commented Jan 9, 2019

Reproduced. Thanks for the report!

@vctrferreira
Copy link

Any workaround for that? I have the same problem using 1.19.1

@jtattermusch
Copy link
Contributor

@vctrferreira It seems that the simplest workaround would be to rename you .proto file to not contain a dot

@jtattermusch
Copy link
Contributor

I think I found the rootcause:

There is a mismatch between the file name generated by protoc and the one expected by CSharpGeneratorServices:

bool doGrpc = GrpcOutputPossible(protoItem);

Protobuf uses google::protobuf::compiler:csharp::GetOutputFile:
https://github.com/protocolbuffers/protobuf/blob/b68a347f56137b4b1a746e8c7438495a6ac1bd91/src/google/protobuf/compiler/csharp/csharp_helpers.cc#L356

which ends up using
https://github.com/protocolbuffers/protobuf/blob/b68a347f56137b4b1a746e8c7438495a6ac1bd91/src/google/protobuf/compiler/csharp/csharp_helpers.cc#L165

which has special handling for '.' in the file name (in this case it get skipped and the next letter is going to be uppercase).

@kkm000
Copy link
Contributor

kkm000 commented Mar 20, 2019

@jtattermusch, yes, I've seen that. I'm very sorry for sitting too long on the issues.

Actually, the root root cause is the difference between protobuf-generated and grpc-plugin-generated filenames, so we end up with HelloWorld.cs but Hello.worldGrpc.cs from protoc. I replicated protoc's way of producing the filename (thus expecting HelloWorldGrpc.cs to be generated in this case), but the generator does not handle the dot specially, if you look at its code (https://github.com/grpc/grpc/blob/c28d35dc1/src/compiler/csharp_generator_helpers.h#L27 and, principally, https://github.com/grpc/grpc/blob/c28d35dc1/src/compiler/generator_helpers.h#L127).

I'm tempted to fix grpc codegen's C# filename production to match that of protoc (the dot is an edge case). Do you think it would be too breaking?

@jtattermusch
Copy link
Contributor

@kkm000 the problem is that the method for generating the output filename is used by all the grpc_*_plugin regardless of the language I believe - so this could potentially break other languages as well.

Here's what I propose:

  1. (hotfix) in Grpc.Tools, replicate the logic for generating file name that protoc uses and use the correct algorithm depending on the value of doGrpc. Btw, in contrary to what you're saying I think currently the CSharpGeneratorServices logic replicates the grpc_*_plugin behavior as it doesn't handle the dot and other nonalphanumeric characters the way protoc does. We'll be cutting the next release branch Tuesday next week so if we can make it until then, it would be great.

  2. file an issue that grpc_*_plugins are using a filename that's incompatible with what protoc does.

  3. see if the right solution for 2) is to change the path generator in grpc plugins (without being in a hurry because thing are broken) and do that if so.

@kkm000
Copy link
Contributor

kkm000 commented Mar 21, 2019

@jtattermusch SGTM, I'll do the (1) ASAP.

Yes, you are right about which of the two algorithms I implemented, it's the other way round. Here's the dependency file from protoc, actually generated files

$ cat obj/Debug/netstandard1.5/bf0dab1a5244446f_hello.world.protodep
obj/Debug/netstandard1.5/Hello.worldGrpc.cs \
obj/Debug/netstandard1.5/HelloWorld.cs: ../../../protos/hello.world.proto

So we do generate Hello.worldGrpc.cs from hello.world.proto, protoc does HelloWorld.cs, and our MSBuild tooling expects Hello.world.cs and Hello.worldGrpc.cs.

@kkm000
Copy link
Contributor

kkm000 commented Mar 22, 2019

ETA couple hours, done and works locally. I am checking if I can cram fixes to other reported issues into one PR.

jtattermusch pushed a commit that referenced this issue Mar 22, 2019
protoc and gRPC codegens differently treat non-ASCII letter
characters and symbols other than underscores when constructing
their respective output filenames for generated .cs files.
This change reproduces their respective behaviors exactly.

Fixes #17661
@lock lock bot locked as resolved and limited conversation to collaborators Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants