gtest packages without Combine feature

[piponazo]

Description of Problem, Request, or Question

I am trying to move my project to use your gtest recipe but I found a issue with the Windows/MSVC15 package. The ::testing::Combine functionality is not available, and I think it is because you have these lines in the recipe:

        if self.settings.compiler == "Visual Studio" and float(str(self.settings.compiler.version)) >= 15:
            self.cpp_info.defines.append("GTEST_LANG_CXX11=1")
            self.cpp_info.defines.append("GTEST_HAS_TR1_TUPLE=0")

Then, the internal gtest-port.h code has:

// Determines whether to support Combine(). This only makes sense when
// value-parameterized tests are enabled.  The implementation doesn't
// work on Sun Studio since it doesn't understand templated conversion
// operators.
#if GTEST_HAS_PARAM_TEST && GTEST_HAS_TR1_TUPLE && !defined(__SUNPRO_CC)
# define GTEST_HAS_COMBINE 1
#endif

And finally the feature is not available. Until now I have been using a customized gtest recipe without defining GTEST_HAS_TR1_TUPLE and it has been working properly with Windows/MSVC15. Why do you need to define such define?

Package Details (Include if Applicable)

• Package Name/Version: gtest/1.8.0
• Operating System: Windows 10
• Compiler+version: MSVC 15.3.2

Steps to reproduce (Include if Applicable)

If you try to use ::testing::Combine in your test_package, you should see the problem.

[uilianries]

@mropert any idea?

[uilianries]

According the log there was some problem to build using Visual Studio 2017.

[piponazo]

I have been checking the internal gtest code and it might be a bug in their side. It seems that they have several ways to indicate that tuple is available:

GTEST_HAS_STD_TUPLE
GTEST_HAS_TR1_TUPLE
GTEST_USE_OWN_TR1_TUPLE
# and others

But later, they only check this (in 1.8.0):

#if GTEST_HAS_PARAM_TEST && GTEST_HAS_TR1_TUPLE && !defined(__SUNPRO_CC)
# define GTEST_HAS_COMBINE 1
#endif

I actually checked this out in their master branch, and now the code is different:

// Determines whether to support Combine(). This only makes sense when
// value-parameterized tests are enabled.  The implementation doesn't
// work on Sun Studio since it doesn't understand templated conversion
// operators.
#if (GTEST_HAS_TR1_TUPLE || GTEST_HAS_STD_TUPLE_) && !defined(__SUNPRO_CC)
# define GTEST_HAS_COMBINE 1
#endif

So, I think it is a existing bug in 1.8.0, that should be fixed in 1.9.0. Feel free to close the issue if you think you do not need to do something about this.

[mropert]

I would agree with @piponazo's analysis. The defines are there to disable the TR1 impl and use the C++11 std:: one because GTest is unable to detect that Visual Studio now supports the standard properly. It seems like they missed something...

In the meantime I'm afraid the only option is to create a package for a particular git commit on the master branch :(

[piponazo]

Apparently they will change the release policy in the next months:

https://github.com/google/googletest/issues/1079 https://github.com/google/googletest/issues/1366

[SSE4]

@solvingj @uilianries so, what should we do? options I see:

1. do nothing and close
2. add patch with fix to our 1.8.0 package
3. build package from the specific revision
4. wait for the next gtest release and keep issue open

[uilianries]

I think that we could patch version 1.8.0

Build a package for specific revision seems like workaround. And if we wait for next months until the next release, we could forget to update this recipe.

WDYT?

[piponazo]

Hi guys,

Just a comment about this. At Pix4D we were using our own version of the recipe, and we were not using at all the definitions you added:

if self.settings.compiler == "Visual Studio" and float(str(self.settings.compiler.version)) >= 15:
            self.cpp_info.defines.append("GTEST_LANG_CXX11=1")
            self.cpp_info.defines.append("GTEST_HAS_TR1_TUPLE=0")

I think that by just removing that conditional, packages will work on Windows. Even though, the recipe will not be as precise as it is now, I think it is another option you might consider. But I think the patch approach would be more correct.

[piponazo]

Let me know if I can help here, I would be happy to contribute to this recipe :smiley:

[mropert]

@piponazo I'm pretty sure if you don't put those flags on VS 2017 you'll get warnings from tr1 usage. Unless you don't use the /std to enforce standard compliance.

[piponazo]

@piponazo I'm pretty sure if you don't put those flags on VS 2017 you'll get warnings from tr1 usage. Unless you don't use the /std to enforce standard compliance.

Right, probably we are silencing those warnings. I remember we have a wrapper to gtest.h with something like this:

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4251)
#pragma warning(disable : 4275)
#endif
#include <gmock/gmock.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif

[SSE4]

@piponazo if you're willing to contribute, could you open PR against our Google Test recipe? or at least provide a patch file against Google Test itself, which solves problems for you?

[piponazo]

PR created, let me know if the patch satisfies your guidelines ;)

[SSE4]

@piponazo changes were merged, pls give another try

[SSE4]

closing as changes were merge, @piponazo if you have any other problems with GTest package, feel free to open new issue(s).