Bug 1529799 - Prevent recursive resolution of options during imply_option. r=chmanchester
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 26 Feb 2019 22:05:24 +0000
changeset 519142 69f758a41070f572edf341089336bc1d4c3168a8
parent 519141 9d7e38697deec620a5cd00ca4c39912526107909
child 519143 a1be066f767a3d6b4c06e8afd8dae29fed528c6f
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschmanchester
bugs1529799
milestone67.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 1529799 - Prevent recursive resolution of options during imply_option. r=chmanchester In cases like those in the added unit test, explicit options on the command line could end up being silently ignored. So instead of that happening, error out. Unfortunately, the error message is not entirely accurate, but it's better than nothing. It's rare anyways (I only stumbled upon it because I was trying to do something fishy), and correlation between the error message and the corresponding changes should make it clear what's going on. Depends on D20822 Differential Revision: https://phabricator.services.mozilla.com/D20823
python/mozbuild/mozbuild/configure/__init__.py
python/mozbuild/mozbuild/test/configure/test_configure.py
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -561,16 +561,24 @@ class ConfigureSandbox(dict):
             reason = implied[e.arg].reason
             if isinstance(reason, Option):
                 reason = self._raw_options.get(reason) or reason.option
                 reason = reason.split('=', 1)[0]
             raise InvalidOptionError(
                 "'%s' implied by '%s' conflicts with '%s' from the %s"
                 % (e.arg, reason, e.old_arg, e.old_origin))
 
+        if value.origin == 'implied':
+            recursed_value = getattr(self, '__value_for_option').get((option,))
+            if recursed_value is not None:
+                _, filename, line, _, _, _ = implied[value.format(option.option)].caller
+                raise ConfigureError(
+                    "'%s' appears somewhere in the direct or indirect dependencies when "
+                    "resolving imply_option at %s:%d" % (option.option, filename, line))
+
         if option_string:
             self._raw_options[option] = option_string
 
         when = self._conditions.get(option)
         # If `when` resolves to a false-ish value, we always return None.
         # This makes option(..., when='--foo') equivalent to
         # option(..., when=depends('--foo')(lambda x: x)).
         if when and not self._value_for(when) and value is not None:
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -782,16 +782,53 @@ class TestConfigure(unittest.TestCase):
                               "with '--without-foo' from the command-line")
 
             config = self.get_config(['--without-qux'])
             self.assertEquals(config, {
                 'FOO': NegativeOptionValue(),
                 'QUX': NegativeOptionValue(),
             })
 
+    def test_imply_option_recursion(self):
+        config_path = mozpath.abspath(
+            mozpath.join(test_data_path, 'moz.configure'))
+
+        message = ("'--without-foo' appears somewhere in the direct or indirect dependencies "
+                   "when resolving imply_option at %s:8" % config_path)
+
+        with self.moz_configure('''
+            option('--without-foo', help='foo')
+
+            imply_option('--with-qux', depends('--with-foo')(lambda x: x or None))
+
+            option('--with-qux', help='qux')
+
+            imply_option('--with-foo', depends('--with-qux')(lambda x: x or None))
+
+            set_config('FOO', depends('--with-foo')(lambda x: x))
+            set_config('QUX', depends('--with-qux')(lambda x: x))
+        '''):
+            # Note: no error is detected when the depends function in the
+            # imply_options resolve to None, which disables the imply_option.
+
+            with self.assertRaises(ConfigureError) as e:
+                config = self.get_config()
+
+            self.assertEquals(e.exception.message, message)
+
+            with self.assertRaises(ConfigureError) as e:
+                config = self.get_config(['--with-qux'])
+
+            self.assertEquals(e.exception.message, message)
+
+            with self.assertRaises(ConfigureError) as e:
+                config = self.get_config(['--without-foo', '--with-qux'])
+
+            self.assertEquals(e.exception.message, message)
+
     def test_option_failures(self):
         with self.assertRaises(ConfigureError) as e:
             with self.moz_configure('option("--with-foo", help="foo")'):
                 self.get_config()
 
         self.assertEquals(
             e.exception.message,
             'Option `--with-foo` is not handled ; reference it with a @depends'