Bug 1415617: Allow specifying mozconfig in mozharness as fragments, rather than repeating the entire path everywhere. r=jlund
authorTom Prince <mozilla@hocat.ca>
Mon, 30 Oct 2017 17:53:51 -0600
changeset 392088 b8cafc100e2ac880b46d05e3c52a8ce0305faef4
parent 392087 188a60783b36f64f977ef4b2e957607d9aae449e
child 392089 967cec5ff7add524afa7e9a8a570e53e99b10ad3
push id32910
push userrgurzau@mozilla.com
push dateThu, 16 Nov 2017 10:02:59 +0000
treeherdermozilla-central@9941e68b5a53 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlund
bugs1415617
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1415617: Allow specifying mozconfig in mozharness as fragments, rather than repeating the entire path everywhere. r=jlund MozReview-Commit-ID: 9OCRoFpFw1G
testing/mozharness/mozharness/mozilla/building/buildbase.py
testing/mozharness/test/helper_files/mozconfig_manifest.json
testing/mozharness/test/test_mozilla_building_buildbase.py
--- a/testing/mozharness/mozharness/mozilla/building/buildbase.py
+++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ -228,16 +228,84 @@ class CheckTestCompleteParser(OutputPars
         # Print the summary.
         summary = tbox_print_summary(self.pass_count,
                                      self.fail_count,
                                      self.leaked)
         self.info("TinderboxPrint: check<br/>%s\n" % summary)
 
         return self.tbpl_status
 
+class MozconfigPathError(Exception):
+    """
+    There was an error getting a mozconfig path from a mozharness config.
+    """
+
+def _get_mozconfig_path(script, config, dirs):
+    """
+    Get the path to the mozconfig file to use from a mozharness config.
+
+    :param script: The object to interact with the filesystem through.
+    :type script: ScriptMixin:
+
+    :param config: The mozharness config to inspect.
+    :type config: dict
+
+    :param dirs: The directories specified for this build.
+    :type dirs: dict
+    """
+    COMPOSITE_KEYS = {'mozconfig_variant', 'app_name', 'mozconfig_platform'}
+    have_composite_mozconfig = COMPOSITE_KEYS <= set(config.keys())
+    have_partial_composite_mozconfig = len(COMPOSITE_KEYS & set(config.keys())) > 0
+    have_src_mozconfig = 'src_mozconfig' in config
+    have_src_mozconfig_manifest = 'src_mozconfig_manifest' in config
+
+    # first determine the mozconfig path
+    if have_partial_composite_mozconfig and not have_composite_mozconfig:
+        raise MozconfigPathError(
+            "All or none of 'app_name', 'mozconfig_platform' and `mozconfig_variant' must be "
+            "in the config in order to determine the mozconfig.")
+    elif have_composite_mozconfig and have_src_mozconfig:
+        raise MozconfigPathError(
+            "'src_mozconfig' or 'mozconfig_variant' must be "
+            "in the config but not both in order to determine the mozconfig.")
+    elif have_composite_mozconfig and have_src_mozconfig_manifest:
+        raise MozconfigPathError(
+            "'src_mozconfig_manifest' or 'mozconfig_variant' must be "
+            "in the config but not both in order to determine the mozconfig.")
+    elif have_src_mozconfig and have_src_mozconfig_manifest:
+        raise MozconfigPathError(
+            "'src_mozconfig' or 'src_mozconfig_manifest' must be "
+            "in the config but not both in order to determine the mozconfig.")
+    elif have_composite_mozconfig:
+        src_mozconfig = '%(app_name)s/config/mozconfigs/%(platform)s/%(variant)s' % {
+            'app_name': config['app_name'],
+            'platform': config['mozconfig_platform'],
+            'variant': config['mozconfig_variant'],
+        }
+        abs_mozconfig_path = os.path.join(dirs['abs_src_dir'], src_mozconfig)
+    elif have_src_mozconfig:
+        abs_mozconfig_path = os.path.join(dirs['abs_src_dir'], config.get('src_mozconfig'))
+    elif have_src_mozconfig_manifest:
+        manifest = os.path.join(dirs['abs_work_dir'], config['src_mozconfig_manifest'])
+        if not os.path.exists(manifest):
+            raise MozconfigPathError(
+                'src_mozconfig_manifest: "%s" not found. Does it exist?' % (manifest,))
+        else:
+            with script.opened(manifest, error_level=ERROR) as (fh, err):
+                if err:
+                    raise MozconfigPathError("%s exists but coud not read properties" % manifest)
+                abs_mozconfig_path = os.path.join(dirs['abs_src_dir'], json.load(fh)['gecko_path'])
+    else:
+        raise MozconfigPathError(
+            "Must provide 'app_name', 'mozconfig_platform' and 'mozconfig_variant'; "
+            "or one of 'src_mozconfig' or 'src_mozconfig_manifest' in the config "
+            "in order to determine the mozconfig.")
+
+    return abs_mozconfig_path
+
 
 class BuildingConfig(BaseConfig):
     # TODO add nosetests for this class
     def get_cfgs_from_files(self, all_config_files, options):
         """
         Determine the configuration from the normal options and from
         `--branch`, `--build-pool`, and `--custom-build-variant-cfg`.  If the
         files for any of the latter options are also given with `--config-file`
@@ -1054,36 +1122,25 @@ or run without that action (ie: --no-{ac
         if old_package_paths:
             for package_path in old_package_paths:
                 self.rmtree(package_path)
         else:
             self.info("There wasn't any old packages to remove.")
 
     def _get_mozconfig(self):
         """assign mozconfig."""
-        c = self.config
         dirs = self.query_abs_dirs()
-        abs_mozconfig_path = ''
 
-        # first determine the mozconfig path
-        if c.get('src_mozconfig') and not c.get('src_mozconfig_manifest'):
-            self.info('Using in-tree mozconfig')
-            abs_mozconfig_path = os.path.join(dirs['abs_src_dir'], c.get('src_mozconfig'))
-        elif c.get('src_mozconfig_manifest') and not c.get('src_mozconfig'):
-            self.info('Using mozconfig based on manifest contents')
-            manifest = os.path.join(dirs['abs_work_dir'], c['src_mozconfig_manifest'])
-            if not os.path.exists(manifest):
-                self.fatal('src_mozconfig_manifest: "%s" not found. Does it exist?' % (manifest,))
-            with self.opened(manifest, error_level=ERROR) as (fh, err):
-                if err:
-                    self.fatal("%s exists but coud not read properties" % manifest)
-                abs_mozconfig_path = os.path.join(dirs['abs_src_dir'], json.load(fh)['gecko_path'])
-        else:
-            self.fatal("'src_mozconfig' or 'src_mozconfig_manifest' must be "
-                       "in the config but not both in order to determine the mozconfig.")
+        try:
+            abs_mozconfig_path = _get_mozconfig_path(
+                script=self, config=self.config, dirs=dirs)
+        except MozconfigPathError as e:
+            self.fatal(e.message)
+
+        self.info("Use mozconfig: {}".format(abs_mozconfig_path))
 
         # print its contents
         content = self.read_from_file(abs_mozconfig_path, error_level=FATAL)
         self.info("mozconfig content:")
         self.info(content)
 
         # finally, copy the mozconfig to a path that 'mach build' expects it to be
         self.copyfile(abs_mozconfig_path, os.path.join(dirs['abs_src_dir'], '.mozconfig'))
new file mode 100644
--- /dev/null
+++ b/testing/mozharness/test/helper_files/mozconfig_manifest.json
@@ -0,0 +1,3 @@
+{
+    "gecko_path": "path/to/mozconfig"
+}
new file mode 100644
--- /dev/null
+++ b/testing/mozharness/test/test_mozilla_building_buildbase.py
@@ -0,0 +1,120 @@
+import os
+import unittest
+from mozharness.base.log import LogMixin
+from mozharness.base.script import ScriptMixin
+from mozharness.mozilla.building.buildbase import (
+    _get_mozconfig_path, MozconfigPathError,
+)
+
+
+class FakeLogger(object):
+    def log_message(self, *args, **kwargs):
+        pass
+
+
+class FakeScriptMixin(LogMixin, ScriptMixin, object):
+    def __init__(self):
+        self.script_obj = self
+        self.log_obj = FakeLogger()
+
+
+class TestMozconfigPath(unittest.TestCase):
+    """
+    Tests for :func:`_get_mozconfig_path`.
+    """
+
+    def test_path(self):
+        """
+        Passing just ``src_mozconfig`` gives that file in ``abs_src_dir``.
+        """
+        script = FakeScriptMixin()
+
+        abs_src_path = _get_mozconfig_path(
+            script, config={'src_mozconfig': "path/to/mozconfig"},
+            dirs={"abs_src_dir": "/src"},
+        )
+        self.assertEqual(abs_src_path, "/src/path/to/mozconfig")
+
+    def test_composite(self):
+        """
+        Passing ``app_name``, ``mozconfig_platform``, and ``mozconfig_variant``
+        find the file in the ``config/mozconfigs`` subdirectory of that app
+        directory.
+        """
+        script = FakeScriptMixin()
+
+        config = {
+            'app_name': 'the-app',
+            'mozconfig_variant': 'variant',
+            'mozconfig_platform': 'platform9000',
+        }
+        abs_src_path = _get_mozconfig_path(
+            script, config=config,
+            dirs={"abs_src_dir": "/src"},
+        )
+        self.assertEqual(
+            abs_src_path,
+            "/src/the-app/config/mozconfigs/platform9000/variant",
+        )
+
+    def test_manifest(self):
+        """
+        Passing just ``src_mozconfig_manifest`` looks in that file in
+        ``abs_work_dir``, and finds the mozconfig file specified there in
+        ``abs_src_dir``.
+        """
+        script = FakeScriptMixin()
+
+        test_dir = os.path.dirname(__file__)
+        config = {
+            'src_mozconfig_manifest': "helper_files/mozconfig_manifest.json"
+        }
+        abs_src_path = _get_mozconfig_path(
+            script, config=config,
+            dirs={
+                "abs_src_dir": "/src",
+                "abs_work_dir": test_dir,
+            },
+        )
+        self.assertEqual(abs_src_path, "/src/path/to/mozconfig")
+
+    def test_errors(self):
+        script = FakeScriptMixin()
+
+        configs = [
+            # Not specifying any parts of a mozconfig path
+            {},
+            # Specifying both src_mozconfig and src_mozconfig_manifest
+            {'src_mozconfig': 'path', 'src_mozconfig_manifest': 'path'},
+            # Specifying src_mozconfig with some or all of a composite
+            # mozconfig path
+            {'src_mozconfig': 'path', 'app_name': 'app',
+             'mozconfig_platform': 'platform', 'mozconfig_variant': 'variant'},
+            {'src_mozconfig': 'path', 'mozconfig_platform': 'platform',
+             'mozconfig_variant': 'variant'},
+            {'src_mozconfig': 'path', 'app_name': 'app',
+             'mozconfig_variant': 'variant'},
+            {'src_mozconfig': 'path', 'app_name': 'app',
+             'mozconfig_platform': 'platform'},
+            # Specifying src_mozconfig_manifest with some or all of a composite
+            # mozconfig path
+            {'src_mozconfig_manifest': 'path', 'app_name': 'app',
+             'mozconfig_platform': 'platform', 'mozconfig_variant': 'variant'},
+            {'src_mozconfig_manifest': 'path',
+             'mozconfig_platform': 'platform', 'mozconfig_variant': 'variant'},
+            {'src_mozconfig_manifest': 'path', 'app_name': 'app',
+             'mozconfig_variant': 'variant'},
+            {'src_mozconfig_manifest': 'path', 'app_name': 'app',
+             'mozconfig_platform': 'platform'},
+            # Specifying only some parts of a compsite mozconfig path
+            {'mozconfig_platform': 'platform', 'mozconfig_variant': 'variant'},
+            {'app_name': 'app', 'mozconfig_variant': 'variant'},
+            {'app_name': 'app', 'mozconfig_platform': 'platform'},
+            {'app_name': 'app'},
+            {'mozconfig_variant': 'variant'},
+            {'mozconfig_platform': 'platform'},
+        ]
+
+        for config in configs:
+            with self.assertRaises(MozconfigPathError):
+                _get_mozconfig_path(script, config=config, dirs={})