Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 406523005: ninja/win: Put common msvs_system_include_dirs into %INCLUDE% (Closed)

Created:
5 years, 3 months ago by Nico
Modified:
5 years, 3 months ago
Reviewers:
scottmg
CC:
gyp-developer_googlegroups.com
Base URL:
https://chromium.googlesource.com/external/gyp.git@master
Visibility:
Public.

Description

ninja/win: Put common msvs_system_include_dirs into %INCLUDE% Paths in INCLUDE are considered system headers by clang-cl and it will suppress warnings from system headers. BUG=chromium:395405 r1953

Patch Set 1 #

Total comments: 4

Patch Set 2 : expand #

Patch Set 3 : order #

Patch Set 4 : move #

Patch Set 5 : test #

Patch Set 6 : noswp #

Total comments: 1

Patch Set 7 : ninja_use_custom_environment_files sigh #

Patch Set 8 : . #

Patch Set 9 : . #

Total comments: 7

Patch Set 10 : . #

Patch Set 11 : rebase #

Patch Set 12 : rebase2 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -41 lines) Patch
M pylib/gyp/generator/ninja.py View 1 2 3 4 5 6 2 chunks +9 lines, -1 line 0 comments Download
M pylib/gyp/msvs_emulation.py View 1 2 3 4 5 6 7 8 9 10 6 chunks +59 lines, -18 lines 0 comments Download
A + test/win/gyptest-system-include.py View 1 2 3 4 11 1 chunk +21 lines, -22 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Nico
That's how the gyp-based thing looks. Worth cleaning up and writing tests for? It seems ...
5 years, 3 months ago (2014-07-19 01:05:31 UTC) #1
scottmg
Sure, seems reasonable if it helps. https://codereview.chromium.org/406523005/diff/1/pylib/gyp/generator/ninja.py File pylib/gyp/generator/ninja.py (right): https://codereview.chromium.org/406523005/diff/1/pylib/gyp/generator/ninja.py#newcode1784 pylib/gyp/generator/ninja.py:1784: for qualified_target in ...
5 years, 3 months ago (2014-07-19 18:49:36 UTC) #2
Nico
ptal. (I'll fix the CL description in a second.) I started writing a test, but ...
5 years, 3 months ago (2014-07-19 23:05:35 UTC) #3
scottmg
lgtm https://codereview.chromium.org/406523005/diff/150001/pylib/gyp/generator/ninja.py File pylib/gyp/generator/ninja.py (right): https://codereview.chromium.org/406523005/diff/150001/pylib/gyp/generator/ninja.py#newcode1784 pylib/gyp/generator/ninja.py:1784: if not generator_flags.get('ninja_use_custom_environment_files', 0): Does this mean this ...
5 years, 3 months ago (2014-07-20 05:18:10 UTC) #4
scottmg
https://codereview.chromium.org/406523005/diff/150001/test/win/system-include/test.gyp File test/win/system-include/test.gyp (right): https://codereview.chromium.org/406523005/diff/150001/test/win/system-include/test.gyp#newcode11 test/win/system-include/test.gyp:11: 'common', # Same for all targets This might need ...
5 years, 3 months ago (2014-07-20 05:20:07 UTC) #5
Nico
https://codereview.chromium.org/406523005/diff/150001/pylib/gyp/generator/ninja.py File pylib/gyp/generator/ninja.py (right): https://codereview.chromium.org/406523005/diff/150001/pylib/gyp/generator/ninja.py#newcode1784 pylib/gyp/generator/ninja.py:1784: if not generator_flags.get('ninja_use_custom_environment_files', 0): On 2014/07/20 05:18:10, scottmg wrote: ...
5 years, 3 months ago (2014-07-20 05:51:02 UTC) #6
Nico
ima try landing the source files first. Maybe the try server has trouble applying the ...
5 years, 3 months ago (2014-07-20 05:53:14 UTC) #7
Nico
5 years, 3 months ago (2014-07-20 16:10:43 UTC) #8
On 2014/07/20 05:53:14, Nico (away) wrote:
> ima try landing the source files first. Maybe the try server has trouble
> applying the patch; the raw git diff looks a bit weird.

that worked! :-)

:-(

Powered by Google App Engine
This is Rietveld 408576698