Bug 1322025 - Enforce --help requirement on indirect dependencies. r=chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 25 Jan 2017 14:25:58 +0900
changeset 466999 6dee89eae3b2b77f2b2f05c4d979f825147198df
parent 466679 a79202aaab99375695d6f58f2334713659f6ee4f
child 467000 f9158bc3711f98a09d6537a73859d56890d7601e
push id43083
push userbmo:mh+mozilla@glandium.org
push dateFri, 27 Jan 2017 00:32:10 +0000
reviewerschmanchester
bugs1322025, 1313306
milestone54.0a1
Bug 1322025 - Enforce --help requirement on indirect dependencies. r=chmanchester Bug 1313306 relaxed the --help dependency requirement in some cases, but while doing so, the requirement was also removed in other, unexpected cases. Specifically, the --help dependency ended up not being required on indirect dependencies that should have had it, had the --help dependency been explicit on the direct dependency.
python/mozbuild/mozbuild/configure/__init__.py
python/mozbuild/mozbuild/test/configure/test_configure.py
python/mozbuild/mozbuild/test/configure/test_lint.py
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -26,17 +26,16 @@ from mozbuild.configure.help import Help
 from mozbuild.configure.util import (
     ConfigureOutputHandler,
     getpreferredencoding,
     LineIO,
 )
 from mozbuild.util import (
     exec_,
     memoize,
-    memoized_property,
     ReadOnlyDict,
     ReadOnlyNamespace,
 )
 
 import mozpack.path as mozpath
 
 
 class ConfigureError(Exception):
@@ -76,22 +75,24 @@ class DependsFunction(object):
 
     @property
     def sandboxed_dependencies(self):
         return [
             d.sandboxed if isinstance(d, DependsFunction) else d
             for d in self.dependencies
         ]
 
-    @memoized_property
-    def result(self):
-        if self.when and not self.sandbox._value_for(self.when):
+    @memoize
+    def result(self, need_help_dependency=False):
+        if self.when and not self.sandbox._value_for(self.when,
+                                                     need_help_dependency):
             return None
 
-        resolved_args = [self.sandbox._value_for(d) for d in self.dependencies]
+        resolved_args = [self.sandbox._value_for(d, need_help_dependency)
+                         for d in self.dependencies]
         return self.func(*resolved_args)
 
     def __repr__(self):
         return '<%s.%s %s(%s)>' % (
             self.__class__.__module__,
             self.__class__.__name__,
             self.name,
             ', '.join(repr(d) for d in self.dependencies),
@@ -120,23 +121,24 @@ class CombinedDependsFunction(DependsFun
             if (isinstance(d, DependsFunction) and
                 sandbox._help_option in d.dependencies):
                 flatten_deps.insert(0, sandbox._help_option)
                 break
 
         super(CombinedDependsFunction, self).__init__(
             sandbox, wrapper, flatten_deps)
 
-    @memoized_property
-    def result(self):
+    @memoize
+    def result(self, need_help_dependency=False):
         # Ignore --help for the combined result
         deps = self.dependencies
         if deps[0] == self.sandbox._help_option:
             deps = deps[1:]
-        resolved_args = [self.sandbox._value_for(d) for d in deps]
+        resolved_args = [self.sandbox._value_for(d, need_help_dependency)
+                         for d in deps]
         return self.func(*resolved_args)
 
     def __eq__(self, other):
         return (isinstance(other, self.__class__) and
                 self.func == other.func and
                 set(self.dependencies) == set(other.dependencies))
 
     def __ne__(self, other):
@@ -411,17 +413,17 @@ class ConfigureSandbox(dict):
         elif isinstance(obj, Option):
             return self._value_for_option(obj)
 
         assert False
 
     @memoize
     def _value_for_depends(self, obj, need_help_dependency=False):
         assert not inspect.isgeneratorfunction(obj.func)
-        return obj.result
+        return obj.result(need_help_dependency)
 
     @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)
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -347,17 +347,17 @@ class TestConfigure(unittest.TestCase):
                 def bar():
                     return sys
                 return bar
             bar = foo()'''),
             sandbox
         )
 
         with self.assertRaises(NameError) as e:
-            sandbox._depends[sandbox['bar']].result
+            sandbox._depends[sandbox['bar']].result()
 
         self.assertEquals(e.exception.message,
                           "global name 'sys' is not defined")
 
     def test_apply_imports(self):
         imports = []
 
         class CountApplyImportsSandbox(ConfigureSandbox):
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -109,16 +109,35 @@ class TestLint(unittest.TestCase):
 
                 include(foo)
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "Missing @depends for `foo`: '--help'")
 
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                @imports('os')
+                def foo(value):
+                    return value
+
+                @depends(foo)
+                def bar(value):
+                    return value
+
+                include(bar)
+            '''):
+                self.lint_test()
+
+        self.assertEquals(e.exception.message,
+                          "Missing @depends for `foo`: '--help'")
+
         # There is a default restricted `os` module when there is no explicit
         # @imports, and it's fine to use it without a dependency on --help.
         with self.moz_configure('''
             option('--foo', help='foo')
             @depends('--foo')
             def foo(value):
                 os
                 return value