> On July 31, 2018, 10:27 a.m., Benjamin Bannier wrote:
> > LGTM. I reopened https://reviews.apache.org/r/67916/#comment289263, using
> > the original upstream patch would be great if possible.
>
> Andrew Schwartzmeyer wrote:
> Ben, this did use the oiginal upsteam patch. I cloned the repo, checked
> out the tag we bundle, and cherry-picked the upstream patch from master, then
> created a patch file from the applied diff. The commit in master does not
> apply to 1.8.0 perfectly, so I can't just turn it into a patch, it had to be
> applied and conflicts fixed. I'm closing the issue again, because this is the
> original upstream patch, or as close as possible as we can get.
>
> If you have another way to do it, I'm all ears!
Sorry for possibly being very dense. I did this
# Applied your patch.
mesos [master?] % ./support/apply-reviews.py -r 67916 -c -n -s
# <SNIP>
# Extracted the _original_ upstream patch.
googletest [master?] % git format-patch
f66ab00704cd47e4e63ef6d425ca14b9192aaebb~1..f66ab00704cd47e4e63ef6d425ca14b9192aaebb
0001-Upstream-cl-199129756.patch
# Use the _original_ upstream patch in Mesos.
mesos [master?] % mv ~/src/googletest/0001-Upstream-cl-199129756.patch
3rdparty/googletest-release-1.8.0.patch
# `patch`, at least on macos-10.13.6, is able to automatically apply this
patch.
mesos [master??] % ninja -C_ 3rdparty/googletest-1.8.0
# <SNIP>
[1/5] Performing patch step for 'googletest-1.8.0'
patching file googletest/include/gtest/gtest-printers.h
Hunk #1 succeeded at 581 with fuzz 2 (offset -55 lines).
patching file googletest/test/gtest-printers_test.cc
Hunk #1 succeeded at 1111 (offset -4 lines).
# <SNIP>
Does this not work on e.g., Windows?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67916/#review206647
-----------------------------------------------------------
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
>
>
