Don't hardcode path to google/protobuf/descriptor.proto #20
Conversation
kolyshkin
force-pushed
the
inc-paths
branch
from
feeb9ad to
d1f3e1a
Compare
|
@stevvooe PTAL |
descriptors.go
Outdated
| return err | ||
| } | ||
|
|
||
| const descProto = "google/protobuf/descriptor.proto" |
There was a problem hiding this comment.
This search code should be outside of marshalTo. Could we just take this path as an argument to newDescriptorSet?
There was a problem hiding this comment.
I have to admit I don't fully get it. Do you mean something like this (please see updated commit)?
If not, maybe you can create something based on the code/idea in this patch?
Go 1.11 and 1.10 are currently supported. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Despite the ability to specify multiple include directories in `config.Include.After`, the `/usr/local/bin` path to `google/protobuf/descriptor.proto` is hardcoded in `func (*descriptorSet).marshalTo()`. Fix by introducing a function to search for this file, and providing the path found to newDesceriptorSet. This (together with the addition of `/usr/include` path to `config.Include.After`) makes it possible to use protobuilder with distro-installed protoc (e.g. with Debian/Ubuntu it is `apt install libprotobuf-dev protobuf-compiler`). Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
inc-paths
branch
from
472e7af to
95af7b7
Compare
|
@kolyshkin Thanks! |
|
OK it's not good:
I can't reproduce it locally (for whatever reason) but it seems that ditching the -I was a mistake. I'll work on a followup PR to fix this. |
Don't hardcode path to google/protobuf/descriptor.proto
|
@kolyshkin didn't see your comment. I opened a pull request as I also had the issue, |
3 participants
Despite the ability to specify multiple include directories
in
config.Include.After, the/usr/local/binpath togoogle/protobuf/descriptor.protois hardcoded infunc (*descriptorSet).marshalTo().Fix by passing
c.Config.After(i.e. list of user-suppliedinclude paths) to the above function, and iterate over those
to search for
descriptor.proto.This (together with the addition of
/usr/includepath toconfig.Include.After) makes it possible to use protobuilderwith distro-installed protoc (e.g. with Debian/Ubuntu it is
apt install libprotobuf-dev protobuf-compiler).