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

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

Open
mariemat opened this issue Oct 10, 2023 · 5 comments

Comments

@mariemat
Copy link

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
Copy link
Contributor

jhump commented Oct 10, 2023

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
Copy link
Author

mariemat commented Oct 11, 2023

Thanks @jhump,

I checked with grpcurl version "v1.8.8-2-g7a845ca" (#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):

  • g.proto
    • n.proto
      • common.proto
      • k.proto
        • common.proto

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
Copy link
Contributor

jhump commented Oct 11, 2023

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
Copy link

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
Copy link
Contributor

jhump commented Oct 13, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants