Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

Issue 12476002: Create msvs_large_pdb workaround. (Closed)

Created:
7 years, 9 months ago by chrisha
Modified:
7 years, 9 months ago
Reviewers:
Evan Martin, scottmg
CC:
gyp-developer_googlegroups.com
Base URL:
https://git.chromium.org/git/external/gyp.git@master
Visibility:
Public.

Description

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

Patch Set 1 : #

Total comments: 15

Patch Set 2 : Address review comments. #

Total comments: 4

Patch Set 3 : Addressed scottmg's nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+280 lines, -6 lines) Patch
A data/win/large-pdb-shim.cc View 1 1 chunk +12 lines, -0 lines 0 comments Download
M pylib/gyp/MSVSUtil.py View 1 2 3 chunks +141 lines, -3 lines 0 comments Download
M pylib/gyp/generator/msvs.py View 2 chunks +6 lines, -0 lines 0 comments Download
M pylib/gyp/generator/ninja.py View 1 1 chunk +3 lines, -0 lines 0 comments Download
A test/win/gyptest-link-large-pdb.py View 1 1 chunk +49 lines, -0 lines 0 comments Download
A + test/win/large-pdb/dllmain.cc View 1 1 chunk +4 lines, -2 lines 0 comments Download
A test/win/large-pdb/large-pdb.gyp View 1 1 chunk +64 lines, -0 lines 0 comments Download
A + test/win/large-pdb/main.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
chrisha
PTAL. I'm not expecting a detailed review here yet, simply a quick flyby to see ...
7 years, 9 months ago (2013-03-05 20:52:25 UTC) #1
scottmg
it'd be nice to do it at a lower level instead (i.e. just insert a ...
7 years, 9 months ago (2013-03-06 19:03:34 UTC) #2
Evan Martin
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#newcode3 data/win/large-pdb-shim.cc:3: // found in the LICENSE file. Should use the ...
7 years, 9 months ago (2013-03-06 20:43:39 UTC) #3
chrisha
I've cleaned this up to address the review comments. I prefer not to do this ...
7 years, 9 months ago (2013-03-14 15:57:23 UTC) #4
scottmg
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#newcode26 pylib/gyp/MSVSUtil.py:26: def _PartiallyDeepCopyDict(dict, keys): docstring (trying to think of ...
7 years, 9 months ago (2013-03-14 17:37:08 UTC) #5
chrisha
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#newcode26 pylib/gyp/MSVSUtil.py:26: def _PartiallyDeepCopyDict(dict, keys): On 2013/03/14 17:37:08, scottmg ...
7 years, 9 months ago (2013-03-15 00:02:44 UTC) #6
chrisha
7 years, 9 months ago (2013-03-18 19:32:25 UTC) #7
Message was sent while issue was closed.
Committed patchset #3 manually as r1600.

Powered by Google App Engine
This is Rietveld 408576698