Bug 1256571 - Move Options handling to ConfigureSandbox._value_for(). r?chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 08 Apr 2016 16:55:33 +0900
changeset 350227 09a1607874d6adbf4cb410fdd6fe7a1a5074c650
parent 350226 0c969f8cf1364ddbf489b7a05154a11fc649e1aa
child 350228 e961522031a50fbd896dc357fb367f394eaee21d
push id15274
push userbmo:mh+mozilla@glandium.org
push dateWed, 13 Apr 2016 01:01:28 +0000
reviewerschmanchester
bugs1256571
milestone48.0a1
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.