[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/170 @jcrist Will review that PR. Thanks! ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user jcrist commented on the issue: https://github.com/apache/orc/pull/170 Thanks @majetideepak. If you also have a chance, it would be nice to get #185 reviewed and merged too. Waiting on both of these so I can use orc in a larger cmake project. ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/170 @jcrist Yes, this should be good to merge with rebasing. I will try to merge this in by today. ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user jcrist commented on the issue: https://github.com/apache/orc/pull/170 I find myself in need of this PR. Looks like this has a few merge conflicts, but other than that is this ready to merge? ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user omalley commented on the issue: https://github.com/apache/orc/pull/170 Ok ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/170 In this PR, `FindProtobuf.cmake` checks if the specified path `PROTOBUF_HOME` contains the required headers and sets `PROTOBUF_INCLUDE_DIRS`, checks for the library and sets `PROTOBUF_LIBRARIES`, and checks for the executable and sets `PROTOBUF_EXECUTABLE`. Even if one of them is missing from the user specified location, the protobuf library gets downloaded. In a future patch, we can extend the `FindX.cmake` to check default system paths (except for protobuf). But by default, we should only download. ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user omalley commented on the issue: https://github.com/apache/orc/pull/170 It looked like you were searching for the protobuf headers and libraries. Is that not the case? Why do we need all of the FindX cmake files for a simple test of a variable? ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/170 I did not understand the first point. `PROTOBUF_HOME` must be explicitly set to use the user specified version. Otherwise, the library gets downloaded. Will remove the travis-ci test. Thanks! ---
[GitHub] orc issue #170: ORC-207: [C++] Enable users the ability to provide their own...
Github user omalley commented on the issue: https://github.com/apache/orc/pull/170 A couple of things: * I'd prefer if the user had to explicitly set the environment to get a non-default version. That is particularly true for protobuf, which is well known to not be API compatible between versions. * I don't think it is necessary to have travis-ci do the build with the environment variables set. ---
