Replacing solutions with gyp files. Moving tests for windows clients in unitt...

Replacing solutions with gyp files. Moving tests for windows clients in unittests.



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=581

[hansl_g, 2010-04-28 17:24:50]

As discussed.

[Sigurður Ásgeirsson, 2010-04-28 21:10:37]

The .vsprops files are all gone from trunk chrome now, so better to pull a new
version of common.gypi and drop the .vsprops. 
I'll keep looking at the rest of the change.

http://codereview.chromium.org/1687018/diff/1/33
File DEPS (right):

http://codereview.chromium.org/1687018/diff/1/33#newcode15
DEPS:15: # We only use this file to ease the steps of generating projects after
sweet :). 
Might want to document the command line to generate the projects here. Also, I
wonder whether there are both VS 2005 and VS 2008 users to this, and how we'd go
about generating one or the other?

http://codereview.chromium.org/1687018/diff/1/33#newcode26
DEPS:26: "src/base":
we add a dependency to base?

[Sigurður Ásgeirsson, 2010-04-28 21:26:25]

The breakpad project is under a BSD license, so presumably we need to use their
license blurb on all files. 
I don't think there's a problem with using that license even to the .gypi files 
lifted from Chrome, though I may be misconstruing that.

http://codereview.chromium.org/1687018/diff/1/33
File DEPS (right):

http://codereview.chromium.org/1687018/diff/1/33#newcode7
DEPS:7: #     http://www.apache.org/licenses/LICENSE-2.0
wrong license?

http://codereview.chromium.org/1687018/diff/1/26
File src/client/windows/breakpad_client.gyp (right):

http://codereview.chromium.org/1687018/diff/1/26#newcode4
src/client/windows/breakpad_client.gyp:4: # you may not use this file except in
compliance with the License.
wrong license?

http://codereview.chromium.org/1687018/diff/1/27
File src/client/windows/crash_generation/crash_generation.gyp (right):

http://codereview.chromium.org/1687018/diff/1/27#newcode31
src/client/windows/crash_generation/crash_generation.gyp:31:
'../../../common/windows/guid_string.cc',
this is in the common library, have clients link that rather than compile this
twice (and risk link-time errors on clients using both libs).

http://codereview.chromium.org/1687018/diff/1/29
File src/client/windows/handler/exception_handler.gyp (right):

http://codereview.chromium.org/1687018/diff/1/29#newcode32
src/client/windows/handler/exception_handler.gyp:32:
"../../../common/windows/guid_string.cc",
remove here, link to the common lib.

http://codereview.chromium.org/1687018/diff/1/23
File src/client/windows/unittests/gtest.gyp (right):

http://codereview.chromium.org/1687018/diff/1/23#newcode22
src/client/windows/unittests/gtest.gyp:22: '../../../testing/gtest',
the chrome variant has only two include paths:

http://www.google.com/codesearch/p?hl=en#V-T7QAbo_LE/chrome/testing/gtest.gyp...

Also, since there's only one target here, please move this into the target.

http://codereview.chromium.org/1687018/diff/1/23#newcode38
src/client/windows/unittests/gtest.gyp:38: '../../../testing/gtest',
the chrome gtest.gyp variant has only one include dir: 

http://www.google.com/codesearch/p?hl=en#V-T7QAbo_LE/chrome/testing/gtest.gyp...

are all these necessary?

[hansl_g, 2010-04-29 19:54:19]

http://codereview.chromium.org/1687018/diff/1/33
File DEPS (right):

http://codereview.chromium.org/1687018/diff/1/33#newcode7
DEPS:7: #     http://www.apache.org/licenses/LICENSE-2.0
On 2010/04/28 21:26:25, Ruðrugis wrote:
> wrong license?

Done.

