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

Issue 10833021: Honor $CC/$CC_host and friends in make generator. (Closed)

Created:
8 years, 5 months ago by Sam Clegg
Modified:
8 years, 4 months ago
CC:
gyp-developer_googlegroups.com
Base URL:
http://git.chromium.org/external/gyp.git@master
Visibility:
Public.

Description

Honor $CC_target and friends in make generator. These get baked into the generated build file if they are set at gyp time. BUG= TEST=compiler-override Committed: https://code.google.com/p/gyp/source/detail?r=1470

Patch Set 1 #

Patch Set 2 : Added test #

Patch Set 3 : remove debugging code #

Patch Set 4 : Remove docstring cleanups (better to commit seperately) #

Total comments: 1

Patch Set 5 : use LD_target rather than LINK_target as environment variable to match existing ninja behaviour #

Patch Set 6 : Fix line length lint in make.py #

Patch Set 7 : my_ld.py now fakes ld.exe a little more so ninja win32 tests pass #

Patch Set 8 : ninja: don't run manifest tool when LD_target is overridden #

Total comments: 6

Patch Set 9 : fix based on brads comments #

Patch Set 10 : #

Total comments: 3

Patch Set 11 : add another test for "make_global_settings" #

Patch Set 12 : ninja now honour LD and LD.host #

Patch Set 13 : #

Total comments: 8

Patch Set 14 : use raw paths in compiler-global-settings.gyp.in #

Patch Set 15 : fix issues from Nico #

Patch Set 16 : don't run cross compiler tests on win32-ninja #

Total comments: 6

Patch Set 17 : #

