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

Issue 10378042: Correct the order in which OutputDirectory and IntermediateDirectory are defined. (Closed)

Created:
8 years, 7 months ago by bradn
Modified:
8 years, 7 months ago
Reviewers:
gab, Nico, jeanluc1, scottmg
CC:
gyp-developer_googlegroups.com
Visibility:
Public.

Description

Correct the order in which OutputDirectory and IntermediateDirectory are defined, so that IntDir can use the definition of OutDir by topologically sorting references to msvs variables. Moved topological sort to a common location and switched it to a simpler more general depth first search version. BUG=http://code.google.com/p/chromium/issues/detail?id=119528 TEST=shared_output Committed: https://code.google.com/p/gyp/source/detail?r=1377

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Total comments: 3

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 6

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : #

Patch Set 12 : #

Patch Set 13 : #

Patch Set 14 : #

Patch Set 15 : #

Total comments: 8

Patch Set 16 : #

Total comments: 4

Patch Set 17 : #

Total comments: 6

Patch Set 18 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -52 lines) Patch
M pylib/gyp/common.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +49 lines, -0 lines 0 comments Download
A pylib/gyp/common_test.py View 1 2 3 4 5 6 7 8 9 10 1 chunk +44 lines, -0 lines 0 comments Download
M pylib/gyp/generator/msvs.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +29 lines, -1 line 0 comments Download
M pylib/gyp/xcode_emulation.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +18 lines, -51 lines 0 comments Download
A test/msvs/shared_output/common.gypi View 1 2 3 1 chunk +17 lines, -0 lines 0 comments Download
A test/msvs/shared_output/gyptest-shared_output.py View 1 2 3 4 5 6 7 8 1 chunk +33 lines, -0 lines 0 comments Download
A test/msvs/shared_output/hello.c View 1 chunk +12 lines, -0 lines 0 comments Download
A test/msvs/shared_output/hello.gyp View 1 1 chunk +21 lines, -0 lines 0 comments Download
A test/msvs/shared_output/there/there.c View 1 chunk +12 lines, -0 lines 0 comments Download
A test/msvs/shared_output/there/there.gyp View 1 1 chunk +16 lines, -0 lines 0 comments Download
M test/small/gyptest-small.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
bradn
8 years, 7 months ago (2012-05-08 20:02:10 UTC) #1
scottmg
Instead of documenting this in a test, could we instead maybe add something to input.py ...
8 years, 7 months ago (2012-05-08 20:25:14 UTC) #2
bradn
Figured out a way to fix it. (Never trust drivel from the interwebs...) PTAL
8 years, 7 months ago (2012-05-09 00:57:22 UTC) #3
scottmg
generally looks fine to me. http://codereview.chromium.org/10378042/diff/7012/test/msvs/shared_output/gyptest-shared_output.py File test/msvs/shared_output/gyptest-shared_output.py (right): http://codereview.chromium.org/10378042/diff/7012/test/msvs/shared_output/gyptest-shared_output.py#newcode17 test/msvs/shared_output/gyptest-shared_output.py:17: test = TestGyp.TestGyp(workdir='workarea_shared_output', formats=['msvs']) ...
8 years, 7 months ago (2012-05-09 02:45:41 UTC) #4
scottmg
On 2012/05/09 02:45:41, scottmg wrote: > work on ninja too Sorry, scratch that, the ninja ...
8 years, 7 months ago (2012-05-09 03:23:01 UTC) #5
gab
The idea lg, see comments below. http://codereview.chromium.org/10378042/diff/7012/pylib/gyp/generator/msvs.py File pylib/gyp/generator/msvs.py (right): http://codereview.chromium.org/10378042/diff/7012/pylib/gyp/generator/msvs.py#newcode2638 pylib/gyp/generator/msvs.py:2638: MSBUILD_PROPERTY_ORDER = [ ...
8 years, 7 months ago (2012-05-09 15:27:11 UTC) #6
bradn
Ok, switched to topological sort. PTAL http://codereview.chromium.org/10378042/diff/7012/pylib/gyp/generator/msvs.py File pylib/gyp/generator/msvs.py (right): http://codereview.chromium.org/10378042/diff/7012/pylib/gyp/generator/msvs.py#newcode2638 pylib/gyp/generator/msvs.py:2638: MSBUILD_PROPERTY_ORDER = [ ...
8 years, 7 months ago (2012-05-10 01:48:52 UTC) #7
gab
lg, see comment below http://codereview.chromium.org/10378042/diff/9005/pylib/gyp/generator/msvs.py File pylib/gyp/generator/msvs.py (right): http://codereview.chromium.org/10378042/diff/9005/pylib/gyp/generator/msvs.py#newcode2660 pylib/gyp/generator/msvs.py:2660: if v in properties and ...
8 years, 7 months ago (2012-05-10 13:57:07 UTC) #8
scottmg
Adding Nico as he wrote the xcode topological sort that looks like it's changed a ...
8 years, 7 months ago (2012-05-10 17:33:57 UTC) #9
Nico
If the tests pass, I'm happy. (rsesek wrote the toposort, I just moved it from ...
8 years, 7 months ago (2012-05-10 19:20:23 UTC) #10
bradn
I've switched to the normal definition of topological order everywhere (nodes appear before the nodes ...
8 years, 7 months ago (2012-05-10 20:17:56 UTC) #11
gab
Haven't thoroughly reviewed the new logic with forward vs reverse topo sort, but thanks for ...
8 years, 7 months ago (2012-05-10 20:45:30 UTC) #12
Nico
http://codereview.chromium.org/10378042/diff/10014/pylib/gyp/generator/make.py File pylib/gyp/generator/make.py (right): http://codereview.chromium.org/10378042/diff/10014/pylib/gyp/generator/make.py#newcode1799 pylib/gyp/generator/make.py:1799: for k in reversed(gyp.xcode_emulation.TopologicallySortedEnvVarKeys(env)): When refactoring a function (gyp.xcode_emu.TopSortEnvVarKeys), ...
8 years, 7 months ago (2012-05-11 14:53:44 UTC) #13
bradn
PTAL http://codereview.chromium.org/10378042/diff/10014/pylib/gyp/generator/make.py File pylib/gyp/generator/make.py (right): http://codereview.chromium.org/10378042/diff/10014/pylib/gyp/generator/make.py#newcode1799 pylib/gyp/generator/make.py:1799: for k in reversed(gyp.xcode_emulation.TopologicallySortedEnvVarKeys(env)): On 2012/05/11 14:53:44, Nico ...
8 years, 7 months ago (2012-05-11 18:57:23 UTC) #14
Nico
Nit is optional. The comment might be why the mac try result is red. http://codereview.chromium.org/10378042/diff/14025/pylib/gyp/xcode_emulation.py ...
8 years, 7 months ago (2012-05-11 19:03:34 UTC) #15
bradn
PTAL http://codereview.chromium.org/10378042/diff/14025/pylib/gyp/xcode_emulation.py File pylib/gyp/xcode_emulation.py (right): http://codereview.chromium.org/10378042/diff/14025/pylib/gyp/xcode_emulation.py#newcode964 pylib/gyp/xcode_emulation.py:964: for k in TopologicallySortedEnvVarKeys(expansions): On 2012/05/11 19:03:34, Nico ...
8 years, 7 months ago (2012-05-11 20:10:31 UTC) #16
Nico
8 years, 7 months ago (2012-05-11 20:16:23 UTC) #17
Lgtm if tests pass.

Thanks for humoring me :-)

Powered by Google App Engine
This is Rietveld 408576698