Create msvs_large_pdb workaround.

Create msvs_large_pdb workaround.

This creates a workaround for targets whose PDB sizes exceeds 1GB, using the same trick originally implemented here:

https://codereview.chromium.org/11968015/

There are currently 4 targets using this, and another 4 targets that need to use it hence the desire to centralize this functionality.

BUG=174136
Committed: https://code.google.com/p/gyp/source/detail?r=1600

[chrisha, 2013-03-05 20:52:25]

PTAL.

I'm not expecting a detailed review here yet, simply a quick flyby to see if the
approach is sane.

Please include anybody else you think may be worthwhile to have on this review.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:179: shim_dict['libraries'] = []
I'd love comments as to the sanity of the two deepcopies here. Is there a
description somewhere of what properties are the minimum that need to be set?
Are there are other optional properties I'm not resetting that could cause
problems?

(Excuse my non-familiarity with GYP's internals.)

[scottmg, 2013-03-06 19:03:34]

it'd be nice to do it at a lower level instead (i.e. just insert a way to
generate a stub 4k pdb as part of the link step in win_tool.py if the pdb's not
there) but i guess that won't work in non-ninja builds. hrmm..

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:125: if d['type'] not in ['executable', 'shared_library']:
is all of this checking necessary? can't we just put msvs_large_pdb only on
things that actually need it?

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:150: # we can't always build a relative path to the shim
source file, and Ninja
sorry, why can't we build a relative path?

