Bug 1256571 - Move Options handling to ConfigureSandbox._value_for(). r=chmanchester
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 08 Apr 2016 16:55:33 +0900
changeset 330984 7e0fa338d4bfac3fb692c605a657462e6f4b1695
parent 330983 dbd34b3b1cf951dff0b19dde1916dc6dc0c2d367
child 330985 cc9713988f7ad2af3741e2f6bd296504636fd26b
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschmanchester
bugs1256571
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1256571 - Move Options handling to ConfigureSandbox._value_for(). r=chmanchester
python/mozbuild/mozbuild/configure/__init__.py
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -23,16 +23,17 @@ from mozbuild.configure.options import (
     PositiveOptionValue,
 )
 from mozbuild.configure.help import HelpFormatter
 from mozbuild.configure.util import (
     ConfigureOutputHandler,
     LineIO,
 )
 from mozbuild.util import (
+    memoize,
     ReadOnlyDict,
     ReadOnlyNamespace,
 )
 import mozpack.path as mozpath
 
 
 class ConfigureError(Exception):
     pass
@@ -114,18 +115,16 @@ class ConfigureSandbox(dict):
         self._depends = {}
         self._seen = set()
         # Store the @imports added to a given function.
         self._imports = {}
 
         self._options = OrderedDict()
         # Store the raw values returned by @depends functions
         self._results = {}
-        # Store values for each Option, as per returned by Option.get_value
-        self._option_values = {}
         # Store raw option (as per command line or environment) for each Option
         self._raw_options = {}
 
         # Store options added with `imply_option`, and the reason they were
         # added (which can either have been given to `imply_option`, or
         # inferred.
         self._implied_options = {}
 
@@ -199,34 +198,41 @@ class ConfigureSandbox(dict):
 
         self._paths.pop(-1)
 
     def run(self, path):
         '''Executes the given file within the sandbox, and ensure the overall
         consistency of the executed script.'''
         self.exec_file(path)
 
-        # All command line arguments should have been removed (handled) by now.
+        for option in self._options.itervalues():
+            # All options must be referenced by some @depends function
+            if option not in self._seen:
+                raise ConfigureError(
+                    'Option `%s` is not handled ; reference it with a @depends'
+                    % option.option
+                )
+
+            # When running with --help, few options are handled but we still
+            # want to find the unknown ones below, so handle them all now. We
+            # however don't run any of the @depends function that depend on
+            # them.
+            if self._help:
+                self._helper.handle(option)
+
+        # All options should have been removed (handled) by now.
         for arg in self._helper:
             without_value = arg.split('=', 1)[0]
             if arg in self._implied_options:
                 frameinfo, reason = self._implied_options[arg]
                 raise ConfigureError(
                     '`%s`, emitted from `%s` line %d, is unknown.'
                     % (without_value, frameinfo[1], frameinfo[2]))
             raise InvalidOptionError('Unknown option: %s' % without_value)
 
-        # All options must be referenced by some @depends function
-        for option in self._options.itervalues():
-            if option not in self._seen:
-                raise ConfigureError(
-                    'Option `%s` is not handled ; reference it with a @depends'
-                    % option.option
-                )
-
         if self._help:
             with LineIO(self.log_impl.info) as out:
                 self._help.usage(out)
 
     def __getitem__(self, key):
         impl = '%s_impl' % key
         func = getattr(self, impl, None)
         if func:
@@ -263,20 +269,36 @@ class ConfigureSandbox(dict):
     def _value_for(self, obj):
         if isinstance(obj, DependsFunction):
             assert obj in self._depends
             func, deps = self._depends[obj]
             assert not inspect.isgeneratorfunction(func)
             assert func in self._results
             return self._results[func]
         elif isinstance(obj, Option):
-            assert obj in self._option_values
-            return self._option_values.get(obj)
+            return self._value_for_option(obj)
+
         assert False
 
+    @memoize
+    def _value_for_option(self, option):
+        try:
+            value, option_string = self._helper.handle(option)
+        except ConflictingOptionError as e:
+            frameinfo, reason = self._implied_options[e.arg]
+            reason = self._raw_options.get(reason) or reason.option
+            raise InvalidOptionError(
+                "'%s' implied by '%s' conflicts with '%s' from the %s"
+                % (e.arg, reason, e.old_arg, e.old_origin))
+
+        self._raw_options[option] = (option_string.split('=', 1)[0]
+                                     if option_string else option_string)
+
+        return value
+
     def option_impl(self, *args, **kwargs):
         '''Implementation of option()
         This function creates and returns an Option() object, passing it the
         resolved arguments (uses the result of functions when functions are
         passed). In most cases, the result of this function is not expected to
         be used.
         Command line argument/environment variable parsing for this Option is
         handled here.
@@ -288,31 +310,21 @@ class ConfigureSandbox(dict):
             raise ConfigureError('Option `%s` already defined' % option.option)
         if option.env in self._options:
             raise ConfigureError('Option `%s` already defined' % option.env)
         if option.name:
             self._options[option.name] = option
         if option.env:
             self._options[option.env] = option
 
-        try:
-            value, option_string = self._helper.handle(option)
-        except ConflictingOptionError as e:
-            frameinfo, reason = self._implied_options[e.arg]
-            reason = self._raw_options.get(reason) or reason.option
-            raise InvalidOptionError(
-                "'%s' implied by '%s' conflicts with '%s' from the %s"
-                % (e.arg, reason, e.old_arg, e.old_origin))
-
         if self._help:
             self._help.add(option)
 
-        self._option_values[option] = value
-        self._raw_options[option] = (option_string.split('=', 1)[0]
-                                     if option_string else option_string)
+        self._value_for(option)
+
         return option
 
     def depends_impl(self, *args):
         '''Implementation of @depends()
         This function is a decorator. It returns a function that subsequently
         takes a function and returns a dummy function. The dummy function
         identifies the actual function for the sandbox, while preventing
         further function calls from within the sandbox.