fullstorydev / grpcurl

Like cURL, but for gRPC: Command-line tool for interacting with gRPC servers
MIT License
10.96k stars 508 forks source link

Failed to process proto source files.: given files include multiple copies of "common.proto" starting with v1.8.8 #417

Open mariemat opened 1 year ago

mariemat commented 1 year ago

When I execute a grpcurl command to execute a call, and I provide multiple proto files that each import the same common.proto, then I get the error :

Failed to process proto source files.: given files include multiple copies of "common.proto" 

This happens only with 1.8.8 (not with 1.8.6, nor 1.8.7) I believe this is common for multiple proto files to import the same utilities, and grpcurl should not enforce that condition

jhump commented 1 year ago

I believe this is common for multiple proto files to import the same utilities

@mariemat, multiple proto files merely importing the same other files wouldn't be the issue. Something else is going awry that causes the same file to be processed 2x (and just importing a file doesn't cause it to be re-processed).

Could you provide more details about the actual command-line arguments you are providing and perhaps a snippet (mainly the precise import statements) from at least two of the files that import "common.proto"?

Also, could you try installing the latest from head (go install github.com/fullstorydev/grpcurl@master) to see if this still occurs? (It's possible this may have been fixed in #416.)

mariemat commented 1 year ago

Thanks @jhump,

I checked with grpcurl version "v1.8.8-2-g7a845ca" (https://github.com/fullstorydev/grpcurl/pull/416) and I do see the same error.

There is not trick around the imports. The hierarchy of import is like (sorry I can not share a lot):

The g.proto and 'common.proto' are provided on the cli with many more, but only those described here are involved with common.proto.

If I remove the common.proto from the CLI params, then it does work (with all versions).

jhump commented 1 year ago

If I remove the common.proto from the CLI params, then it does work (with all versions).

Ah. What does the actual import line look like then? I suspect you have a mismatch between the relative path used on the command-line and the import statements in referring files. So the tool doesn't know that the two paths are the same file and thus tries to load both.

The changes I mentioned (in #416) should have helped to match the behavior of the older versions more closely. But if they did not, then can you provide more info about:

  1. The actual command-line arguments you are providing. In particular, what are the -proto and -import-path values you are sending. (Anonymized like above, to not reveal details of your codebase/schema is fine.)
  2. The actual layout on disk of the relevant source files. Are they all in a single tree with one import path, split across directories with multiple import paths, etc. Showing the absolute paths for each file in the schema (anonymized of course) would help.
iwittkau commented 1 year ago

I got the same error.

I think the reason is that grpcui now resolves all imported proto packages from -import-path.

Example:

example
├── service1
│   └── service1.proto # imports service2/service2.proto
└── service2
    └── service2.proto

If you now run grpcui like this

cd example
grpcui -import-path=. \
    -proto=service1/service1.proto \ 
    -proto=service2/service2.proto \ 
    host:port

it will produce the output:

Failed to process proto source files.: given files include multiple copies of "service2/service2.proto"

Not sure if this is intended behavior, but the workaround for now is to remove -proto=service2/service2.proto flag.

jhump commented 1 year ago

Not sure if this is intended behavior

Definitely not. That is surprising that so simple an example triggers it with the latest (merged but unreleased) code, given the tests I added to the underlying function in protoreflect 😕. I'll see if I can repro the issue with the suggested layout and arguments and debug a little further if so.