> On Nov. 17, 2016, 2:48 p.m., Zameer Manji wrote: > > Seems like a lot of patches to enable this behaviour. I'm not opposed but > > it seems risky.
You find everything risky! > On Nov. 17, 2016, 2:48 p.m., Zameer Manji wrote: > > api/src/main/thrift/org/apache/aurora/gen/BUILD, line 34 > > <https://reviews.apache.org/r/53836/diff/1/?file=1565790#file1565790line34> > > > > Can't we have our own 3rdparty thrift target that includes the thrift > > lib and this prepare binary? Then we can put the dep in one place. I can, but this requires a custom pants plugin (housed as a loose python sourcefile in the repo) to expose, say, an `aurora_py_thrift_lib` target. Your comments here expressed reservations about that: https://reviews.apache.org/r/40219/ If you re-ack you're in-fact OK with a custom plugin for this, I'll whip up diff 2. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53836/#review156248 ----------------------------------------------------------- On Nov. 16, 2016, 9:46 p.m., John Sirois wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53836/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2016, 9:46 p.m.) > > > Review request for Aurora, Joshua Cohen, Stephan Erb, and Zameer Manji. > > > Repository: aurora > > > Description > ------- > > This required an upgrade to the latest pants dev release to correct > an issue with the setup.py binary packager we use to generate sdists. > > This is for sanity sake, and, once the TODO in > `build-support/thrift/prepare_binary.sh` is addressed, it will also > allow pants-patch-free addition of new platforms (thinking ARM). > > api/src/main/thrift/org/apache/aurora/gen/BUILD | 3 > +++ > api/src/main/thrift/org/apache/thermos/BUILD | 1 + > build-support/thrift/.gitignore | 1 + > build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch | 42 > +++++++++++++------------------ > build-support/thrift/AURORA-1727.lib.py.setup.py.patch | 45 > +++++++++++++++++++++++++++++++++ > {api/src/main/thrift/org/apache/thermos => build-support/thrift}/BUILD | 18 > ++++--------- > build-support/thrift/Makefile | 2 > +- > build-support/thrift/prepare_binary.sh | 68 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > build-support/thrift/thriftw | 12 > +++++++-- > pants.ini | 7 > +++++- > 10 files changed, 158 insertions(+), 41 deletions(-) > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/BUILD > f4004a3ca36078c6d24991db4beb68903a05652c > api/src/main/thrift/org/apache/thermos/BUILD > cb655a2fd35d22f7d7e80c4311742bad763d8614 > build-support/thrift/.gitignore 9a3adb69210ba3dbf3c1f408895561da37e8f4c3 > build-support/thrift/AURORA-1727.compiler.cpp.Makefile.in.patch > b69e3fef137cd73c6f2b73201463a0705ef8082a > build-support/thrift/AURORA-1727.lib.py.setup.py.patch PRE-CREATION > build-support/thrift/BUILD PRE-CREATION > build-support/thrift/Makefile 48b174ad622288d2738a5fa37bbb72385fcc3a27 > build-support/thrift/prepare_binary.sh PRE-CREATION > build-support/thrift/thriftw 50d6dfdeb16ca8bf14aaff7aa826e3d69c5e13f0 > pants.ini cecdb277f327f77b2652f76a30fc8d4ffd9ff1db > > Diff: https://reviews.apache.org/r/53836/diff/ > > > Testing > ------- > > Locally green: > ``` > rm -rf ~/.cache/pants .cache/ > git clean -fdx build-support/thrift > ./pants clean-all > ./build-support/jenkins/build.sh > > vagrant ssh --command "rm -rf ~/.cache/pants && cd aurora && rm -rf .cache/ > && ./pants clean-all" > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > ./build-support/python/make-pycharm-virtualenv > ``` > > > Thanks, > > John Sirois > >
