> On April 18, 2016, 8:11 p.m., Vinod Kone wrote: > > Looks good to me. Is there already a bug reported for this in the protobuf > > project? If not, can you create one and link it here? > > > > Also, this review is incomplete. There has to be corresponding changes in > > the Makefiles and CMake files to apply this patch. Look how other patches > > are applied for reference. > > Benjamin Bannier wrote: > The patch is extracted from the upstream repo, but I couldn't find the > original issue leading to the patch fix. I updated the description to reflect > where the patch came from though. > > > https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/Makefile.am#L80 > > Benjamin Bannier wrote: > Ups, mutilated comment. > > I updated the cmake config to apply the patch as well, even though the > cmake build always succeeded for me (might be either a problem on my side > setting up the compiler to use, or a problem on the cmake side passing > `CMAKE_C_COMPILER` and `CMAKE_CXX_COMPILER` through to the protobuf build > which is autotools-based). > > The autotools setup does not need to be updated as it automagically will > apply any `$NAME.patch` file if `$NAME` matches a 3rdparty library being > built, > > > https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/Makefile.am#L80 > > Vinod Kone wrote: > You still need to add the patch to the distribution, otherwise someone > who downloads the source tar ball (relased after make distcheck) will not > have the patch. > > See > https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/Makefile.am#L54
Done. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129384 ----------------------------------------------------------- On April 22, 2016, 7:58 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > ----------------------------------------------------------- > > (Updated April 22, 2016, 7:58 a.m.) > > > Review request for mesos, Zhiwei Chen and Vinod Kone. > > > Bugs: MESOS-4678 > https://issues.apache.org/jira/browse/MESOS-4678 > > > Repository: mesos > > > Description > ------- > > This adds the upstream patch `717f807` which is contained in > `protobuf-3.0.0-alpha-1`. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/CMakeLists.txt > 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 > 3rdparty/libprocess/3rdparty/Makefile.am > 895663b0095cfb1e682da90606c34a96ad036834 > 3rdparty/libprocess/3rdparty/protobuf-2.6.1.patch PRE-CREATION > > Diff: https://reviews.apache.org/r/46314/diff/ > > > Testing > ------- > > Building with GCC6 w/o this patch leads to a hard failure to a comparison > between a signed and unsigned types; with this patch the build succeeds. > > > Thanks, > > Benjamin Bannier > >
