Re: Review Request 62590: WIP: Update to Thrift 0.10.0

[Bill Farner <wf...@apache.org>]

> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote:
> > ```
> > /bin/sh: cmake: command not found
> > ```
> > 
> > But now i need to install cmake, so i'm not sure this pays off.
> 
> Bill Farner wrote:
>     (this = the switch to cmake)
> 
> Stephan Erb wrote:
>     Bison on MacOs is 10 years old. I assumed they have a good reason for it and considered an added dependency a less risky endeavor. (Or is it just negligence?)
> 
> Bill Farner wrote:
>     What i meant to say - currently, i can build aurora on a stock macOS machine with only a modern JDK and xcode command line utilities.  Thrift 0.10.0 seems to put us in the position of choosing between:
>     
>     a.) adding another step to our bootstrap routine to pre-build bison
>     b.) adding bison as a build-time dependency
>     c.) adding cmake as a build-time dependency
>     d.) other options? (i'd like to float the idea of hosting thrift binaries like how pants does)
> 
> Stephan Erb wrote:
>     Giving it some thought, you are right that just requring cmake does not improve the situation in any way.
>     
>     I also like that we can build Aurora on a stock MacOS without much hassle (or administrator rights). I think we should retain this property. This would restrict us to options a) and d).
> 
> Bill Farner wrote:
>     I would support (d).  We can place unofficial binaries in svn for dev platforms as needed, and could support using thrift from the `PATH` as a fallback.  This has the bonus of making from-scratch builds much faster.
> 
> Stephan Erb wrote:
>     I saw your patch to https://github.com/morimekta/providence. Did you manage to get something working with it?
> 
> Bill Farner wrote:
>     I'm about 20% along.  A very large mechanical patch is needed for Aurora.  The only untested piece is binary format compatibility (which providence aims to achieve).  There is some incompatibility i need to investigate further (trivial details like a round-tripped `null` collection turning into an empty collection), but so far it looks good!  I'm proceeding with high confidence, as there is also the future promise of a more approachable HTTP/JSON interface via thrift IDL when using providence.

I should have clarified - i don't think we should hold back on upgrading thrift.  It's not yet guaranteed that the migration to providence will be successful or timely.

As for my proposed (d) above, we could store prebuilt binaries similarly to how we store mesos eggs: https://svn.apache.org/repos/asf/aurora/3rdparty/


- Bill


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


On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62590/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2017, 1:50 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Update to Thrift 0.10.0 (https://github.com/apache/thrift/blob/master/CHANGES)
> 
> Included changes:
> 
> * Switch to cmake rather than building with make. The cmake build is more lenient with its build requirments. We would have needed to upgrade the system bison on MacOS otherwise.
> * The Java `hashcode` option has been removed as it is now the default.
> 
> 
> ToDo / Questions:
> 
> * Rebuild the Aurora image to include Thrift 0.10. Until this is done, one needs to manually run `sudo apt-get install flex bison cmake build-essential` within the vagrant box to ensure Thrift can be bootstrapped.
> * The Gradle workaround looks strange. If it is really required we should extract it into a separate patch.
> * The Pants build is complaining about the usage of `new_style` option. See if we can get to work around it.
> * Do we want to maintain some patches to ensure we are just building the required Thrift subset? Is this worth the effort?
> 
> 
> Diffs
> -----
> 
>   3rdparty/python/requirements.txt 0c04b98e791c5de1985724304f384e8dbade8c19 
>   build-support/packer/build.sh 7b7914c11e376a5f7ba7a5ee69fa0962e9bca379 
>   build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch 77c966caa3d1f644241bcc2b1968bc9306c56689 
>   build-support/thrift/AURORA-1727.compiler.cpp.src.generate.t_java_generator.cc.patch 42300b43a8f72e45c96b975e5d3a6a7bd0283529 
>   build-support/thrift/AURORA-1727.lib.py.setup.py.patch 11c7b13341e156f3686511cb40ab13c1256203a6 
>   build-support/thrift/Makefile f440b610afe321af663e393a29eebda7af7bd7a8 
>   build.gradle 851bab52fd389bc9577a6938f646bb29bae06139 
>   buildSrc/src/main/groovy/org/apache/aurora/build/ThriftPlugin.groovy fc2bc9082dae2c63aa578c05dc89feb346260a67 
>   gradle.properties PRE-CREATION 
>   pants.ini f25e1d1a50f457325dbbabf327c31d760cb90339 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/Util.java c28fb65010af5e3db925487929d4e0e12b4101a4 
> 
> 
> Diff: https://reviews.apache.org/r/62590/diff/1/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>