Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review130171 --- Ship it! Ship It! - Vinod Kone On April 22, 2016, 5: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, 5: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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review130062 --- Patch looks great! Reviews applied: [46314] Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On April 22, 2016, 5: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, 5: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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
> 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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- 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. Changes --- Added patch to distribution. Bugs: MESOS-4678 https://issues.apache.org/jira/browse/MESOS-4678 Repository: mesos Description (updated) --- This adds the upstream patch `717f807` which is contained in `protobuf-3.0.0-alpha-1`. Diffs (updated) - 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
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
> On April 18, 2016, 6: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 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 - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129384 --- On April 19, 2016, 2:46 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 19, 2016, 2:46 p.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/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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/ --- (Updated April 19, 2016, 4:46 p.m.) Review request for mesos, Zhiwei Chen and Vinod Kone. Changes --- Made the commit message wrappederer. Bugs: MESOS-4678 https://issues.apache.org/jira/browse/MESOS-4678 Repository: mesos Description (updated) --- 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/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
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129535 --- Bad patch! Reviews applied: [46314] Failed command: ./support/apply-review.sh -n -r 46314 Error: 2016-04-19 14:42:22 URL:https://reviews.apache.org/r/46314/diff/raw/ [1890/1890] -> "46314.patch" [1] No files to lint Error: No line in the commit message summary may exceed 72 characters. Full log: https://builds.apache.org/job/mesos-reviewbot/12611/console - Mesos ReviewBot On April 19, 2016, 9:32 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 19, 2016, 9:32 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/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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
> 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 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 - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129384 --- On April 19, 2016, 11:32 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 19, 2016, 11:32 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/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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
> 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. 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 --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129384 --- On April 19, 2016, 11:32 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 19, 2016, 11:32 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/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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/ --- (Updated April 19, 2016, 11:32 a.m.) Review request for mesos, Zhiwei Chen and Vinod Kone. Changes --- Updated CMakeLists.txt to apply protobuf patch. Bugs: MESOS-4678 https://issues.apache.org/jira/browse/MESOS-4678 Repository: mesos Description (updated) --- This adds the upstream patch `717f807` which is contained in `protobuf-3.0.0-alpha-1`. Diffs (updated) - 3rdparty/libprocess/3rdparty/CMakeLists.txt 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 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
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129384 --- 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. - Vinod Kone On April 18, 2016, 8:06 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 18, 2016, 8:06 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 > --- > > See summary. > > > Diffs > - > > 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 > >
Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/#review129311 --- Patch looks great! Reviews applied: [46314] Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On April 18, 2016, 8:06 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46314/ > --- > > (Updated April 18, 2016, 8:06 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 > --- > > See summary. > > > Diffs > - > > 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 > >
Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46314/ --- Review request for mesos, Zhiwei Chen and Vinod Kone. Bugs: MESOS-4678 https://issues.apache.org/jira/browse/MESOS-4678 Repository: mesos Description --- See summary. Diffs - 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
