Fix missing include clause in the gRPC proto compilation #36153
Conversation
Fix missing include statements in the proto compilation and also improve the syntax of the proto include/exclude clauses (comma-separated list accepting spaces)
| var pathToProtoFile = files.normalize().toAbsolutePath(); | ||
| var pathToParentDir = files.getParent(); | ||
| // Add the proto file to the list of proto to compile, but also add the directory containing the | ||
| // proto file to the list of directories to include (it's a set, so no duplicate). |
There was a problem hiding this comment.
Because if you don't, it cannot resolve import statements from proto files included in the application. That was the bug.
There was a problem hiding this comment.
Ah. B/c the paths are probably relative to the file (it's dir) that imports them?
|
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
|
The failure is related, investigating... |
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
|
I fixed the problem. It was more a CI issue than a real issue. We were computing a directory location. With our CI we had 2 distinct locations (one from the first build and one from the IT build) leading to duplicate proto files. I fixed it to generate unique but conservative locations. |
|
@cescoffier should we consider backporting this for 3.4.2? |
|
No I think it's fine. Also, this Pr introduced a regression I fixed in another PR. |
Fix missing include statements in the proto compilation and also improve the syntax of the proto include/exclude clauses (comma-separated list accepting spaces)
Fix #36078