Skip to content

Commit

Permalink
gyp_chromium: Better parsing of -G command line flag.
Browse files Browse the repository at this point in the history
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-Commit-Position: refs/heads/master@{#314381}
  • Loading branch information
sbc100 authored and Commit bot committed Feb 3, 2015
1 parent 90e6cbf commit 10ef1e2
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 19 deletions.
18 changes: 18 additions & 0 deletions build/PRESUBMIT.py
Original file line number Diff line number Diff line change
@@ -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)
34 changes: 15 additions & 19 deletions build/gyp_chromium
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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=[]):
Expand Down
66 changes: 66 additions & 0 deletions build/gyp_chromium_test.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit 10ef1e2

Please sign in to comment.