Bug 1513134 - Detect unnecessary --help dependencies. r=firefox-build-system-reviewers,gps
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 11 Dec 2018 19:34:28 +0000
changeset 507217 0f7e4ff45912953576a5c3420cc0ac94abe0ca5b
parent 507216 bcbedb8171376816a4b4ee399d4bffe197e42aae
child 507218 1cf8fafae8d8c5379247c3c24fa316e3a5101fef
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfirefox-build-system-reviewers, gps
bugs1513134
milestone66.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 1513134 - Detect unnecessary --help dependencies. r=firefox-build-system-reviewers,gps Depends on D14125 Differential Revision: https://phabricator.services.mozilla.com/D14126
build/moz.configure/init.configure
build/moz.configure/toolchain.configure
build/moz.configure/warnings.configure
js/moz.configure
mobile/android/moz.configure
python/mozbuild/mozbuild/configure/lint.py
python/mozbuild/mozbuild/test/configure/test_lint.py
toolkit/moz.configure
toolkit/nss.configure
--- a/build/moz.configure/init.configure
+++ b/build/moz.configure/init.configure
@@ -982,27 +982,27 @@ def target_is_solaris(target):
 set_define('XP_SOLARIS', target_is_solaris)
 
 # The application/project to build
 # ==============================================================
 option('--enable-application', nargs=1, env='MOZ_BUILD_APP',
        help='Application to build. Same as --enable-project.')
 
 
-@depends('--enable-application', '--help')
-def application(app, help):
+@depends('--enable-application')
+def application(app):
     if app:
         return app
 
 
 imply_option('--enable-project', application)
 
 
-@depends(check_build_environment, '--help')
-def default_project(build_env, help):
+@depends(check_build_environment)
+def default_project(build_env):
     if build_env.topobjdir.endswith('/js/src'):
         return 'js'
     return 'browser'
 
 
 option('--enable-project', nargs=1, default=default_project,
        help='Project to build')
 
