Bug 1282256 - Avoid loading mozconfig in MozbuildObject.from_environment. r?gps draft
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 04 Aug 2016 11:17:41 +0900
changeset 396566 7124d823190c0ca98af03a5a30ccf40ffe150099
parent 396454 de6fd5bb93d26985b6b7eb8b1e2ec896b8e7611a
child 396569 6474eb370154e1f7976a984d7a7f30a41c4cee58
push id25052
push userbmo:mh+mozilla@glandium.org
push dateThu, 04 Aug 2016 04:20:17 +0000
reviewersgps
bugs1282256, 1278415
milestone51.0a1
Bug 1282256 - Avoid loading mozconfig in MozbuildObject.from_environment. r?gps We've been reading the mozconfig in MozbuildObject.from_environment to check whether the mozconfig topobjdir matches the detected topobjdir. Since bug 1278415, everything using the buildconfig python module now calls MozbuildObject.from_environment, which reads the mozconfig. A lot of things to that during the build. But none of them actually need the data from the mozconfig, and the topobjdir match test has been breaking things randomly on multiple occasions. The topobjdir match test, however, really only needs to happen once: when a mach command starts. So we can move the test to MachCommandBase, where it belongs, and anything actively using MozbuildObject.mozconfig will have the mozconfig read, but everything else won't. On a Linux64 opt build, this brings down the number of times the mozconfig is read during `mach build` from 979 to 9.
python/mozbuild/mozbuild/base.py
python/mozbuild/mozbuild/test/test_base.py
--- a/python/mozbuild/mozbuild/base.py
+++ b/python/mozbuild/mozbuild/base.py
@@ -154,52 +154,27 @@ class MozbuildObject(ProcessExecutionMix
             topsrcdir, topobjdir, mozconfig = load_mozinfo(mozinfo_path)
 
         # If we were successful, we're only guaranteed to find a topsrcdir. If
         # we couldn't find that, there's nothing we can do.
         if not topsrcdir:
             raise BuildEnvironmentNotFoundException(
                 'Could not find Mozilla source tree or build environment.')
 
-        # Now we try to load the config for this environment. If mozconfig is
-        # None, read_mozconfig() will attempt to find one in the existing
-        # environment. If no mozconfig is present, the config will not have
-        # much defined.
-        loader = MozconfigLoader(topsrcdir)
-        current_project = os.environ.get('MOZ_CURRENT_PROJECT')
-        config = loader.read_mozconfig(mozconfig, moz_build_app=current_project)
-
-        config_topobjdir = MozbuildObject.resolve_mozconfig_topobjdir(
-            topsrcdir, config)
-
-        # If we're inside a objdir and the found mozconfig resolves to
-        # another objdir, we abort. The reasoning here is that if you are
-        # inside an objdir you probably want to perform actions on that objdir,
-        # not another one. This prevents accidental usage of the wrong objdir
-        # when the current objdir is ambiguous.
-        if topobjdir and config_topobjdir:
-            if current_project:
-                config_topobjdir = os.path.join(config_topobjdir, current_project)
-
-            _config_topobjdir = config_topobjdir
-            if not samepath(topobjdir, _config_topobjdir):
-                raise ObjdirMismatchException(topobjdir, _config_topobjdir)
-
         topsrcdir = mozpath.normsep(topsrcdir)
-        topobjdir = topobjdir or config_topobjdir
         if topobjdir:
             topobjdir = mozpath.normsep(os.path.normpath(topobjdir))
 
             if topsrcdir == topobjdir:
                 raise BadEnvironmentException('The object directory appears '
                     'to be the same as your source directory (%s). This build '
                     'configuration is not supported.' % topsrcdir)
 
-        # If we can't resolve topobjdir, oh well. The constructor will figure
-        # it out via config.guess.
+        # If we can't resolve topobjdir, oh well. We'll figure out when we need
+        # one.
         return cls(topsrcdir, None, None, topobjdir=topobjdir,
                    mozconfig=mozconfig)
 
     @staticmethod
     def resolve_mozconfig_topobjdir(topsrcdir, mozconfig, default=None):
         topobjdir = mozconfig['topobjdir'] or default
         if not topobjdir:
             return None
@@ -232,17 +207,17 @@ class MozbuildObject(ProcessExecutionMix
         return self._virtualenv_manager
 
     @property
     def mozconfig(self):
         """Returns information about the current mozconfig file.
 
         This a dict as returned by MozconfigLoader.read_mozconfig()
         """
-        if self._mozconfig is MozconfigLoader.AUTODETECT:
+        if not isinstance(self._mozconfig, dict):
             loader = MozconfigLoader(self.topsrcdir)
             self._mozconfig = loader.read_mozconfig(path=self._mozconfig,
                 moz_build_app=os.environ.get('MOZ_CURRENT_PROJECT'))
 
         return self._mozconfig
 
     @property
     def config_environment(self):
@@ -694,16 +669,27 @@ class MachCommandBase(MozbuildObject):
         if hasattr(context, 'detect_virtualenv_mozinfo'):
             detect_virtualenv_mozinfo = getattr(context,
                 'detect_virtualenv_mozinfo')
         try:
             dummy = MozbuildObject.from_environment(cwd=context.cwd,
                 detect_virtualenv_mozinfo=detect_virtualenv_mozinfo)
             topsrcdir = dummy.topsrcdir
             topobjdir = dummy._topobjdir
+            if topobjdir:
+                # If we're inside a objdir and the found mozconfig resolves to
+                # another objdir, we abort. The reasoning here is that if you
+                # are inside an objdir you probably want to perform actions on
+                # that objdir, not another one. This prevents accidental usage
+                # of the wrong objdir when the current objdir is ambiguous.
+                config_topobjdir = MozbuildObject.resolve_mozconfig_topobjdir(
+                    topsrcdir, dummy.mozconfig)
+                if config_topobjdir and not samepath(topobjdir,
+                                                     config_topobjdir):
+                    raise ObjdirMismatchException(topobjdir, config_topobjdir)
         except BuildEnvironmentNotFoundException:
             pass
         except ObjdirMismatchException as e:
             print('Ambiguous object directory detected. We detected that '
                 'both %s and %s could be object directories. This is '
                 'typically caused by having a mozconfig pointing to a '
                 'different object directory from the current working '
                 'directory. To solve this problem, ensure you do not have a '
--- a/python/mozbuild/mozbuild/test/test_base.py
+++ b/python/mozbuild/mozbuild/test/test_base.py
@@ -7,16 +7,17 @@ from __future__ import unicode_literals
 import json
 import os
 import shutil
 import subprocess
 import sys
 import tempfile
 import unittest
 
+from cStringIO import StringIO
 from mozfile.mozfile import NamedTemporaryFile
 
 from mozunit import main
 
 from mach.logging import LoggingManager
 
 from mozbuild.base import (
     BadEnvironmentException,
@@ -267,18 +268,36 @@ class TestMozbuildObject(unittest.TestCa
             with open(mozinfo, 'wt') as fh:
                 json.dump(dict(
                     topsrcdir=topsrcdir,
                     mozconfig=mozconfig,
                 ), fh)
 
             os.chdir(topobjdir)
 
-            with self.assertRaises(ObjdirMismatchException):
-                MozbuildObject.from_environment(detect_virtualenv_mozinfo=False)
+            class MockMachContext(object):
+                pass
+
+            context = MockMachContext()
+            context.cwd = topobjdir
+            context.topdir = topsrcdir
+            context.settings = None
+            context.log_manager = None
+            context.detect_virtualenv_mozinfo=False
+
+            stdout = sys.stdout
+            sys.stdout = StringIO()
+            try:
+                with self.assertRaises(SystemExit):
+                    MachCommandBase(context)
+
+                self.assertTrue(sys.stdout.getvalue().startswith(
+                    'Ambiguous object directory detected.'))
+            finally:
+                sys.stdout = stdout
 
         finally:
             os.chdir(self._old_cwd)
             shutil.rmtree(d)
 
     def test_config_guess(self):
         # It's difficult to test for exact values from the output of
         # config.guess because they vary depending on platform.