gyp_chromium: Better parsing of -G command line flag.

Handle -G output_dir=foo as well as -Goutput_dir=foo.
I'm not sure how common this case is but we have
NaCl SDK bot that do this.

Getting this wrong results in windows toolchain being
installed in 'out/' (which doesn't necessarily exist)
rather than the output_dir tree and gyp uses.

Add unittests for the affected parts of gyp_chromium.

BUG=454594

Review URL: https://codereview.chromium.org/896663002

Cr-Original-Commit-Position: refs/heads/master@{#314381}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 10ef1e2aa8ebb806ccfe7c52235f0fb76564ee2c

PRESUBMIT.py

@@ -0,0 +1,18 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+WHITELIST = [ r'.+_test.py$' ]
+
+
+def _RunTests(input_api, output_api):
+  return (input_api.canned_checks.RunUnitTestsInDirectory(
+          input_api, output_api, '.', whitelist=[r'.+_test.py$']))
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return _RunTests(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return _RunTests(input_api, output_api)

gyp_chromium

@@ -7,6 +7,7 @@
 # This script is wrapper for Chromium that adds some support for how GYP
 # is invoked by Chromium beyond what can be done in the gclient hooks.
 
+import argparse
 import glob
 import gyp_environment
 import os
@@ -124,15 +125,10 @@ def GetGypVars(supplemental_files):
   env_items = ProcessGypDefinesItems(
       shlex.split(os.environ.get('GYP_DEFINES', '')))
 
-  # GYP defines from the command line. We can't use optparse since we want
-  # to ignore all arguments other than "-D".
-  cmdline_input_items = []
-  for i in range(len(sys.argv))[1:]:
-    if sys.argv[i].startswith('-D'):
-      if sys.argv[i] == '-D' and i + 1 < len(sys.argv):
-        cmdline_input_items += [sys.argv[i + 1]]
-      elif len(sys.argv[i]) > 2:
-        cmdline_input_items += [sys.argv[i][2:]]
+  # GYP defines from the command line.
+  parser = argparse.ArgumentParser()
+  parser.add_argument('-D', dest='defines', action='append', default=[])
+  cmdline_input_items = parser.parse_known_args()[0].defines
   cmdline_items = ProcessGypDefinesItems(cmdline_input_items)
 
   vars_dict = dict(supp_items + env_items + cmdline_items)
@@ -141,21 +137,21 @@ def GetGypVars(supplemental_files):
 
 def GetOutputDirectory():
   """Returns the output directory that GYP will use."""
-  # GYP generator flags from the command line. We can't use optparse since we
-  # want to ignore all arguments other than "-G".
-  needle = '-Goutput_dir='
-  cmdline_input_items = []
-  for item in sys.argv[1:]:
-    if item.startswith(needle):
-      return item[len(needle):]
 
-  env_items = shlex.split(os.environ.get('GYP_GENERATOR_FLAGS', ''))
+  # Handle command line generator flags.
+  parser = argparse.ArgumentParser()
+  parser.add_argument('-G', dest='genflags', default=[], action='append')
+  genflags = parser.parse_known_args()[0].genflags
+
+  # Handle generator flags from the environment.
+  genflags += shlex.split(os.environ.get('GYP_GENERATOR_FLAGS', ''))
+
   needle = 'output_dir='
-  for item in env_items:
+  for item in genflags:
     if item.startswith(needle):
       return item[len(needle):]
 
-  return "out"
+  return 'out'
 
 
 def additional_include_files(supplemental_files, args=[]):

gyp_chromium_test.py

@@ -0,0 +1,66 @@
+#!/usr/bin/env python
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import sys
+import unittest
+
+SCRIPT_DIR = os.path.abspath(os.path.dirname(__file__))
+SRC_DIR = os.path.dirname(SCRIPT_DIR)
+
+sys.path.append(os.path.join(SRC_DIR, 'third_party', 'pymock'))
+
+import mock
+
+# TODO(sbc): Make gyp_chromium more testable by putting the code in
+# a .py file.
+gyp_chromium = __import__('gyp_chromium')
+
+
+class TestGetOutputDirectory(unittest.TestCase):
+  @mock.patch('os.environ', {})
+  @mock.patch('sys.argv', [__file__])
+  def testDefaultValue(self):
+    self.assertEqual(gyp_chromium.GetOutputDirectory(), 'out')
+
+  @mock.patch('os.environ', {'GYP_GENERATOR_FLAGS': 'output_dir=envfoo'})
+  @mock.patch('sys.argv', [__file__])
+  def testEnvironment(self):
+    self.assertEqual(gyp_chromium.GetOutputDirectory(), 'envfoo')
+
+  @mock.patch('os.environ', {'GYP_GENERATOR_FLAGS': 'output_dir=envfoo'})
+  @mock.patch('sys.argv', [__file__, '-Goutput_dir=cmdfoo'])
+  def testGFlagOverridesEnv(self):
+    self.assertEqual(gyp_chromium.GetOutputDirectory(), 'cmdfoo')
+
+  @mock.patch('os.environ', {})
+  @mock.patch('sys.argv', [__file__, '-G', 'output_dir=foo'])
+  def testGFlagWithSpace(self):
+    self.assertEqual(gyp_chromium.GetOutputDirectory(), 'foo')
+
+
+class TestGetGypVars(unittest.TestCase):
+  @mock.patch('os.environ', {})
+  def testDefault(self):
+    self.assertEqual(gyp_chromium.GetGypVars([]), {})
+
+  @mock.patch('os.environ', {})
+  @mock.patch('sys.argv', [__file__, '-D', 'foo=bar'])
+  def testDFlags(self):
+    self.assertEqual(gyp_chromium.GetGypVars([]), {'foo': 'bar'})
+
+  @mock.patch('os.environ', {})
+  @mock.patch('sys.argv', [__file__, '-D', 'foo'])
+  def testDFlagsNoValue(self):
+    self.assertEqual(gyp_chromium.GetGypVars([]), {'foo': '1'})
+
+  @mock.patch('os.environ', {})
+  @mock.patch('sys.argv', [__file__, '-D', 'foo=bar', '-Dbaz'])
+  def testDFlagMulti(self):
+    self.assertEqual(gyp_chromium.GetGypVars([]), {'foo': 'bar', 'baz': '1'})
+
+
+if __name__ == '__main__':
+  unittest.main()