Selective build clobbering feature (landmines.py and android build scripts).

build/gyp_helper.py

+import os

[M-A Ruel, 2012/10/24 00:40:58]

Copyright line.

[iannucci, 2012/10/24 04:12:25]

Done

+
+def apply_gyp_environment(file_path=None):

[M-A Ruel, 2012/10/24 00:40:58]

why =None?

[iannucci, 2012/10/24 04:12:25]

This whole function was transplanted verbatim from gyp_chromium. Not sure why it
was =None there either. Weird.

+  """

[M-A Ruel, 2012/10/24 00:40:58]

In general we start the comment right after """.

[iannucci, 2012/10/24 04:12:25]

Done

+  Reads in a *.gyp_env file and applies the valid keys to os.environ.
+  """
+  if not file_path or not os.path.exists(file_path):
+    return
+  file_contents = open(file_path).read()

[iannucci, 2012/10/24 04:12:25]

I went ahead and put a 'with' on this... No reason to wait for python to close
this handle.

+  try:
+    file_data = eval(file_contents, {'__builtins__': None}, None)
+  except SyntaxError, e:
+    e.filename = os.path.abspath(file_path)
+    raise
+  supported_vars = ( 'CC',
+                     'CHROMIUM_GYP_FILE',
+                     'CHROMIUM_GYP_SYNTAX_CHECK',
+                     'CXX',
+                     'GYP_DEFINES',
+                     'GYP_GENERATOR_FLAGS',
+                     'GYP_GENERATOR_OUTPUT',
+                     'GYP_GENERATORS', )
+  for var in supported_vars:
+    val = file_data.get(var)

[M-A Ruel, 2012/10/24 00:40:58]

val&var are confusing, find a better variable name?

[iannucci, 2012/10/24 04:12:25]

Better?

+    if val:
+      if var in os.environ:
+        print 'INFO: Environment value for "%s" overrides value in %s.' % (

[M-A Ruel, 2012/10/24 00:40:58]

Is this necessary? Because otherwise you could do:
os.environ.setdefault(var, val)

Or you can look at the return value of setdefault().

[iannucci, 2012/10/24 04:12:25]

Again, moved from gyp_chromium :D. Did this have a purpose there? I'm assuming
yes?

[M-A Ruel, 2012/10/24 12:16:10]

No idea. I don't mind though.

+            var, os.path.abspath(file_path)
+        )
+      else:
+        os.environ[var] = val
+
+def apply_chromium_gyp_env():

[M-A Ruel, 2012/10/24 00:40:58]

The function names are really confusing.

[iannucci, 2012/10/24 04:12:25]

New ones better?

+  script_dir = os.path.dirname(os.path.realpath(__file__))

[M-A Ruel, 2012/10/24 00:40:58]

You can only do that while the module is being parsed. Afterward you don't know
if someone called os.chdir(). Make it a global variable.

[iannucci, 2012/10/24 04:12:25]

Good catch

+  chrome_src = os.path.abspath(os.path.join(script_dir, os.pardir))
+
+  if 'SKIP_CHROMIUM_GYP_ENV' not in os.environ:
+    # Update the environment based on chromium.gyp_env
+    path = os.path.join(os.path.dirname(chrome_src), 'chromium.gyp_env')
+    apply_gyp_environment(path)
+

[M-A Ruel, 2012/10/24 00:40:58]

Remove extraneous line.