----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67916/#review206647 -----------------------------------------------------------
Ship it! LGTM. I reopened https://reviews.apache.org/r/67916/#comment289263, using the original upstream patch would be great if possible. - Benjamin Bannier On July 30, 2018, 8:50 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67916/ > ----------------------------------------------------------- > > (Updated July 30, 2018, 8:50 p.m.) > > > Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff. > > > Bugs: MESOS-8990 > https://issues.apache.org/jira/browse/MESOS-8990 > > > Repository: mesos > > > Description > ------- > > Per MESOS-8990, our Google Test dependency needs a patch from > upstream, https://github.com/google/googletest/pull/1620, in order to > continue building with the next version of MSVC (and potentially other > compilers). > > This patch file was generated by cherry-picking `f66ab00` from > `master` onto `release-1.8.0` in the Google Test repo, and resolving > the merge conflict. > > Additionally, we need to define `GTEST_LANG_CXX11=1` when including > the GoogleTest headers such that the patch is used. > > > Diffs > ----- > > 3rdparty/CMakeLists.txt d8d113c17d124b763659dcc5ace9066499de6cd4 > 3rdparty/googletest-release-1.8.0.patch PRE-CREATION > > > Diff: https://reviews.apache.org/r/67916/diff/2/ > > > Testing > ------- > > Repro'ed MESOS-8990 on Windows with Visual Studio Preview, and the following > (temporary) CMake changes: > > ``` > # Set the default standard to C++11 for all targets. > -set(CMAKE_CXX_STANDARD 11) > -set(CMAKE_CXX_STANDARD_REQUIRED ON) > +# set(CMAKE_CXX_STANDARD 11) > +# set(CMAKE_CXX_STANDARD_REQUIRED ON) > # Do not use, for example, `-std=gnu++11`. > -set(CMAKE_CXX_EXTENSIONS OFF) > +# set(CMAKE_CXX_EXTENSIONS OFF) > +add_compile_options(/std:c++latest) > +add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING) > +add_definitions(-D_HAS_AUTO_PTR_ETC=1) > +add_definitions(-D_HAS_TR1_NAMESPACE=1) > +add_definitions(-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING) > +add_definitions(-D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING) > ``` > > After adding the necessary compile interface definition, `ninja tests` > successfully built on Windows with this patch applied, after not building > before the patch. > > > Thanks, > > Andrew Schwartzmeyer > >
