MSVC 2010 (10.0) TR1 tuple implementation not detected by the current version of gtest

MSVC 2010 (10.0) already provides TR1 tuple implementation.  Unfortunately, the
current version of gtest that is used in this project does not properly detect
and handle MSVC 2010 which results in build breaks.  The following is a
temporary workaround until the next gtest upgrade.  Another issue is that the
current version of gyp used in this project does not detect MSVC 2010, so the
following workaround only works when a later version of gyp is used.

Thanks,
-Ivan

[Ivan Penkov]

.

[hansl]

I haven't had any implication in breakpad for at least a year, and moved to
a Google3 team. I don't think I'm the right reviewer for this.


On Thu, Aug 16, 2012 at 12:58 AM, <ivan.penkov@gmail.com> wrote:

> Reviewers: Mark Mentovai, hansl, mihaip,
>
> Message:
> .
>
> Description:
> MSVC 2010 (10.0) already provides TR1 tuple implementation.
> Unfortunately, the current version of gtest that is used in this project
> does not properly detect and handle MSVC 2010 which results in build
> breaks.  The following is a temporary workaround until the next gtest
> upgrade.  Another issue is that the current version of gyp used in this
> project does not detect MSVC 2010, so the following workaround only
> works when a later version of gyp is used.
>
> Thanks,
> -Ivan
>
> Please review this at
http://breakpad.appspot.com/**428003/<http://breakpad.appspot.com/428003/>
>
> Affected files:
>   M     client/windows/build/common.**gypi
>
>
> Index: client/windows/build/common.**gypi
> ==============================**==============================**=======
> --- client/windows/build/common.**gypi    (revision 1013)
> +++ client/windows/build/common.**gypi    (working copy)
> @@ -342,6 +342,18 @@
>            },{
>              'msvs_large_module_debug_link_**mode%': '2',  # Yes
>            }],
> +          # MSVC 2010 (10.0) provides TR1 tuple implementation.
>  Unfortunately
> +          # the current version of gtest that is used in this project
> does not
> +          # properly detect and handle MSVC 2010 which results in build
> breaks.
> +          # The following is a temporary workaround until the next gtest
> +          # upgrade.  Another issue is that the current version of gyp
> used in
> +          # this project does not detect MSVC 2010, so the following
> workaround
> +          # only works when a later version of gyp is used.
> +          ['MSVS_VERSION>="2010"', {
> +            'msvs_tr1_defines%': 'GTEST_USE_OWN_TR1_TUPLE=0',
> +          },{
> +            'msvs_tr1_defines%': '_HAS_TR1=0',
> +          }],
>          ],
>          'nacl_win64_defines': [
>            # This flag is used to minimize dependencies when building
> @@ -1176,7 +1188,7 @@
>            'CERT_CHAIN_PARA_HAS_EXTRA_**FIELDS',
>            'WIN32_LEAN_AND_MEAN',
>            '_SECURE_ATL',
> -          '_HAS_TR1=0',
> +          '<@(msvs_tr1_defines)',
>          ],
>          'msvs_system_include_dirs': [
>            '<(DEPTH)/third_party/**platformsdk_win7/files/**Include',
>
>
>

[Mark Mentovai]

Why don’t you just upgrade gtest? Nobody’s going to remember to back this change
out when gtest is ultimately upgraded to a version that doesn’t need this,
unless you someone does it as the very next thing.

[Ivan Penkov]

On 2012/08/16 12:37:45, Mark Mentovai wrote:
> Why don’t you just upgrade gtest? Nobody’s going to remember to back this
change
> out when gtest is ultimately upgraded to a version that doesn’t need this,
> unless you someone does it as the very next thing.

Just to clarify.  Even if we upgrade to the latest gtest, the change I'm
proposing is not expected to cause any problems.  My change basically bypasses
the detection code in the gtest headers.  Since, it is safe to assume that
Microsoft won't remove the TR1 tuple implementation post MSVC 2010, gtest
upgrade (where we'll get the proper fix in the gtest headers) won't break
anything.

[Mark Mentovai]

Won't be necessary if we upgrade gtest either, right?

We should upgrade it. I'm told that it will also eliminate the need for
some Android-specific hacks as well.
On Aug 16, 2012 9:51 PM, <ivan.penkov@gmail.com> wrote:

> On 2012/08/16 12:37:45, Mark Mentovai wrote:
>
>> Why don’t you just upgrade gtest? Nobody’s going to remember to back
>>
> this change
>
>> out when gtest is ultimately upgraded to a version that doesn’t need
>>
> this,
>
>> unless you someone does it as the very next thing.
>>
>
> Just to clarify.  Even if we upgrade to the latest gtest, the change I'm
> proposing is not expected to cause any problems.  My change basically
> bypasses the detection code in the gtest headers.  Since, it is safe to
> assume that Microsoft won't remove the TR1 tuple implementation post
> MSVC 2010, gtest upgrade (where we'll get the proper fix in the gtest
> headers) won't break anything.
>
> http://breakpad.appspot.com/**428003/<http://breakpad.appspot.com/428003/>
>

[Ivan Penkov]

Yes, upgrading gtest should resolve this issue as well.  Mark, if you can
give me some pointers I can follow up with the gtest upgrade.

Thanks,
-Ivan

On Thu, Aug 16, 2012 at 7:04 PM, Mark Mentovai <mark@chromium.org> wrote:

> Won't be necessary if we upgrade gtest either, right?
>
> We should upgrade it. I'm told that it will also eliminate the need for
> some Android-specific hacks as well.
> On Aug 16, 2012 9:51 PM, <ivan.penkov@gmail.com> wrote:
>
>> On 2012/08/16 12:37:45, Mark Mentovai wrote:
>>
>>> Why don’t you just upgrade gtest? Nobody’s going to remember to back
>>>
>> this change
>>
>>> out when gtest is ultimately upgraded to a version that doesn’t need
>>>
>> this,
>>
>>> unless you someone does it as the very next thing.
>>>
>>
>> Just to clarify.  Even if we upgrade to the latest gtest, the change I'm
>> proposing is not expected to cause any problems.  My change basically
>> bypasses the detection code in the gtest headers.  Since, it is safe to
>> assume that Microsoft won't remove the TR1 tuple implementation post
>> MSVC 2010, gtest upgrade (where we'll get the proper fix in the gtest
>> headers) won't break anything.
>>
>> http://breakpad.appspot.com/**428003/<http://breakpad.appspot.com/428003/>
>>
>

[Mark Mentovai]

The contents of src/testing are managed by an svn:externals definition in src:

mark@cougar bash$ svn propget svn:externals src
testing -r175 http://googlemock.googlecode.com/svn/trunk

meaning that src/testing is pinned to r175 of the gmock trunk. In turn, the
gmock trunk at r175 depends on the gtest trunk at r267 in its gtest directory,
so it shows up in Breakpad at src/testing/gtest. We don’t need to worry about
the version of gtest, because we just bring in a gmock version that we want, and
it brings in a compatible gtest for us.

You can point Breakpad at a different version of gmock by editing the
svn:externals property:

mark@cougar bash$ svn propedit svn:externals src

Then, when you “svn update,” svn will check out the new version you specified.
All you need to do then is make sure that Breakpad is compatible with the new
version, fixing things as needed. When you’re done with that, it should just be
a normal “gcl” code review, but Rietveld doesn’t handle property changes so
well, so you may need to mail an “svn diff” to the list for review.