mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 67916: Patched Google Test with upstream bugfix.
Date Tue, 31 Jul 2018 16:42:14 GMT


> On July 31, 2018, 1: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.

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!


- Andrew


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


On July 30, 2018, 11:50 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67916/
> -----------------------------------------------------------
> 
> (Updated July 30, 2018, 11:50 a.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
> 
>


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