Bug 1256571 - Move applying implied options to ConfigureSandbox._value_for(). r=chmanchester
authorMike Hommey <mh+mozilla@glandium.org>
Sat, 09 Apr 2016 17:45:07 +0900
changeset 316889 ae6bd5ce0abb525898a970caa70162b77c0e55a8
parent 316888 cc9713988f7ad2af3741e2f6bd296504636fd26b
child 316890 2916126df6689dc379b1b7c3a55fc236932a9a2d
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschmanchester
bugs1256571
milestone48.0a1
Bug 1256571 - Move applying implied options 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
@@ -118,18 +118,18 @@ class ConfigureSandbox(dict):
         self._imports = {}
 
         self._options = OrderedDict()
         # 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 = {}
+        # inferred. Their order matters, so use a list.
+        self._implied_options = []
 
         # Store all results from _prepare_function
         self._prepared_functions = set()
 
         self._helper = CommandLineHelper(environ, argv)
 
         assert isinstance(config, dict)
         self._config = config
@@ -211,24 +211,26 @@ class ConfigureSandbox(dict):
 
             # 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 implied options should exist.
+        for implied_option in self._implied_options:
+            raise ConfigureError(
+                '`%s`, emitted from `%s` line %d, is unknown.'
+                % (implied_option.option, implied_option.caller[1],
+                   implied_option.caller[2]))
+
         # 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)
 
         if self._help:
             with LineIO(self.log_impl.info) as out:
                 self._help.usage(out)
 
     def __getitem__(self, key):
         impl = '%s_impl' % key
@@ -289,20 +291,48 @@ class ConfigureSandbox(dict):
                             "`%s` must depend on '--help'"
                             % (func.__name__, arg.__name__, arg.__name__))
 
         resolved_args = [self._value_for(d) for d in dependencies]
         return func(*resolved_args)
 
     @memoize
     def _value_for_option(self, option):
+        implied = {}
+        for implied_option in self._implied_options[:]:
+            if implied_option.name not in (option.name, option.env):
+                continue
+            self._implied_options.remove(implied_option)
+
+            value = self._resolve(implied_option.value,
+                                  need_help_dependency=False)
+
+            if value is not None:
+                if isinstance(value, OptionValue):
+                    pass
+                elif value is True:
+                    value = PositiveOptionValue()
+                elif value is False or value == ():
+                    value = NegativeOptionValue()
+                elif isinstance(value, types.StringTypes):
+                    value = PositiveOptionValue((value,))
+                elif isinstance(value, tuple):
+                    value = PositiveOptionValue(value)
+                else:
+                    raise TypeError("Unexpected type: '%s'"
+                                    % type(value).__name__)
+
+                opt = value.format(implied_option.option)
+                self._helper.add(opt, 'implied')
+                implied[opt] = implied_option
+
         try:
             value, option_string = self._helper.handle(option)
         except ConflictingOptionError as e:
-            frameinfo, reason = self._implied_options[e.arg]
+            reason = implied[e.arg].reason
             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)
 
@@ -609,34 +639,28 @@ class ConfigureSandbox(dict):
                     reason = possible_reasons[0]
 
         if not reason:
             raise ConfigureError(
                 "Cannot infer what implies '%s'. Please add a `reason` to "
                 "the `imply_option` call."
                 % option)
 
-        value = self._resolve(value, need_help_dependency=False)
-        if value is not None:
-            if isinstance(value, OptionValue):
-                pass
-            elif value is True:
-                value = PositiveOptionValue()
-            elif value is False or value == ():
-                value = NegativeOptionValue()
-            elif isinstance(value, types.StringTypes):
-                value = PositiveOptionValue((value,))
-            elif isinstance(value, tuple):
-                value = PositiveOptionValue(value)
-            else:
-                raise TypeError("Unexpected type: '%s'" % type(value).__name__)
+        prefix, name, values = Option.split_option(option)
+        if values != ():
+            raise ConfigureError("Implied option must not contain an '='")
 
-            option = value.format(option)
-            self._helper.add(option, 'implied')
-            self._implied_options[option] = inspect.stack()[1], reason
+        self._implied_options.append(ReadOnlyNamespace(
+            option=option,
+            prefix=prefix,
+            name=name,
+            value=value,
+            caller=inspect.stack()[1],
+            reason=reason,
+        ))
 
     def _prepare_function(self, func):
         '''Alter the given function global namespace with the common ground
         for @depends, and @template.
         '''
         if not inspect.isfunction(func):
             raise TypeError("Unexpected type: '%s'" % type(func).__name__)
         if func in self._prepared_functions: