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 40851: Windows:[1/2] Updated zookeeper-3.4.5.patch to fix VS2015 build.
Date Wed, 27 Jan 2016 10:12:20 GMT


> On Jan. 27, 2016, 3 a.m., Alex Clemmer wrote:
> > Aside from the relatively minor comments below, I have one major suggestion: I'd
like to point Windows builds at ZK commit 06d3f3fa1bff258e62c0670309ad1849b1434bb1[1], (or
_some_ commit later in the version history) so that we can transition as much away from patching
the ASM and .c files as we can. I'll volunteer to prepare a candidate fix for this patch tonight
so we have something to complete the review tomorrow.
> > 
> > My justification follows:
> > 
> > (1) At some point, when the diff between the C/ASM source in the supported version
and the version that compiles on Windows becomes big enough, it stops being worth it to maintain
a .patch file instead of upgrading the ZK version. In this particular case, I am not comfortable
replacing that ASM code with that function call without a code review from the ZK team, and
since there is a competing implementation in the master branch (see issue in the tracker[2]
and the `fetch_and_add` function in the relevant source file[3]) I propose we just upgrade
the Windows build to point at a version of ZK with this patch.
> > (2) We already point to commit hashes for other third party projects, like glog,
so this is not a solution without precedent.
> > 
> > My next steps are the following:
> > 
> > * We'll probably need to keep the patch file, since we still need .vcxproj and .sln
files that build on VS2015.
> > * The current build solution basically unpacks a tarball from the `3rdparty` directory
that has the Jute files pre-generated; we'll need to make another tarball that the Windows
can download that also has the jute files generated.
> > 
> > Let me know of other thoughts or issues; otherwise I'll see you on the other side
with a (god willing) working version of this patch.
> > 
> > [1] https://github.com/apache/zookeeper/commit/06d3f3fa1bff258e62c0670309ad1849b1434bb1
> > [2] https://issues.apache.org/jira/browse/ZOOKEEPER-1635
> > [3] https://github.com/apache/zookeeper/blob/trunk/src/c/src/mt_adaptor.c

I happy to say that we have a working cadidate fix for these changes. They're in two temp
commits: https://github.com/hausdorff/mesos/commit/3ed2626c11ddf45108d2a9711131cce80bd85dfb
https://github.com/hausdorff/mesos/commit/945b239de3af4f4a9fc1b36c88d70f9913f1cda1 I'll work
with Lawindi tomorrow to formulate them into cohesive patches as part of these two ZK patches
he's made.


- Alex


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


On Jan. 26, 2016, 9:03 p.m., M Lawindi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40851/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2016, 9:03 p.m.)
> 
> 
> Review request for Alex Naparu, Dario Bazan, Daniel Pravat, Alex Clemmer, and M Lawindi.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows:[1/2] Updated zookeeper-3.4.5.patch to fix VS2015 build.
> 
> 
> Diffs
> -----
> 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/40851/diff/
> 
> 
> Testing
> -------
> 
> - Applied patch to original zookeeper 3.4.5
> - Successfully opened 2015 solution in VS2015
> - Cli and Zookeeper build successfully
> 
> 
> Thanks,
> 
> M Lawindi
> 
>


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