Bug 1513134 - Detect unnecessary --help dependencies. r=firefox-build-system-reviewers,gps

Depends on D14125

Differential Revision: https://phabricator.services.mozilla.com/D14126

--HG--
extra : moz-landing-system : lando

build/moz.configure/init.configure

@@ -987,8 +987,8 @@ 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
 
@@ -996,8 +996,8 @@ def application(app, help):
 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'
@@ -1024,8 +1024,8 @@ def include_project_configure(project, external_source_dir, build_env, help):
     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
@@ -1209,8 +1209,8 @@ def project_flag(env=None, set_for_old_configure=False,
 # 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

build/moz.configure/toolchain.configure

@@ -191,8 +191,8 @@ add_old_configure_assignment('YASM', have_yasm)
 # ==============================================================
 
 
-@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')
 
 

build/moz.configure/warnings.configure

@@ -5,7 +5,7 @@
 # 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(

js/moz.configure

@@ -7,8 +7,8 @@
 # /!\ 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
@@ -258,8 +258,8 @@ def callgrind(value):
 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,
@@ -401,8 +401,8 @@ 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',
@@ -424,8 +424,8 @@ 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)

mobile/android/moz.configure

@@ -36,8 +36,8 @@ 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',

python/mozbuild/mozbuild/configure/lint.py

@@ -72,7 +72,7 @@ class LintSandbox(ConfigureSandbox):
         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:
@@ -82,12 +82,11 @@ class LintSandbox(ConfigureSandbox):
                         % (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:
@@ -108,6 +107,12 @@ class LintSandbox(ConfigureSandbox):
                     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

python/mozbuild/mozbuild/test/configure/test_lint.py

@@ -43,11 +43,29 @@ class TestLint(unittest.TestCase):
                 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')
@@ -57,6 +75,7 @@ class TestLint(unittest.TestCase):
                     return value
 
                 @depends('--help', foo)
+                @imports('os')
                 def bar(help, foo):
                     return foo
             '''):
@@ -79,6 +98,7 @@ class TestLint(unittest.TestCase):
                         return value
 
                     @depends('--help', foo)
+                    @imports('os')
                     def bar(help, foo):
                         return foo
                 tmpl()

toolkit/moz.configure

@@ -466,8 +466,8 @@ 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'
 
@@ -711,8 +711,8 @@ set_config('MOZ_SYNTH_SPEECHD',
 # ==============================================================
 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
 
@@ -1241,8 +1241,8 @@ 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
 
@@ -1258,8 +1258,8 @@ 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
 

toolkit/nss.configure

@@ -7,8 +7,8 @@
 
 # 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,