Patch Set 18 : Remove msvs_emulation.py changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+241 lines, -77 lines) Patch
M pylib/gyp/common.py View 1 2 3 4 5 6 7 8 9 10 1 chunk +9 lines, -0 lines 0 comments Download
M pylib/gyp/generator/make.py View 1 2 3 4 5 6 7 8 9 10 8 chunks +34 lines, -23 lines 0 comments Download
M pylib/gyp/generator/ninja.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 7 chunks +42 lines, -29 lines 0 comments Download
A + test/compiler-override/compiler.gyp View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -1 line 0 comments Download
A test/compiler-override/compiler-global-settings.gyp.in View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +34 lines, -0 lines 0 comments Download
A + test/compiler-override/compiler-host.gyp View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
A + test/compiler-override/cxxtest.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -4 lines 0 comments Download
A test/compiler-override/gyptest-compiler-env.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +55 lines, -0 lines 0 comments Download
A test/compiler-override/gyptest-compiler-global-settings.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +52 lines, -0 lines 0 comments Download
A + test/compiler-override/my_cc.py View 1 2 3 4 5 6 7 8 1 chunk +1 line, -5 lines 0 comments Download
A + test/compiler-override/my_cxx.py View 1 2 3 4 5 6 7 8 1 chunk +1 line, -5 lines 0 comments Download
A + test/compiler-override/my_ld.py View 1 2 3 4 5 6 7 8 1 chunk +1 line, -5 lines 0 comments Download
A + test/compiler-override/test.c View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -4 lines 0 comments Download
M test/lib/TestCommon.py View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 34 (0 generated)
Sam Clegg
8 years, 5 months ago (2012-07-27 01:06:42 UTC) #1
bradn
https://chromiumcodereview.appspot.com/10833021/diff/6001/pylib/gyp/generator/make.py File pylib/gyp/generator/make.py (right): https://chromiumcodereview.appspot.com/10833021/diff/6001/pylib/gyp/generator/make.py#newcode1997 pylib/gyp/generator/make.py:1997: header_params.update({ Style guide wants these <=80 chars per line
8 years, 4 months ago (2012-07-27 23:08:53 UTC) #2
bradn
Have you been able to kick off tryjobs for this?
8 years, 4 months ago (2012-07-27 23:09:27 UTC) #3
Sam Clegg
On 2012/07/27 23:09:27, bradn wrote: > Have you been able to kick off tryjobs for ...
8 years, 4 months ago (2012-07-30 18:52:02 UTC) #4
Sam Clegg
On 2012/07/30 18:52:02, Sam Clegg wrote: > On 2012/07/27 23:09:27, bradn wrote: > > Have ...
8 years, 4 months ago (2012-07-30 22:52:36 UTC) #5
Sam Clegg
Bots now passing
8 years, 4 months ago (2012-07-31 16:20:30 UTC) #6
bradn
Getting close. Nico as this does affect make + ninja behavior I want to make ...
8 years, 4 months ago (2012-07-31 17:19:23 UTC) #7
Sam Clegg
fixed style issues
8 years, 4 months ago (2012-07-31 18:32:10 UTC) #8
Sam Clegg
On 2012/07/31 18:32:10, Sam Clegg wrote: > fixed style issues Is there some way to ...
8 years, 4 months ago (2012-07-31 18:37:25 UTC) #9
bradn
lgtm
8 years, 4 months ago (2012-07-31 22:03:48 UTC) #10
bradn
I think you can ignore that one presubmit warning.
8 years, 4 months ago (2012-07-31 22:04:42 UTC) #11
bradn
Please wait for thakis' LG on this one.
8 years, 4 months ago (2012-07-31 22:04:57 UTC) #12
Nico
What do you need this for, and how does it compare to https://chromiumcodereview.appspot.com/9649016/ ?
8 years, 4 months ago (2012-08-01 18:18:46 UTC) #13
Sam Clegg
On 2012/08/01 18:18:46, Nico wrote: > What do you need this for, and how does ...
8 years, 4 months ago (2012-08-01 18:34:55 UTC) #14
Sam Clegg
On 2012/08/01 18:34:55, Sam Clegg wrote: > On 2012/08/01 18:18:46, Nico wrote: > > What ...
8 years, 4 months ago (2012-08-01 18:39:00 UTC) #15
Sam Clegg
On 2012/08/01 18:39:00, Sam Clegg wrote: > On 2012/08/01 18:34:55, Sam Clegg wrote: > > ...
8 years, 4 months ago (2012-08-01 18:39:55 UTC) #16
Nico
I like the tests. Do you have a preference if ninja should work like make, ...
8 years, 4 months ago (2012-08-01 18:42:46 UTC) #17
Sam Clegg
On 2012/08/01 18:42:46, Nico wrote: > I like the tests. > > Do you have ...
8 years, 4 months ago (2012-08-01 20:53:25 UTC) #18
Sam Clegg
On 2012/08/01 20:53:25, Sam Clegg wrote: > On 2012/08/01 18:42:46, Nico wrote: > > I ...
8 years, 4 months ago (2012-08-02 18:22:41 UTC) #19
Nico
Normally for crosscompiling with make, one sets CC and CC.host. I'm not sure why we ...
8 years, 4 months ago (2012-08-06 19:25:07 UTC) #20
Sam Clegg
On 2012/08/06 19:25:07, Nico wrote: > Normally for crosscompiling with make, one sets CC and ...
8 years, 4 months ago (2012-08-06 19:41:23 UTC) #21
Sam Clegg
Add more tests. Support CC in a an alias for CC_target if CC_target is not ...
8 years, 4 months ago (2012-08-08 21:15:35 UTC) #22
Nico
http://codereview.chromium.org/10833021/diff/20001/pylib/gyp/generator/make.py File pylib/gyp/generator/make.py (right): http://codereview.chromium.org/10833021/diff/20001/pylib/gyp/generator/make.py#newcode2012 pylib/gyp/generator/make.py:2012: 'LINK.target': GetEnv(('LD_target', 'LD'), '$(LINK)'), If I understood michaelbai right, ...
8 years, 4 months ago (2012-08-08 21:26:08 UTC) #23
Sam Clegg
http://codereview.chromium.org/10833021/diff/20001/pylib/gyp/generator/make.py File pylib/gyp/generator/make.py (right): http://codereview.chromium.org/10833021/diff/20001/pylib/gyp/generator/make.py#newcode2012 pylib/gyp/generator/make.py:2012: 'LINK.target': GetEnv(('LD_target', 'LD'), '$(LINK)'), On 2012/08/08 21:26:08, Nico wrote: ...
8 years, 4 months ago (2012-08-10 00:43:58 UTC) #24
Sam Clegg
make and ninja are now more or less consistent in the way host and target ...
8 years, 4 months ago (2012-08-13 19:23:05 UTC) #25
Nico
LGTM, thanks! I'd undo the windows cross-compiler bits though. http://codereview.chromium.org/10833021/diff/14012/pylib/gyp/generator/ninja.py File pylib/gyp/generator/ninja.py (right): http://codereview.chromium.org/10833021/diff/14012/pylib/gyp/generator/ninja.py#newcode1530 pylib/gyp/generator/ninja.py:1530: ...
8 years, 4 months ago (2012-08-13 23:14:07 UTC) #26
Sam Clegg
Ok. All requested fixed made. http://codereview.chromium.org/10833021/diff/14012/pylib/gyp/generator/ninja.py File pylib/gyp/generator/ninja.py (right): http://codereview.chromium.org/10833021/diff/14012/pylib/gyp/generator/ninja.py#newcode1530 pylib/gyp/generator/ninja.py:1530: '$mt -nologo -manifest $manifests ...
8 years, 4 months ago (2012-08-14 21:10:32 UTC) #27
Nico
Two more questions, sorry. http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py File pylib/gyp/msvs_emulation.py (right): http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py#newcode117 pylib/gyp/msvs_emulation.py:117: return '' Why do you ...
8 years, 4 months ago (2012-08-14 21:36:26 UTC) #28
Sam Clegg
http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py File pylib/gyp/msvs_emulation.py (right): http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py#newcode117 pylib/gyp/msvs_emulation.py:117: return '' On 2012/08/14 21:36:26, Nico wrote: > Why ...
8 years, 4 months ago (2012-08-14 22:36:57 UTC) #29
Nico
http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py File pylib/gyp/msvs_emulation.py (right): http://codereview.chromium.org/10833021/diff/3056/pylib/gyp/msvs_emulation.py#newcode117 pylib/gyp/msvs_emulation.py:117: return '' On 2012/08/14 22:36:57, Sam Clegg wrote: > ...
8 years, 4 months ago (2012-08-14 22:42:26 UTC) #30
Sam Clegg
Ok. This should be ok to commit now?
8 years, 4 months ago (2012-08-15 00:07:57 UTC) #31
Nico
On 2012/08/15 00:07:57, Sam Clegg wrote: > Ok. This should be ok to commit now? ...
8 years, 4 months ago (2012-08-15 00:08:40 UTC) #32
Sam Clegg
8 years, 4 months ago (2012-08-15 01:38:24 UTC) #33
Nico
8 years, 4 months ago (2012-08-15 02:36:21 UTC) #34
patch set 18 lgtm

Powered by Google App Engine
This is Rietveld 408576698