http://codereview.chromium.org/1687018/diff/1/33#newcode15
DEPS:15: # We only use this file to ease the steps of generating projects after
On 2010/04/28 21:10:37, Ruðrugis wrote:
> sweet :). 
> Might want to document the command line to generate the projects here. Also, I
> wonder whether there are both VS 2005 and VS 2008 users to this, and how we'd
go
> about generating one or the other?

GYP will generate VS2005 projects if it's installed on the machine (from what I
gathered).

http://codereview.chromium.org/1687018/diff/1/33#newcode26
DEPS:26: "src/base":
On 2010/04/28 21:10:37, Ruðrugis wrote:
> we add a dependency to base?

Done. Removed. This was used by minidump and I've removed the dependency in the
code.

http://codereview.chromium.org/1687018/diff/1/26
File src/client/windows/breakpad_client.gyp (right):

http://codereview.chromium.org/1687018/diff/1/26#newcode4
src/client/windows/breakpad_client.gyp:4: # you may not use this file except in
compliance with the License.
On 2010/04/28 21:26:25, Ruðrugis wrote:
> wrong license?

Done.

http://codereview.chromium.org/1687018/diff/1/27
File src/client/windows/crash_generation/crash_generation.gyp (right):

http://codereview.chromium.org/1687018/diff/1/27#newcode31
src/client/windows/crash_generation/crash_generation.gyp:31:
'../../../common/windows/guid_string.cc',
On 2010/04/28 21:26:25, Ruðrugis wrote:
> this is in the common library, have clients link that rather than compile this
> twice (and risk link-time errors on clients using both libs).

Done.

[Sigurður Ásgeirsson, 2010-05-03 17:40:15]

looks good, except for the following nits. 

Mark, is there any reason to leave the old project and solution files for now
(e.g. do folks in automated builds), or do you think it's fine to scrub them
now.

http://codereview.chromium.org/1687018/diff/11002/26006
File src/client/windows/unittests/gtest.gyp (right):

http://codereview.chromium.org/1687018/diff/11002/26006#newcode35
src/client/windows/unittests/gtest.gyp:35: 'include_dirs': [
move this to the 'gtest' target? 
Also, do you need all four paths here and in direct_dependent_settings? Chromium
makes do with only two paths.

http://codereview.chromium.org/1687018/diff/11002/26006#newcode54
src/client/windows/unittests/gtest.gyp:54: '<(DEPTH)/testing/gtest/include',
Chromium only exports the gtest/include path to dependents, do you need all four
here?

[hansl_g, 2010-05-03 17:50:34]

I'll wait for a final okay from Mark and you.

http://codereview.chromium.org/1687018/diff/11002/26006
File src/client/windows/unittests/gtest.gyp (right):

http://codereview.chromium.org/1687018/diff/11002/26006#newcode35
src/client/windows/unittests/gtest.gyp:35: 'include_dirs': [
On 2010/05/03 17:40:15, Ruðrugis wrote:
> move this to the 'gtest' target? 
done.

> Also, do you need all four paths here and in direct_dependent_settings?
Chromium
> makes do with only two paths.
we need two: gtest/include and testing/include (for gmock). I removed the unused
others.

http://codereview.chromium.org/1687018/diff/11002/26006#newcode54
src/client/windows/unittests/gtest.gyp:54: '<(DEPTH)/testing/gtest/include',
On 2010/05/03 17:40:15, Ruðrugis wrote:
> Chromium only exports the gtest/include path to dependents, do you need all
four
> here?
See comment above. We need two of those 4.

[Mark Mentovai, 2010-05-03 17:59:47]

siggi@chromium.org wrote:
> looks good, except for the following nits.
>
> Mark, is there any reason to leave the old project and solution files for
> now
> (e.g. do folks in automated builds), or do you think it's fine to scrub them
> now.

I think we can chuck ’em now. If the complaint level is too high, we
can resurrect them from svn history pretty easily and call them
deprecated.

Mark

[Sigurður Ásgeirsson, 2010-05-03 18:14:48]

lgtm