Bug 1258615 - Remove the "magic" that sets a result from set_config() in @depends functions draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 22 Mar 2016 15:46:16 +0900
changeset 343296 eaa14a9edf6da986b5ea4891b831107358d1c470
parent 343258 2f1e15a63791878c61c7482d5e1ec8320aae76af
child 343314 3b68ecbe4a329c916c5d570a769cf3c6b94d5e5a
push id13580
push userbmo:mh+mozilla@glandium.org
push dateTue, 22 Mar 2016 07:33:05 +0000
bugs1258615
milestone48.0a1
Bug 1258615 - Remove the "magic" that sets a result from set_config() in @depends functions Currently, if a @depends function doesn't have a return statement or return None, a result is automatically set from all the set_config() called from the function. As we're going to move set_config to the global namespace, and as this feature is only used once, and it's only used for something that was written before ReadOnlyNamespace was exposed to the sandbox, we can "safely" get rid of it.
build/moz.configure/init.configure
build/moz.configure/old.configure
python/mozbuild/mozbuild/configure/__init__.py
python/mozbuild/mozbuild/test/configure/data/moz.configure
--- a/build/moz.configure/init.configure
+++ b/build/moz.configure/init.configure
@@ -11,26 +11,33 @@ option(env='DIST', nargs=1, help='DIST d
 # Do not allow objdir == srcdir builds.
 # ==============================================================
 @depends('--help', 'DIST')
 def check_build_environment(help, dist):
     topobjdir = os.path.realpath(os.path.abspath('.'))
     topsrcdir = os.path.realpath(os.path.abspath(
         os.path.join(os.path.dirname(__file__), '..', '..')))
 
+    if dist:
+        dist = normsep(dist[0])
+    else:
+        dist = os.path.join(topobjdir, 'dist')
+
+    result = namespace(
+        topsrcdir=topsrcdir,
+        topobjdir=topobjdir,
+        dist=dist,
+    )
     set_config('TOPSRCDIR', topsrcdir)
     set_config('TOPOBJDIR', topobjdir)
     set_config('MOZ_BUILD_ROOT', topobjdir)
-    if dist:
-        set_config('DIST', normsep(dist[0]))
-    else:
-        set_config('DIST', os.path.join(topobjdir, 'dist'))
+    set_config('DIST', dist)
 
     if help:
-        return
+        return result
 
     if topsrcdir == topobjdir:
         error(
             '  ***\n'
             '  * Building directly in the main source directory is not allowed.\n'
             '  *\n'
             '  * To build, you must run configure from a separate directory\n'
             '  * (referred to as an object directory).\n'
@@ -55,16 +62,18 @@ def check_build_environment(help, dist):
             '  *\n'
             '  *   To clean up the source tree:\n'
             '  *     1. cd %s\n'
             '  *     2. gmake distclean\n'
             '  ***'
             % ('\n  '.join(conflict_files), topsrcdir)
         )
 
+    return result
+
 
 option(env='OLD_CONFIGURE', nargs=1, help='Path to the old configure script')
 
 option(env='MOZ_CURRENT_PROJECT', nargs=1, help='Current build project')
 option(env='MOZCONFIG', nargs=1, help='Mozconfig location')
 
 # Read user mozconfig
 # ==============================================================
@@ -94,17 +103,17 @@ def mozconfig(current_project, mozconfig
     # Unfortunately, there is no direct way to tell whether the running
     # configure is the js configure. The indirect way is to look at the
     # OLD_CONFIGURE path, which points to js/src/old-configure.
     # I expect we'll have figured things out for mozconfigs well before
     # old-configure dies.
     if os.path.dirname(os.path.abspath(old_configure[0])).endswith('/js/src'):
         return {'path': None}
 
-    loader = MozconfigLoader(build_env['TOPSRCDIR'])
+    loader = MozconfigLoader(build_env.topsrcdir)
     current_project = current_project[0] if current_project else None
     mozconfig = mozconfig[0] if mozconfig else None
     mozconfig = loader.find_mozconfig(env={'MOZCONFIG': mozconfig})
     mozconfig = loader.read_mozconfig(mozconfig, moz_build_app=current_project)
 
     return mozconfig
 
 
@@ -160,17 +169,17 @@ def virtualenv_python(env_python, build_
         elif 'PYTHON' in mozconfig['env']['modified']:
             python = mozconfig['env']['modified']['PYTHON'][1]
         elif 'PYTHON' in mozconfig['vars']['added']:
             python = mozconfig['vars']['added']['PYTHON']
         elif 'PYTHON' in mozconfig['vars']['modified']:
             python = mozconfig['vars']['modified']['PYTHON'][1]
 
     verify_python_version(sys.stderr)
-    topsrcdir, topobjdir = build_env['TOPSRCDIR'], build_env['TOPOBJDIR']
+    topsrcdir, topobjdir = build_env.topsrcdir, build_env.topobjdir
     if topobjdir.endswith('/js/src'):
         topobjdir = topobjdir[:-7]
 
     manager = VirtualenvManager(
         topsrcdir, topobjdir,
         os.path.join(topobjdir, '_virtualenv'), sys.stdout,
         os.path.join(topsrcdir, 'build', 'virtualenv_packages.txt'))
 
@@ -544,63 +553,63 @@ option('--enable-application', nargs=1, 
 @depends('--enable-application', '--help')
 def application(app, help):
     if app:
         imply_option(app.format('--enable-project'))
 
 
 @depends(check_build_environment, '--help')
 def default_project(build_env, help):
-    if build_env['TOPOBJDIR'].endswith('/js/src'):
+    if build_env.topobjdir.endswith('/js/src'):
         return 'js'
     return 'browser'
 
 option('--enable-project', nargs=1, default=default_project,
        help='Project to build')
 
 option('--with-external-source-dir', env='EXTERNAL_SOURCE_DIR', nargs=1,
        help='External directory containing additional build files')
 
 @depends('--enable-project', '--with-external-source-dir',
          check_build_environment, '--help')
 def include_project_configure(project, external_source_dir, build_env, help):
     if not project:
         error('--enable-project is required.')
 
-    base_dir = build_env['TOPSRCDIR']
+    base_dir = build_env.topsrcdir
     if external_source_dir:
         set_config('EXTERNAL_SOURCE_DIR', external_source_dir[0])
         add_old_configure_assignment('EXTERNAL_SOURCE_DIR',
                                      external_source_dir[0])
         base_dir = os.path.join(base_dir, external_source_dir[0])
 
     path = os.path.join(base_dir, project[0], 'moz.configure')
     if not os.path.exists(path):
         error('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):
     ret = os.path.dirname(os.path.relpath(include_project_configure,
-                                          build_env['TOPSRCDIR']))
+                                          build_env.topsrcdir))
     set_config('MOZ_BUILD_APP', ret)
     set_define('MOZ_BUILD_APP', ret)
     add_old_configure_assignment('MOZ_BUILD_APP', ret)
     return ret
 
 
 # set RELEASE_BUILD and NIGHTLY_BUILD variables depending on the cycle we're in
 # The logic works like this:
 # - if we have "a1" in GRE_MILESTONE, we're building Nightly (define NIGHTLY_BUILD)
 # - otherwise, if we have "a" in GRE_MILESTONE, we're building Nightly or Aurora
 # - otherwise, we're building Release/Beta (define RELEASE_BUILD)
 @depends(check_build_environment)
 @advanced
 def milestone(build_env):
-    milestone_path = os.path.join(build_env['TOPSRCDIR'],
+    milestone_path = os.path.join(build_env.topsrcdir,
                                   'config',
                                   'milestone.txt')
     with open(milestone_path, 'r') as fh:
         milestone = fh.read().splitlines()[-1]
 
     set_config('GRE_MILESTONE', milestone)
 
     is_nightly = is_release = False
--- a/build/moz.configure/old.configure
+++ b/build/moz.configure/old.configure
@@ -102,17 +102,17 @@ def prepare_configure(old_configure, moz
         old_configure_dir = os.path.dirname(old_configure)
         if not old_configure_dir.endswith('/js/src'):
             old_configure = os.path.join(old_configure_dir, 'js', 'src',
                                          os.path.basename(old_configure))
 
     refresh = True
     if os.path.exists(old_configure):
         mtime = getmtime(old_configure)
-        aclocal = os.path.join(build_env['TOPSRCDIR'], 'build', 'autoconf',
+        aclocal = os.path.join(build_env.topsrcdir, 'build', 'autoconf',
                                '*.m4')
         for input in itertools.chain(
             (old_configure + '.in',
              os.path.join(os.path.dirname(old_configure), 'aclocal.m4')),
             glob.iglob(aclocal),
         ):
             if getmtime(input) > mtime:
                 break
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -121,17 +121,16 @@ class ConfigureSandbox(dict):
         self._seen = set()
 
         self._options = OrderedDict()
         # Store the raw values returned by @depends functions
         self._results = {}
         # Store several kind of information:
         # - value for each Option, as per returned by Option.get_value
         # - raw option (as per command line or environment) for each value
-        # - config set by each @depends function
         self._db = {}
 
         # Store options added with `imply_option`, and the reason they were
         # added (which can either have been given to `imply_option`, or
         # infered.
         self._implied_options = {}
 
         self._helper = CommandLineHelper(environ, argv)
@@ -224,17 +223,16 @@ class ConfigureSandbox(dict):
         if isinstance(arg, DummyFunction):
             assert arg in self._depends
             func = self._depends[arg]
             assert not inspect.isgeneratorfunction(func)
             assert func in self._results
             if not func.with_help:
                 raise ConfigureError("Missing @depends for `%s`: '--help'" %
                                      func.__name__)
-            self._seen.add(func)
             result = self._results[func]
             return result
         return arg
 
     def option_impl(self, *args, **kwargs):
         '''Implementation of option()
         This function creates and returns an Option() object, passing it the
         resolved arguments (uses the result of functions when functions are
@@ -306,28 +304,27 @@ class ConfigureSandbox(dict):
                     raise ConfigureError("Option must not contain an '='")
                 if name not in self._options:
                     raise ConfigureError("'%s' is not a known option. "
                                          "Maybe it's declared too late?"
                                          % arg)
                 arg = self._options[name]
                 if arg == self._help_option:
                     with_help = True
+                self._seen.add(arg)
+                assert arg in self._db or self._help
+                resolved_arg = self._db.get(arg)
             elif isinstance(arg, DummyFunction):
                 assert arg in self._depends
                 arg = self._depends[arg]
+                resolved_arg = self._results.get(arg)
             else:
                 raise TypeError(
                     "Cannot use object of type '%s' as argument to @depends"
                     % type(arg))
-            self._seen.add(arg)
-            resolved_arg = self._results.get(arg)
-            if resolved_arg is None:
-                assert arg in self._db or self._help
-                resolved_arg = self._db.get(arg)
             resolved_args.append(resolved_arg)
 
         def decorator(func):
             if inspect.isgeneratorfunction(func):
                 raise ConfigureError(
                     'Cannot decorate generator functions with @depends')
             func, glob = self._prepare_function(func)
             result = DependsOutput()
@@ -346,17 +343,16 @@ class ConfigureSandbox(dict):
                             "`%s` depends on '--help' and `%s`. "
                             "`%s` must depend on '--help'"
                             % (func.__name__, arg.__name__, arg.__name__))
 
             if self._help and not with_help:
                 return dummy
 
             self._results[func] = func(*resolved_args)
-            self._db[func] = ReadOnlyDict(result)
 
             for option, reason in result.implied_options:
                 self._helper.add(option, 'implied')
                 if not reason:
                     deps = []
                     for name, value in zip(args, resolved_args):
                         if not isinstance(value, OptionValue):
                             raise ConfigureError(
--- a/python/mozbuild/mozbuild/test/configure/data/moz.configure
+++ b/python/mozbuild/mozbuild/test/configure/data/moz.configure
@@ -50,37 +50,38 @@ def simple(simple):
 @depends('--enable-with-env')
 def with_env(with_env):
     set_config('WITH_ENV', with_env)
 
 # It doesn't matter if the dependency is on --enable or --disable
 @depends('--disable-values')
 def with_env2(values):
     set_config('VALUES', values)
+    return values
 
 # It is possible to @depends on environment-only options.
 @depends('CC')
 def is_gcc(cc):
     return cc and 'gcc' in cc[0]
 
 @depends(is_gcc)
 def is_gcc_check(is_gcc):
     set_config('IS_GCC', is_gcc)
 
 # It is possible to depend on the result from another function.
-# The input argument is a dict fed with the elements the function implied.
 @depends(with_env2)
 def with_env3(values):
-    set_config('VALUES2', values['VALUES'])
+    set_config('VALUES2', values)
+    return values
 
 # @depends functions can also return results for use as input to another
 # @depends.
 @depends(with_env3)
 def with_env4(values):
-    return values['VALUES2']
+    return values
 
 @depends(with_env4)
 def with_env5(values):
     set_config('VALUES3', values)
 
 # The result from @depends functions can also be used as input to options.
 # The result must be returned, not implied. The function must also depend
 # on --help.