@@ -1019,18 +1019,18 @@ def include_project_configure(project, e
         base_dir = os.path.join(base_dir, external_source_dir[0])
 
     path = os.path.join(base_dir, project[0], 'moz.configure')
     if not exists(path):
         die('Cannot find project %s', project[0])
     return path
 
 
-@depends(include_project_configure, check_build_environment, '--help')
-def build_project(include_project_configure, build_env, help):
+@depends(include_project_configure, check_build_environment)
+def build_project(include_project_configure, build_env):
     ret = os.path.dirname(os.path.relpath(include_project_configure,
                                           build_env.topsrcdir))
     return ret
 
 
 set_config('MOZ_BUILD_APP', build_project)
 set_define('MOZ_BUILD_APP', build_project)
 add_old_configure_assignment('MOZ_BUILD_APP', build_project)
@@ -1204,18 +1204,18 @@ def project_flag(env=None, set_for_old_c
     if set_as_define:
         set_define(env, option_implementation)
     if set_for_old_configure:
         add_old_configure_assignment(env, option_implementation)
 
 # milestone.is_nightly corresponds to cases NIGHTLY_BUILD is set.
 
 
-@depends(milestone, '--help')
-def enabled_in_nightly(milestone, _):
+@depends(milestone)
+def enabled_in_nightly(milestone):
     return milestone.is_nightly
 
 # Set the MOZ_CONFIGURE_OPTIONS variable with all the options that
 # were passed somehow (environment, command line, mozconfig)
 
 
 @dependable
 @imports(_from='mozbuild.shellutil', _import='quote')
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -186,18 +186,18 @@ set_config('HAVE_NASM', have_nasm)
 set_config('HAVE_YASM', have_yasm)
 # Until the YASM variable is not necessary in old-configure.
 add_old_configure_assignment('YASM', have_yasm)
 
 # Android NDK
 # ==============================================================
 
 
-@depends('--disable-compile-environment', build_project, '--help')
-def compiling_android(compile_env, build_project, _):
+@depends('--disable-compile-environment', build_project)
+def compiling_android(compile_env, build_project):
     return compile_env and build_project in ('mobile/android', 'js')
 
 
 include('android-ndk.configure', when=compiling_android)
 
 # MacOS deployment target version
 # ==============================================================
 # This needs to happen before any compilation test is done.
--- a/build/moz.configure/warnings.configure
+++ b/build/moz.configure/warnings.configure
@@ -1,16 +1,16 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 js_option('--enable-warnings-as-errors', env='MOZ_ENABLE_WARNINGS_AS_ERRORS',
-          default=depends('MOZ_AUTOMATION', '--help')(lambda x, _: bool(x)),
+          default=depends('MOZ_AUTOMATION')(lambda x: bool(x)),
           help='{Enable|Disable} treating warnings as errors')
 
 add_old_configure_assignment(
     'MOZ_ENABLE_WARNINGS_AS_ERRORS',
     depends('--enable-warnings-as-errors')(lambda x: bool(x)))
 
 
 # GCC/Clang warnings:
--- a/js/moz.configure
+++ b/js/moz.configure
@@ -2,18 +2,18 @@
 # vim: set filetype=python:
 # 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/.
 
 # /!\ Use js_option() instead of option() in this file. /!\
 # =========================================================
 
-@depends(build_project, '--help')
-def building_js(build_project, help):
+@depends(build_project)
+def building_js(build_project):
     return build_project == 'js'
 
 # Exception to the rule above: JS_STANDALONE is a special option that doesn't
 # want the js_option treatment. When we're done merging js/src/configure and
 # top-level configure, it can go away, although the JS_STANDALONE config
 # will still need to be set depending on building_js above.
 option(env='JS_STANDALONE', default=building_js,
        help='Reserved for internal use')
@@ -253,18 +253,18 @@ js_option('--enable-callgrind', env='MOZ
 @depends('--enable-callgrind')
 def callgrind(value):
     if value:
         return True
 
 set_define('MOZ_CALLGRIND', callgrind)
 imply_option('--enable-profiling', callgrind)
 
-@depends(milestone, '--help')
-def enable_profiling(milestone, help):
+@depends(milestone)
+def enable_profiling(milestone):
     return milestone.is_nightly
 
 js_option('--enable-profiling', env='MOZ_PROFILING', default=enable_profiling,
           help='{Set|Do not set} compile flags necessary for using sampling '
                 'profilers (e.g. shark, perf)')
 
 @depends('--enable-profiling')
 def profiling(value):
@@ -396,18 +396,18 @@ def more_deterministic(value):
     if value:
         return True
 
 set_define('JS_MORE_DETERMINISTIC', more_deterministic)
 
 
 # CTypes
 # =======================================================
-@depends(building_js, '--help')
-def ctypes_default(building_js, _):
+@depends(building_js)
+def ctypes_default(building_js):
     return not building_js
 
 js_option('--enable-ctypes',
           default=ctypes_default,
           help='{Enable|Disable} js-ctypes')
 
 build_ctypes = depends_if('--enable-ctypes')(lambda _: True)
 
@@ -419,18 +419,18 @@ add_old_configure_assignment('BUILD_CTYP
 def js_has_ctypes(ctypes, js):
     if ctypes and js:
         return True
 
 set_config('JS_HAS_CTYPES', js_has_ctypes)
 set_define('JS_HAS_CTYPES', js_has_ctypes)
 add_old_configure_assignment('JS_HAS_CTYPES', js_has_ctypes)
 
-@depends('--enable-ctypes', '--enable-compile-environment', '--help')
-def ctypes_and_compile_environment(ctypes, compile_environment, _):
+@depends('--enable-ctypes', '--enable-compile-environment')
+def ctypes_and_compile_environment(ctypes, compile_environment):
     return ctypes and compile_environment
 
 include('ffi.configure', when=ctypes_and_compile_environment)
 
 
 # Support various fuzzing options
 # ==============================================================
 with only_when('--enable-compile-environment'):
--- a/mobile/android/moz.configure
+++ b/mobile/android/moz.configure
@@ -31,18 +31,18 @@ option(env='MOZ_NATIVE_DEVICES',
        default=google_play_services)
 
 set_config('MOZ_NATIVE_DEVICES',
            depends_if('MOZ_NATIVE_DEVICES')(lambda _: True))
 add_old_configure_assignment('MOZ_NATIVE_DEVICES',
                              depends_if('MOZ_NATIVE_DEVICES')(lambda _: True))
 
 # Enable install tracking SDK if we have Google Play support.
-@depends(milestone, google_play_services, '--help')
-def install_tracking_default(milestone, google_play_services, help):
+@depends(milestone, google_play_services)
+def install_tracking_default(milestone, google_play_services):
     return bool(milestone.is_release_or_beta and google_play_services)
 
 option(env='MOZ_INSTALL_TRACKING',
        help='Enable install tracking (currently using the Adjust SDK).',
        default=install_tracking_default)
 
 set_config('MOZ_INSTALL_TRACKING',
            depends_if('MOZ_INSTALL_TRACKING')(lambda _: True))
--- a/python/mozbuild/mozbuild/configure/lint.py
+++ b/python/mozbuild/mozbuild/configure/lint.py
@@ -67,32 +67,31 @@ class LintSandbox(ConfigureSandbox):
         for op, arg in disassemble_as_iter(func):
             if op in ('LOAD_FAST', 'LOAD_CLOSURE'):
                 if arg in all_args:
                     used_args.add(arg)
 
         for num, arg in enumerate(all_args):
             if arg not in used_args:
                 dep = obj.dependencies[num]
-                if dep != self._help_option:
+                if dep != self._help_option or not self._need_help_dependency(obj):
                     if isinstance(dep, DependsFunction):
                         dep = dep.name
                     else:
                         dep = dep.option
                     raise ConfigureError(
                         '%s: The dependency on `%s` is unused.'
                         % (loc, dep)
                     )
 
-    def _missing_help_dependency(self, obj):
+    def _need_help_dependency(self, obj):
         if isinstance(obj, (CombinedDependsFunction, TrivialDependsFunction)):
             return False
         if isinstance(obj, DependsFunction):
-            if (self._help_option in obj.dependencies or
-                obj in (self._always, self._never)):
+            if obj in (self._always, self._never):
                 return False
             func, glob = self.unwrap(obj._func)
             # We allow missing --help dependencies for functions that:
             # - don't use @imports
             # - don't have a closure
             # - don't use global variables
             if func in self._has_imports or func.func_closure:
                 return True
@@ -103,16 +102,22 @@ class LintSandbox(ConfigureSandbox):
                     # dependency.
                     if arg == 'os' and glob.get('os') is self.OS:
                         continue
                     if arg in self.BUILTINS:
                         continue
                     return True
         return False
 
+    def _missing_help_dependency(self, obj):
+        if (isinstance(obj, DependsFunction) and
+                self._help_option in obj.dependencies):
+            return False
+        return self._need_help_dependency(obj)
+
     @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 self._missing_help_dependency(arg):
                     raise ConfigureError(
                         "`%s` depends on '--help' and `%s`. "
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -38,30 +38,49 @@ class TestLint(unittest.TestCase):
     def test_depends_failures(self):
         with self.moz_configure('''
             option('--foo', help='foo')
             @depends('--foo')
             def foo(value):
                 return value
 
             @depends('--help', foo)
+            @imports('os')
             def bar(help, foo):
                 return foo
         '''):
             self.lint_test()
 
         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 foo
+            '''):
+                self.lint_test()
+
+        self.assertEquals(e.exception.message,
+                          "%s:7: The dependency on `--help` is unused."
+                          % mozpath.join(test_data_path, 'moz.configure'))
+
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--foo', help='foo')
+                @depends('--foo')
                 @imports('os')
                 def foo(value):
                     return value
 
                 @depends('--help', foo)
+                @imports('os')
                 def bar(help, foo):
                     return foo
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "`bar` depends on '--help' and `foo`. "
                           "`foo` must depend on '--help'")
@@ -74,16 +93,17 @@ class TestLint(unittest.TestCase):
 
                     option('--foo', help='foo')
                     @depends('--foo')
                     def foo(value):
                         qux
                         return value
 
                     @depends('--help', foo)
+                    @imports('os')
                     def bar(help, foo):
                         return foo
                 tmpl()
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "`bar` depends on '--help' and `foo`. "
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -461,18 +461,18 @@ def openmax(value):
         return True
 
 set_config('MOZ_OMX', openmax)
 set_define('MOZ_OMX', openmax)
 
 # EME Support
 # ==============================================================
 # Widevine is enabled by default in desktop browser builds.
-@depends(build_project, '--help')
-def eme_default(build_project, help):
+@depends(build_project)
+def eme_default(build_project):
     if build_project == 'browser':
         return 'widevine'
 
 option('--enable-eme',
        nargs='*',
        choices=('widevine',),
        default=eme_default,
        help='{Enable|Disable} support for Encrypted Media Extensions')
@@ -706,18 +706,18 @@ option('--disable-synth-speechd', help='
 
 set_config('MOZ_SYNTH_SPEECHD',
            depends_if('--disable-synth-speechd')(lambda _: True))
 
 # Speech API
 # ==============================================================
 option('--disable-webspeech', help='Disable support for HTML Speech API')
 
-@depends('--disable-webspeech', '--help')
-def webspeech(value, _):
+@depends('--disable-webspeech')
+def webspeech(value):
     if value:
         return True
 
 set_config('MOZ_WEBSPEECH', webspeech)
 set_define('MOZ_WEBSPEECH', webspeech)
 add_old_configure_assignment('MOZ_WEBSPEECH', webspeech)
 
 # Speech API test backend
@@ -1236,35 +1236,35 @@ set_config('MOZ_VERIFY_MAR_SIGNATURE',
 option('--enable-tasktracer', help='Enable TaskTracer')
 
 set_define('MOZ_TASK_TRACER', depends_if('--enable-tasktracer')(lambda _: True))
 set_config('MOZ_TASK_TRACER', depends_if('--enable-tasktracer')(lambda _: True))
 
 # Reflow counting
 # ==============================================================
 
-@depends(moz_debug, '--help')
-def reflow_perf(debug, _):
+@depends(moz_debug)
+def reflow_perf(debug):
     if debug:
         return True
 
 option('--enable-reflow-perf',
        default=reflow_perf,
        help='{Enable|Disable} reflow performance tracing')
 
 # The difference in conditions here comes from the initial implementation
 # in old-configure, which was unexplained there as well.
 set_define('MOZ_REFLOW_PERF', depends_if('--enable-reflow-perf')(lambda _: True))
 set_define('MOZ_REFLOW_PERF_DSP', reflow_perf)
 
 # Layout debugger
 # ==============================================================
 
-@depends(moz_debug, '--help')
-def layout_debugger(debug, _):
+@depends(moz_debug)
+def layout_debugger(debug):
     if debug:
         return True
 
 option('--enable-layout-debugger',
        default=layout_debugger,
        help='{Enable|Disable} layout debugger')
 
 set_config('MOZ_LAYOUT_DEBUGGER', depends_if('--enable-layout-debugger')(lambda _: True))
--- a/toolkit/nss.configure
+++ b/toolkit/nss.configure
@@ -2,16 +2,16 @@
 # vim: set filetype=python:
 # 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/.
 
 
 # DBM support in NSS
 # ==============================================================
-@depends(build_project, '--help')
-def dbm_default(build_project, _):
+@depends(build_project)
+def dbm_default(build_project):
     return build_project != 'mobile/android'
 
 option('--enable-dbm', default=dbm_default,
        help='{Enable|Disable} building DBM')
 
 set_config('NSS_DISABLE_DBM', depends('--enable-dbm')(lambda x: not x))