search

memsharded / conan-protobuf

Google protocol buffers conan package
MIT License
1 stars 17 forks source link

Release/3.3.0 #17

Open ebclark2 opened 7 years ago

ebclark2 commented 7 years ago

I was unable to use built in proto types with the existing package using proto3 syntax (did not try 2). This seemed to be because protoc couldn't find its own proto include directory. Using make install seems to fix this, though I have not tested for macos, nor made changes for windows...

To reproduce the problem include a built in proto in a proto file, such as google/protobuf/struct.proto.

memsharded commented 7 years ago

Thanks very much for trying to fix this. However, as you are pointing out, this seems a non-portable solution. Wouldn't it be better to directly copy all builtins protos when packaging?

 self.copy("*.proto", ...)
ebclark2 commented 7 years ago

I tried self.copy("*.proto", ...) first but for some reason protoc was still not able to find the .proto files. I verified they were in the correct place but couldn't get it going. It wouldn't be the first time I just goofed on testing something if it does work.

It seems to me using make install, or whatever the install technique is for the build tool, should be desired, since it ensures no build shenanigans, such as setting permissions, are skipped, and leaves the conan package to just copy the results of the build.

While this is a non-portable solution, there were already separate build and package sections and this made a relatively large reduction in the size of the package function. I'd like to think it now works properly on linux/macos and windows is no worse off but the CI jobs are failing with zlib errors so I'm not sure. I wouldn't be surprised if it was worse off on a mac.