Add support for a .gyp_env so you don't have to set a bunch of independent...

Add support for a chromium.gyp_env at the top of the tree (peer of src) so you
  don't have to set a bunch of independent variables and can instead set
  things in a group per tree.
BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65408

[TVL, 2010-11-08 18:42:47]

http://codereview.chromium.org/4705001/diff/1/2
File build/gyp_chromium (right):

http://codereview.chromium.org/4705001/diff/1/2#newcode80
build/gyp_chromium:80: gyp_env_path = os.path.join(chrome_src, '.gyp_env')
this makes the file src/.gyp_env, should I push that up a level so it lives next
to the .gclient file (and isn't in a directory under a vcs system)

[TVL, 2010-11-08 19:06:37]

added env var to skip the file (for debugging, etc.)

[TVL, 2010-11-08 19:18:17]

Went ahead and moved the file outside the src tree.  Also made the error report
use an absolute path.

[Mark Mentovai, 2010-11-08 19:44:24]

The CL description should contain some sort of reference to src so that people
will know where the file belongs. I think based on a read here, it would be a
sibling to src (and .gclient), which is what I’d consider the correct location.

http://codereview.chromium.org/4705001/diff/7002/9001
File build/gyp_chromium (right):

http://codereview.chromium.org/4705001/diff/7002/9001#newcode33
build/gyp_chromium:33: except Exception, e:
If Exception is uncaught here, won’t it automatically be propagated beyond this
try?

http://codereview.chromium.org/4705001/diff/7002/9001#newcode35
build/gyp_chromium:35: supported_vars = ( 'CHROMIUM_GYP_FILE',
'CHROMIUM_GYP_SYNTAX_CHECK',
One per line, please.

http://codereview.chromium.org/4705001/diff/7002/9001#newcode43
build/gyp_chromium:43: val = existing_value + ' ' + val
Is this safe with all of the above supported_vars?

http://codereview.chromium.org/4705001/diff/7002/9001#newcode81
build/gyp_chromium:81: gyp_env_path = os.path.join(os.path.dirname(chrome_src),
'.gyp_env')
I don’t think the file should be named .gyp_env (and thus hidden). I thought
.gyp_env would be a good extension.

[TVL, 2010-11-08 19:49:43]

description also updated.

http://codereview.chromium.org/4705001/diff/7002/9001
File build/gyp_chromium (right):

http://codereview.chromium.org/4705001/diff/7002/9001#newcode33
build/gyp_chromium:33: except Exception, e:
On 2010/11/08 19:44:24, Mark Mentovai wrote:
> If Exception is uncaught here, won’t it automatically be propagated beyond
this
> try?

Done.

http://codereview.chromium.org/4705001/diff/7002/9001#newcode35
build/gyp_chromium:35: supported_vars = ( 'CHROMIUM_GYP_FILE',
'CHROMIUM_GYP_SYNTAX_CHECK',
On 2010/11/08 19:44:24, Mark Mentovai wrote:
> One per line, please.

Done.

http://codereview.chromium.org/4705001/diff/7002/9001#newcode43
build/gyp_chromium:43: val = existing_value + ' ' + val
On 2010/11/08 19:44:24, Mark Mentovai wrote:
> Is this safe with all of the above supported_vars?

The syntax check and generator output are the only two that are single values,
so I keep it simple.  The other option was if there is something sent in the
env, the ignore what is in the file?

http://codereview.chromium.org/4705001/diff/7002/9001#newcode81
build/gyp_chromium:81: gyp_env_path = os.path.join(os.path.dirname(chrome_src),
'.gyp_env')
On 2010/11/08 19:44:24, Mark Mentovai wrote:
> I don’t think the file should be named .gyp_env (and thus hidden). I thought
> .gyp_env would be a good extension.

Done.

[Mark Mentovai, 2010-11-08 19:58:21]

LG otherwise

http://codereview.chromium.org/4705001/diff/14001/15001
File build/gyp_chromium (right):

http://codereview.chromium.org/4705001/diff/14001/15001#newcode1
build/gyp_chromium:1: #!/usr/bin/python
CL description:
> Add support for a chromium.gyp_env at the top of the tree (per of src) so you

per -> peer

> don't have to set a bunch of independent variables and can instead set
> things in a group per tree.

http://codereview.chromium.org/4705001/diff/14001/15001#newcode23
build/gyp_chromium:23: Reads in a *.gyp_env and applies the valid keys to
os.environ.
*.gyp_env file.

http://codereview.chromium.org/4705001/diff/14001/15001#newcode42
build/gyp_chromium:42: if existing_value:
I would prefer not overwriting existing values at all. The ' '-append may not be
right, and even for things that are properly handled by ' '-appending, doing an
append may not be right given the values involved.

http://codereview.chromium.org/4705001/diff/14001/15001#newcode80
build/gyp_chromium:80: # Update the environment based on .gyp_env
chromium.gyp_env.