if it's not possible, since the file doesn't need anything in it, i think it'd
be preferable to just output an empty .cc in the intermediate dir at generation
time.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:179: shim_dict['libraries'] = []
On 2013/03/05 20:52:25, chrisha wrote:
> I'd love comments as to the sanity of the two deepcopies here. Is there a
> description somewhere of what properties are the minimum that need to be set?
> Are there are other optional properties I'm not resetting that could cause
> problems?
> 
> (Excuse my non-familiarity with GYP's internals.)

i'm afraid i don't know the answer to this question. this looks a bit scary
though... i'd be inclined to try to model it after what's in input.py rather
than deepcopy'ing.

https://codereview.chromium.org/12476002/diff/3001/test/win/large-pdb/large-p...
File test/win/large-pdb/large-pdb.gyp (right):

https://codereview.chromium.org/12476002/diff/3001/test/win/large-pdb/large-p...
test/win/large-pdb/large-pdb.gyp:9: 'type': 'executable',
please add a dll test too

[Evan Martin, 2013-03-06 20:43:39]

https://codereview.chromium.org/12476002/diff/3001/data/win/large-pdb-shim.cc
File data/win/large-pdb-shim.cc (right):

https://codereview.chromium.org/12476002/diff/3001/data/win/large-pdb-shim.cc...
data/win/large-pdb-shim.cc:3: // found in the LICENSE file.
Should use the gyp license header, not the Chrome one.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:15: 
Nit: rather than put all of these variables at module scope, might be better to
def _GetPathToLargePDBShim() that contains this logic.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:125: if d['type'] not in ['executable', 'shared_library']:
On 2013/03/06 19:03:34, scottmg wrote:
> is all of this checking necessary? can't we just put msvs_large_pdb only on
> things that actually need it?

I think it's better to fail (or at least warn) loudly than silently ignore it,
too.

[chrisha, 2013-03-14 15:57:23]

I've cleaned this up to address the review comments.

I prefer not to do this 'lower' because I also want it to work for generated
MSVS solution files.

I agree that avoiding the copy from the GYP tree to the shared intermediate
directory would be nice but this currently happens as a 'transform' on the
target tree, before going in to the generator. Thus, we don't know what
directories will be used and they may not even be created. It would require
delving into generator internals to get around this.

PTAnotherL?

https://codereview.chromium.org/12476002/diff/3001/data/win/large-pdb-shim.cc
File data/win/large-pdb-shim.cc (right):

https://codereview.chromium.org/12476002/diff/3001/data/win/large-pdb-shim.cc...
data/win/large-pdb-shim.cc:3: // found in the LICENSE file.
On 2013/03/06 20:43:39, Evan Martin wrote:
> Should use the gyp license header, not the Chrome one.

Done.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:15: 
On 2013/03/06 20:43:39, Evan Martin wrote:
> Nit: rather than put all of these variables at module scope, might be better
to
> def _GetPathToLargePDBShim() that contains this logic.

Done.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:125: if d['type'] not in ['executable', 'shared_library']:
On 2013/03/06 19:03:34, scottmg wrote:
> is all of this checking necessary? can't we just put msvs_large_pdb only on
> things that actually need it?

Done.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:150: # we can't always build a relative path to the shim
source file, and Ninja
On 2013/03/06 19:03:34, scottmg wrote:
> sorry, why can't we build a relative path?
> 
> if it's not possible, since the file doesn't need anything in it, i think it'd
> be preferable to just output an empty .cc in the intermediate dir at
generation
> time.

Because on windows your gyp install might be on a different drive from the
project in question, unfortunately. I'd prefer to simply create the file at
generation time, but are we guaranteed that the output directories exist? This
bit of code seems to run before that is the case.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:179: shim_dict['libraries'] = []
On 2013/03/06 19:03:34, scottmg wrote:
> On 2013/03/05 20:52:25, chrisha wrote:
> > I'd love comments as to the sanity of the two deepcopies here. Is there a
> > description somewhere of what properties are the minimum that need to be
set?
> > Are there are other optional properties I'm not resetting that could cause
> > problems?
> > 
> > (Excuse my non-familiarity with GYP's internals.)
> 
> i'm afraid i don't know the answer to this question. this looks a bit scary
> though... i'd be inclined to try to model it after what's in input.py rather
> than deepcopy'ing.

Done.

https://codereview.chromium.org/12476002/diff/3001/pylib/gyp/MSVSUtil.py#newc...
pylib/gyp/MSVSUtil.py:179: shim_dict['libraries'] = []
On 2013/03/06 19:03:34, scottmg wrote:
> On 2013/03/05 20:52:25, chrisha wrote:
> > I'd love comments as to the sanity of the two deepcopies here. Is there a
> > description somewhere of what properties are the minimum that need to be
set?
> > Are there are other optional properties I'm not resetting that could cause
> > problems?
> > 
> > (Excuse my non-familiarity with GYP's internals.)
> 
> i'm afraid i don't know the answer to this question. this looks a bit scary
> though... i'd be inclined to try to model it after what's in input.py rather
> than deepcopy'ing.

Went with a mix of the two. deepcopied what is absolutely required, injected
things I need. Feels safer ;)

https://codereview.chromium.org/12476002/diff/3001/test/win/large-pdb/large-p...
File test/win/large-pdb/large-pdb.gyp (right):

https://codereview.chromium.org/12476002/diff/3001/test/win/large-pdb/large-p...
test/win/large-pdb/large-pdb.gyp:9: 'type': 'executable',
On 2013/03/06 19:03:34, scottmg wrote:
> please add a dll test too

Done.

[scottmg, 2013-03-14 17:37:08]

lgtm

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py#new...
pylib/gyp/MSVSUtil.py:26: def _PartiallyDeepCopyDict(dict, keys):
docstring

(trying to think of a better name too, but no great suggestions,
_DeepCopySomeKeys?)

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py#new...
pylib/gyp/MSVSUtil.py:133: if target_dict.get('msvs_large_pdb', '0') != '1':
nit, most gyp code is: if target_dict.get('msvs_large_pdb', 0):

[chrisha, 2013-03-15 00:02:44]

Thanks, committing.

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py
File pylib/gyp/MSVSUtil.py (right):

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py#new...
pylib/gyp/MSVSUtil.py:26: def _PartiallyDeepCopyDict(dict, keys):
On 2013/03/14 17:37:08, scottmg wrote:
> docstring
> 
> (trying to think of a better name too, but no great suggestions,
> _DeepCopySomeKeys?)

Done.

https://codereview.chromium.org/12476002/diff/11001/pylib/gyp/MSVSUtil.py#new...
pylib/gyp/MSVSUtil.py:133: if target_dict.get('msvs_large_pdb', '0') != '1':
On 2013/03/14 17:37:08, scottmg wrote:
> nit, most gyp code is: if target_dict.get('msvs_large_pdb', 0):

Done.

[chrisha, 2013-03-18 19:32:25]

Committed patchset #3 manually as r1600.