mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Clemmer <clemmer.alexan...@gmail.com>
Subject Re: Review Request 50415: Added build step to build Java Protobuf.
Date Tue, 13 Dec 2016 00:48:15 GMT


> On Dec. 8, 2016, 8:59 p.m., Alex Clemmer wrote:
> > 3rdparty/CMakeLists.txt, line 347
> > <https://reviews.apache.org/r/50415/diff/6/?file=1579858#file1579858line347>
> >
> >     If I'm understanding this correctly, this will download a fresh copy of the
protobuf tarball, and then build only the Java parts of it.
> >     
> >     Since we're already downloading this for the normal Protobuf build, it seems
like it would be easier to get rid of the `ExternalProject_Add` call and simply append the
java config, build, and install commands to the protobuf config, build, and install commands.
For example:
> >     
> >     ```
> >     set(PROTOBUF_BUILD_CMD ${PROTOBUF_BUILD_CMD} ${PROTOBUF_JAVA_BUILD_CMD})
> >     ```
> >     
> >     This should cause the normal protobuf build to build the Java stuff as well,
no?
> 
> Srinivas Brahmaroutu wrote:
>     I prefer to build the jar as a step for Java and I will add another step for Python.
Hope this works.

Can you speak a little more about why you prefer it? I personally would prefer to keep the
Java, native, and Python builds of PB all coming from the same source tree just because it's
less error-prone.


> On Dec. 8, 2016, 8:59 p.m., Alex Clemmer wrote:
> > 3rdparty/CMakeLists.txt, line 311
> > <https://reviews.apache.org/r/50415/diff/6/?file=1579858#file1579858line311>
> >
> >     I must be missing something, because I don't quite see why it's necessary to
copy `protoc` over to the Java root?
> 
> Srinivas Brahmaroutu wrote:
>     the build step in the source code requires protoc at relative location ../src/protoc

Ah, you seem to have removed it now? Also, I suspect that this will work if you build the
native code in the same source tree as the Java code. Is that not correct?


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50415/#review158565
-----------------------------------------------------------


On Dec. 10, 2016, 7:18 p.m., Srinivas Brahmaroutu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50415/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5902
>     https://issues.apache.org/jira/browse/MESOS-5902
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added config, build and install steps for the External project
> protobuf_java.
> 'make probuf_java' produces the
> ${PROTOBUF_JAVA_ROOT}/java/target/protobuf-java-2.6.1.jar
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt a340b29fc1cc8b68bf74bfc0c9c2274f0af84eee 
> 
> Diff: https://reviews.apache.org/r/50415/diff/
> 
> 
> Testing
> -------
> 
> cmake && make protobuf-2.6.1-java 
> check if 3rdparty/protobuf-2.6.1/src/protobuf-2.6.1-java/java/target/protobuf-java-2.6.1.jar
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message