Bug 1322025 - Don't wrap the combination function in CombinedDependsFunction. r?chmanchester
Several things were wrong with the wrapping:
- the equality test on functions was actually comparing the memoized
functions, which have a type memoize, which inherits from dict. So
they weren't comparing actual functions, but the dict used to store
the cache of their invocation.
- each CombinedDependsFunction created for the same combination function
used a different wrapped function, so even if the dict problem wasn't
there, the equality test still wouldn't work, except if the function
wrapping itself was memoized.
- the memoization was not particularly useful.
Also, for upcoming changes, we'd actually like the combination function to
take an iterable instead of a variable argument list, so that items of
the iterable can be skipped.
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -103,54 +103,49 @@ class DependsFunction(object):
self.__class__.__name__,
self.name,
', '.join(repr(d) for d in self.dependencies),
)
class CombinedDependsFunction(DependsFunction):
def __init__(self, sandbox, func, dependencies):
- @memoize
- @wraps(func)
- def wrapper(*args):
- return func(args)
-
flatten_deps = []
for d in dependencies:
- if isinstance(d, CombinedDependsFunction) and d._func == wrapper:
+ if isinstance(d, CombinedDependsFunction) and d._func is func:
for d2 in d.dependencies:
if d2 not in flatten_deps:
flatten_deps.append(d2)
elif d not in flatten_deps:
flatten_deps.append(d)
# Automatically add a --help dependency if one of the dependencies
# depends on it.
for d in flatten_deps:
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)
+ sandbox, func, flatten_deps)
@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, need_help_dependency)
for d in deps]
- return self._func(*resolved_args)
+ return self._func(resolved_args)
def __eq__(self, other):
return (isinstance(other, self.__class__) and
- self._func == other._func and
+ self._func is other._func and
set(self.dependencies) == set(other.dependencies))
def __ne__(self, other):
return not self == other
class SandboxedGlobal(dict):
'''Identifiable dict type for use as function global'''
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -867,16 +867,35 @@ class TestConfigure(unittest.TestCase):
'''):
with self.assertRaises(ConfigureError) as e:
self.get_config()
self.assertEquals(e.exception.message,
'@depends function needs the same `when` as '
'options it depends on')
+ with self.moz_configure('''
+ @depends(when=True)
+ def always():
+ return True
+ @depends(when=True)
+ def always2():
+ return True
+ with only_when(always2):
+ option('--with-foo', help='foo', when=always)
+ # include() triggers resolution of its dependencies, and their
+ # side effects.
+ include(depends('--with-foo', when=always)(lambda x: x))
+ # The sandbox should figure that the `when` here is
+ # appropriate. Bad behavior in CombinedDependsFunction.__eq__
+ # made this fail in the past.
+ set_config('FOO', depends('--with-foo', when=always)(lambda x: x))
+ '''):
+ self.get_config()
+
def test_include_failures(self):
with self.assertRaises(ConfigureError) as e:
with self.moz_configure('include("../foo.configure")'):
self.get_config()
self.assertEquals(
e.exception.message,
'Cannot include `%s` because it is not in a subdirectory of `%s`'