Failing to import `google/api` dependencies
Opened this issue · 16 comments
Describe the bug
Not sure if this is the expected behaviour or not, but as of 1.15.0 Kreya no longer implicitly loads the dependencies on Google's google/api common types. The error I'm getting:
google/api/annotations.proto: File not found.
v1/test.proto:7:1: Import "google/api/annotations.proto" was not found or had errors.
google/protobuf deps are getting loaded without any issues.
To Reproduce
- Create a proto file importing
google/api/annotations.proto. - Try to create a Kreya project importing the file.
Expected behavior
Dependencies are being fetched without requiring to store them in the project directory.
Environment (if possible, copy the information from the error dialog or the About menu):
- OS: MacOS Sonoma
- Kreya Version 1.15.0
We did some changes in Kreya 1.15 regarding importing proto files (we rely on the official protoc to do the parsing now). However, this should not have change anything for google/api imports, because those weren't implicitly imported by Kreya before.
Only types from google/protobuf are implicitly imported, which is the same behavior that protoc has. All others type have to be provided explicitely.
Are your google/api types located in the same place as your other protobuf files? If so, it could be that Kreya (mistakenly) picked them up in earlier version. Starting with Kreya 1.15, you may need to specifiy an additional import path (same as proto_path with protoc) so that Kreya finds the additional types.
@CommonGuy We are using Buf for protobuf development, so actually we've never been downloading the google dependencies to the project directory since one of the features of Buf is that it's able to fetch the dependencies on compilation.
In that case I'm clueless how Kreya was resolving them 🤔
Correction, the types from google/api/annotations.proto and google/api/http.proto were implicitely loaded in Kreya 1.14 (due to the library used). Switching to the official protoc implementation lost us this feature. This will be fixed in the next release.
@CommonGuy is there any way to install older version of kreya? like 1.14.1 for example
@sneaky-peeky We currently do not provide a way to download older versions.
However, we have released a fix for this on our beta release channel. If you want to try it, see https://kreya.app/docs/faq/#how-can-i-use-kreya-beta-versions
@CommonGuy I had the same issue in Kreya 1.15.0, just upgraded to 1.15.1 beta and now the issue changed to google.api.http being loaded twice:
google/api/http.proto:33:21: "google.api.Http.rules" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:29:9: "google.api.Http" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto: "google.api.HttpRule.pattern" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:321:10: "google.api.HttpRule.selector" is already defined in file "node_modules/protobufjs/google/api/http.proto".
[....etc....]
In my repo, the protofiles are in pkg/proto but there are also some proto files in pkg/proto/node_modules, and there is no way to load pkg/proto/*.proto without also loading pkg/proto/**/*.proto. So all the proto files in node_modules are loaded as well, which includes a second copy of google/api/http.proto. In Kreya 1.14, this wasn't an issue.
@sgielen I cannot reproduce this. Is the http.proto only in pkg/proto/node_modules/... or also a second time in pkg/proto ? Or is it maybe twice in pkg/proto/node_modules/... ?
@ni507 It is only once in node_modules, but I just noticed that only http.proto does not cause the issue, but having also annotations.proto in the same directory does cause the issue. Perhaps because http.proto itself is parsed, and then annotations.proto also imports http.proto and this causes the double definitions issue?
In case it matters, I've attached my exact two copies. Renamed to .txt, so that upload to github is allowed.
annotations.proto.txt
http.proto.txt
google/api/http.proto:33:21: "google.api.Http.rules" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:29:9: "google.api.Http" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto: "google.api.HttpRule.pattern" is already defined in file "node_modules/protobufjs/google/api/http.proto".
[...]
node_modules/protobufjs/google/api/annotations.proto:5:1: Import "google/api/http.proto" was not found or had errors.
node_modules/protobufjs/google/api/annotations.proto:10:5: "google.api.HttpRule" seems to be defined in "node_modules/protobufjs/google/api/http.proto", which is not imported by "node_modules/protobufjs/google/api/annotations.proto". To use it here, please add the necessary import.
@ni507 just checking - can you reproduce this on your end with this information, or is there something I could investigate further to help reproduce this?
@sgielen I was able to reproduce it, but only in a way, as it did not work in version 1.14 either.
I created a folder with protos and this structure:
protos
│ some.proto
│ ....
└───node_modules
│ │
│ └───google/api
│ │ http.proto
│ │ annotations.proto
Then I get your error because I used import "google/api/http.proto"; in the some.proto file. But I also tried this in Kreya 1.14, which also resulted in an error.
To fix this in the current beta version, I added an import path in the importer settings to the node_modules folder like this {insert here your path}/protos/node_modules. Then the import works.
Can you try adding an import path to your importer as well?
I'm wondering why your setup worked on version 1.14. Could something be different in my example?
@sgielen I was able to reproduce it, but only in a way, as it did not work in version 1.14 either.
That is interesting.
I did some further investigating by removing files from my node_modules until I had the minimum set of files that caused the error. Lo and behold, it's just those two files I sent earlier: annotations.proto and http.proto, even outside of any node_modules. So for me, just having those two files reproduces the issue on 1.15.1beta1 in a way that doesn't seem to happen for you, is that correct?
I have a video here to show how I reproduce this with just those two files in my proto path:
https://github.com/riok/Kreya/assets/328769/62a62abb-5525-4a93-aa83-d25bafbbab9d
In case that doesn't work, here's the video in a .zip file:
reproduce.zip
And the exact two proto files in a .zip:
proto.zip
Does Kreya use anything from the host (protoc/proto libraries) in a way that could change our outcomes? (This might also be the reason that creating a project takes a relatively long time, or is that normal?)
I'm not 100% certain my earlier installed Kreya version was 1.14, but I am certain I didn't change anything in node_modules between going from that version to 1.15. If you want, I can try to find a 1.14 download and perform the same steps with it on my machine?
@sgielen I've verified it with a similar setup.
When i have a subfolder with the 2 files annotations.proto and http.proto it will not generate an error during the import in v1.14, while in v1.15.1-beta.1 it does.
We will look into this with high priority.
It seems the only workaround right now is to explicitly remove these 2 files from your project, since they are imported per default in v1.15.1-beta.1.
@hoyau thank you for this! I can’t remove them unfortunately, as they are needed during the build. But, for now, as a workaround, I just copy the proto files elsewhere and import from there until it’s fixed in a newer version. Thanks a lot for the fast responses!
@sgielen I analyzed this for a while. Your directory structure looks something like this:
pkg/proto
│ some.proto
│ ....
└───node_modules
│ │
│ └───google/api
│ │ http.proto
│ │ annotations.proto
In your proto file importer, you added the pkg/proto directory as the "source". Kreya then imports all proto files within that directory and subdirectories. As you mentioned, there is currently no way to change that.
The problem is that the http.proto file is imported as a relative path to the root (pkg/proto), so it is turned into node_modules/google/api/http.proto. protoc thinks is a completely new file and parses it. Since other files import google/api/http.proto, that one is imported as well (the one that Kreya provides), which results in the duplication error.
To solve this issue, you can add the node_modules folder as an import path (in the Kreya file importer options). It should be the first import path that is declared, as the order matters. Since the node_modules is added as an import path, the duplicated files are recognized and everything should work.
Thanks for this @CommonGuy! To be exact, the directory structure is:
pkg/proto$ find . -name '*.proto'
./some.proto
...
./node_modules/grpc-tools/bin/google/protobuf/timestamp.proto
./node_modules/grpc-tools/bin/google/protobuf/...
./node_modules/protobufjs/google/api/http.proto
./node_modules/protobufjs/google/api/...
./node_modules/protobufjs/google/protobuf/api.proto
./node_modules/protobufjs/google/protobuf/...
I tried to add ../pkg/proto/node_modules/grpc-tools/bin and ../pkg/proto/node_modules/protobufjs as import paths and leave ../pkg/proto as proto directory, but got the error:
[repo]/pkg/proto/node_modules/protobufjs/google/protobuf/api.proto: Input is shadowed in the --proto_path by "[repo]/pkg/proto/node_modules/grpc-tools/bin/google/protobuf/api.proto". Either use the latter file as your input or reorder the --proto_path so that the former file's location comes first.
If I only put ../pkg/proto/node_modules/protobufjs as an import path, since I don't really need the files provided by grpc-tools/bin, I still get:
node_modules/grpc-tools/bin/google/protobuf/timestamp.proto:140:9: "google.protobuf.Timestamp.seconds" is already defined in file "google/protobuf/timestamp.proto".
node_modules/grpc-tools/bin/google/protobuf/timestamp.proto:146:9: "google.protobuf.Timestamp.nanos" is already defined in file "google/protobuf/timestamp.proto".
node_modules/grpc-tools/bin/google/protobuf/timestamp.proto:136:9: "google.protobuf.Timestamp" is already defined in file "google/protobuf/timestamp.proto".
To be perfectly honest, I think at least one of the root causes of this is that it's impossible to specify a directory without its subdirectories, or at least excluding specific subdirectories. In my case, node_modules contains libraries that are necessary to transform my protos to Typescript, and those libraries require copies of the Google .protos, so there's not really any other place where those could reasonably be other than pkg/proto. Luckily, none of those .proto files include any gRPC services, just Protobuf message definitions. If any of those libraries ever start shipping the standard health.proto for example, I expect Kreya would also import it as a service, even though it's not really in the directory I wanted to specify (simply pkg/proto/*.proto).
I think if users were allowed to give their own glob format, i.e. allowing to input pkg/proto/*.proto instead of automatically translating pkg/proto into pkg/proto/**/*.proto, this would also solve the issue without sacrificing any existing features.
Ah yes, with that many subfolders each including their own google/api dependencies it's difficult/impossible to handle this correctly.
Yes, I think allowing users to input a custom glob or adding a "exclusion list" would solve this problem