Bug 1363585 - Forbid boolean operations on @depends functions. r?chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 10 May 2017 11:35:22 +0900
changeset 575165 15ee34935cfbf4ae66614b1a4738eb8d601e486e
parent 575161 27916ed0f76d73ebfcfb2cf0f06bb8f10d2decef
child 627843 a61e8a1b4239ba4132cfdf9a5d28a21a227ee051
push id57978
push userbmo:mh+mozilla@glandium.org
push dateWed, 10 May 2017 02:37:39 +0000
reviewerschmanchester
bugs1363585
milestone55.0a1
Bug 1363585 - Forbid boolean operations on @depends functions. r?chmanchester Doing something like "not foo" when foo is a @depends function is never going to do what the user expects, while not necessarily leading to an error (like, when used in set_config). It is better to have an error in those cases where it's expected not to work, at the expense of making templates a little more verbose, rather than silently do something the user is not expecting.
build/moz.configure/checks.configure
build/moz.configure/compile-checks.configure
build/moz.configure/toolchain.configure
python/mozbuild/mozbuild/configure/__init__.py
--- a/build/moz.configure/checks.configure
+++ b/build/moz.configure/checks.configure
@@ -90,17 +90,17 @@ def checking(what, callback=None):
 #   check_prog('PROG', ('a', 'b'))
 # This will look for 'a' or 'b' in $PATH, and set_config PROG to the one
 # it can find. If PROG is already set from the environment or command line,
 # use that value instead.
 @template
 @imports(_from='mozbuild.shellutil', _import='quote')
 def check_prog(var, progs, what=None, input=None, allow_missing=False,
                paths=None):
-    if input:
+    if input is not None:
         # Wrap input with type checking and normalization.
         @depends(input)
         def input(value):
             if not value:
                 return
             if isinstance(value, str):
                 return (value,)
             if isinstance(value, (tuple, list)) and len(value) == 1:
--- a/build/moz.configure/compile-checks.configure
+++ b/build/moz.configure/compile-checks.configure
@@ -40,17 +40,18 @@ def try_compile(includes=None, body='', 
 #   used.
 # - `flags` are the flags to be passed to the compiler, in addition to `-c`.
 # - `includes` are additional includes, as file names, to appear before the
 #   header checked for.
 # - `when` is a depends function that if present will make performing the check
 #   conditional on the value of that function.
 @template
 def check_header(header, language='C++', flags=None, includes=None, when=None):
-    when = when or always
+    if when is None:
+        when = always
 
     if includes:
         includes = includes[:]
     else:
         includes = []
     includes.append(header)
 
     have_header = try_compile(includes=includes, language=language, flags=flags,
@@ -92,22 +93,23 @@ def warnings_cxxflags():
 #   are tested.
 # - `when` (optional) is a @depends function or option name conditioning
 #   when the warning flag is wanted.
 # - `check`, when not set, skips checking whether the flag is supported and
 #   adds it to the list of warning flags unconditionally. This is only meant
 #   for add_gcc_warning().
 @template
 def check_and_add_gcc_warning(warning, compiler=None, when=None, check=True):
-    if compiler:
+    if compiler is not None:
         compilers = (compiler,)
     else:
         compilers = (c_compiler, cxx_compiler)
 
-    when = when or always
+    if when is None:
+        when = always
 
     for c in compilers:
         assert c in (c_compiler, cxx_compiler)
         lang, warnings_flags = {
             c_compiler: ('C', warnings_cflags),
             cxx_compiler: ('C++', warnings_cxxflags),
         }[c]
 
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -560,19 +560,20 @@ def compiler(language, host_or_target, c
     When `host_or_target` is `host`, `other_compiler` is the result of the
     `compiler` template for the same `language` for `target`.
     When `host_or_target` is `host` and the language is 'C++',
     `other_c_compiler` is the result of the `compiler` template for the
     language 'C' for `target`.
     '''
     assert host_or_target in (host, target)
     assert language in ('C', 'C++')
-    assert language == 'C' or c_compiler
-    assert host_or_target == target or other_compiler
-    assert language == 'C' or host_or_target == target or other_c_compiler
+    assert language == 'C' or c_compiler is not None
+    assert host_or_target == target or other_compiler is not None
+    assert language == 'C' or host_or_target == target or \
+        other_c_compiler is not None
 
     host_or_target_str = {
         host: 'host',
         target: 'target',
     }[host_or_target]
 
     var = {
         ('C', target): 'CC',
@@ -614,17 +615,20 @@ def compiler(language, host_or_target, c
     # Derive the host compiler from the corresponding target compiler when no
     # explicit compiler was given and we're not cross compiling. For the C++
     # compiler, though, prefer to derive from the host C compiler when it
     # doesn't match the target C compiler.
     # As a special case, since clang supports all kinds of targets in the same
     # executable, when cross compiling with clang, default to the same compiler
     # as the target compiler, resetting flags.
     if host_or_target == host:
-        args = (c_compiler, other_c_compiler) if other_c_compiler else ()
+        if other_c_compiler is not None:
+            args = (c_compiler, other_c_compiler)
+        else:
+            args = ()
 
         @depends(provided_compiler, other_compiler, cross_compiling, *args)
         def provided_compiler(value, other_compiler, cross_compiling, *args):
             if value:
                 return value
             c_compiler, other_c_compiler = args if args else (None, None)
             if not cross_compiling and c_compiler == other_c_compiler:
                 return other_compiler
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -52,16 +52,20 @@ class SandboxDependsFunction(object):
                              % self.__name__)
 
     def __or__(self, other):
         if not isinstance(other, SandboxDependsFunction):
             raise ConfigureError('Can only do binary arithmetic operations '
                                  'with another @depends function.')
         return self._or(other).sandboxed
 
+    def __nonzero__(self):
+        raise ConfigureError(
+            'Cannot do boolean operations on @depends functions.')
+
 
 class DependsFunction(object):
     __slots__ = (
         '_func', '_name', 'dependencies', 'when', 'sandboxed', 'sandbox',
         '_result')
 
     def __init__(self, sandbox, func, dependencies, when=None):
         assert isinstance(sandbox, ConfigureSandbox)