Bug 1313306 - Move --help dependency checks to the linter. r?chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 27 Oct 2016 10:02:21 +0900
changeset 430143 d3f548b9453b9c15ae854d0511a2a48521eefde0
parent 430142 f71785cdb341f241e1683bf54a780d188dc70579
child 430144 711322281e6f7b85c38c53d2607f017a56556b03
push id33750
push userbmo:mh+mozilla@glandium.org
push dateThu, 27 Oct 2016 07:43:21 +0000
reviewerschmanchester
bugs1313306
milestone52.0a1
Bug 1313306 - Move --help dependency checks to the linter. r?chmanchester Missing such dependencies shouldn't impair running configure itself after local modifications, but they are currently required for (mostly) documentation purpose. Which means they are better done in the linter.
python/moz.build
python/mozbuild/mozbuild/configure/__init__.py
python/mozbuild/mozbuild/configure/lint.py
python/mozbuild/mozbuild/test/configure/test_configure.py
python/mozbuild/mozbuild/test/configure/test_lint.py
--- a/python/moz.build
+++ b/python/moz.build
@@ -36,16 +36,17 @@ PYTHON_UNIT_TESTS += [
     'mozbuild/mozbuild/test/backend/test_configenvironment.py',
     'mozbuild/mozbuild/test/backend/test_recursivemake.py',
     'mozbuild/mozbuild/test/backend/test_visualstudio.py',
     'mozbuild/mozbuild/test/compilation/test_warnings.py',
     'mozbuild/mozbuild/test/configure/lint.py',
     'mozbuild/mozbuild/test/configure/test_checks_configure.py',
     'mozbuild/mozbuild/test/configure/test_compile_checks.py',
     'mozbuild/mozbuild/test/configure/test_configure.py',
+    'mozbuild/mozbuild/test/configure/test_lint.py',
     'mozbuild/mozbuild/test/configure/test_moz_configure.py',
     'mozbuild/mozbuild/test/configure/test_options.py',
     'mozbuild/mozbuild/test/configure/test_toolchain_configure.py',
     'mozbuild/mozbuild/test/configure/test_toolchain_helpers.py',
     'mozbuild/mozbuild/test/configure/test_toolkit_moz_configure.py',
     'mozbuild/mozbuild/test/configure/test_util.py',
     'mozbuild/mozbuild/test/controller/test_ccachestats.py',
     'mozbuild/mozbuild/test/controller/test_clobber.py',
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -405,28 +405,16 @@ 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)
-        with_help = self._help_option in obj.dependencies
-        if with_help:
-            for arg in obj.dependencies:
-                if isinstance(arg, DependsFunction):
-                    if self._help_option not in arg.dependencies:
-                        raise ConfigureError(
-                            "`%s` depends on '--help' and `%s`. "
-                            "`%s` must depend on '--help'"
-                            % (obj.name, arg.name, arg.name))
-        elif self._help or need_help_dependency:
-            raise ConfigureError("Missing @depends for `%s`: '--help'" %
-                                 obj.name)
         return obj.result
 
     @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
--- a/python/mozbuild/mozbuild/configure/lint.py
+++ b/python/mozbuild/mozbuild/configure/lint.py
@@ -3,26 +3,48 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import, print_function, unicode_literals
 
 import inspect
 from StringIO import StringIO
 from collections import defaultdict
 from contextlib import contextmanager
-from . import ConfigureSandbox
+from . import (
+    ConfigureError,
+    ConfigureSandbox,
+    DependsFunction,
+)
+from mozbuild.util import memoize
 
 
 class LintSandbox(ConfigureSandbox):
     def __init__(self, environ=None, argv=None):
         out = StringIO()
         environ = environ or {}
         argv = argv or []
         super(LintSandbox, self).__init__({}, environ=environ, argv=argv,
                                           stdout=out, stderr=out)
 
     def run(self, path=None):
         if path:
             self.include_file(path)
 
+    @memoize
+    def _value_for_depends(self, obj, need_help_dependency=False):
+        with_help = self._help_option in obj.dependencies
+        if with_help:
+            for arg in obj.dependencies:
+                if isinstance(arg, DependsFunction):
+                    if self._help_option not in arg.dependencies:
+                        raise ConfigureError(
+                            "`%s` depends on '--help' and `%s`. "
+                            "`%s` must depend on '--help'"
+                            % (obj.name, arg.name, arg.name))
+        elif self._help or need_help_dependency:
+            raise ConfigureError("Missing @depends for `%s`: '--help'" %
+                                 obj.name)
+        return super(LintSandbox, self)._value_for_depends(
+            obj, need_help_dependency)
+
     @contextmanager
     def only_when_impl(self, when):
         yield
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -1059,63 +1059,32 @@ class TestConfigure(unittest.TestCase):
                 def foo(value):
                     yield
             '''):
                 self.get_config()
 
         self.assertEquals(e.exception.message,
                           "Cannot decorate generator functions with @depends")
 
-        with self.assertRaises(ConfigureError) as e:
-            with self.moz_configure('''
-                option('--foo', help='foo')
-                @depends('--foo')
-                def foo(value):
-                    return value
-
-                @depends('--help', foo)
-                def bar(help, foo):
-                    return
-            '''):
-                self.get_config()
-
-        self.assertEquals(e.exception.message,
-                          "`bar` depends on '--help' and `foo`. "
-                          "`foo` must depend on '--help'")
-
         with self.assertRaises(TypeError) as e:
             with self.moz_configure('''
                 depends('--help')(42)
             '''):
                 self.get_config()
 
         self.assertEquals(e.exception.message,
                           "Unexpected type: 'int'")
 
         with self.assertRaises(ConfigureError) as e:
             with self.moz_configure('''
                 option('--foo', help='foo')
                 @depends('--foo')
                 def foo(value):
                     return value
 
-                include(foo)
-            '''):
-                self.get_config()
-
-        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')
-                def foo(value):
-                    return value
-
                 foo()
             '''):
                 self.get_config()
 
         self.assertEquals(e.exception.message,
                           "The `foo` function may not be called")
 
         with self.assertRaises(TypeError) as e:
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -0,0 +1,72 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+from StringIO import StringIO
+import os
+import textwrap
+import unittest
+
+from mozunit import (
+    main,
+    MockedOpen,
+)
+
+from mozbuild.configure import ConfigureError
+from mozbuild.configure.lint import LintSandbox
+
+import mozpack.path as mozpath
+
+test_data_path = mozpath.abspath(mozpath.dirname(__file__))
+test_data_path = mozpath.join(test_data_path, 'data')
+
+
+class TestLint(unittest.TestCase):
+    def lint_test(self, options=[], env={}):
+        sandbox = LintSandbox(env, ['configure'] + options)
+
+        sandbox.run(mozpath.join(test_data_path, 'moz.configure'))
+
+    def moz_configure(self, source):
+        return MockedOpen({
+            os.path.join(test_data_path,
+                         'moz.configure'): textwrap.dedent(source)
+        })
+
+    def test_depends_failures(self):
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                def foo(value):
+                    return value
+
+                @depends('--help', foo)
+                def bar(help, foo):
+                    return
+            '''):
+                self.lint_test()
+
+        self.assertEquals(e.exception.message,
+                          "`bar` depends on '--help' and `foo`. "
+                          "`foo` must depend on '--help'")
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
+                def foo(value):
+                    return value
+
+                include(foo)
+            '''):
+                self.lint_test()
+
+        self.assertEquals(e.exception.message,
+                          "Missing @depends for `foo`: '--help'")
+
+
+if __name__ == '__main__':
+    main()