Bug 1415617: Allow specifying mozconfig in mozharness as fragments, rather than repeating the entire path everywhere. draft
authorTom Prince <mozilla@hocat.ca>
Mon, 30 Oct 2017 17:53:51 -0600
changeset 698727 feb628cc02e6307974c5099d678fa90b1d965344
parent 698726 7754730f99c01a690adce8ae7479c75c1ac47d71
child 698728 24deb969ed732bc5b2965c862f4fcd8f9537fdd8
push id89339
push userbmo:mozilla@hocat.ca
push dateThu, 16 Nov 2017 01:32:27 +0000
bugs1415617
milestone59.0a1
Bug 1415617: Allow specifying mozconfig in mozharness as fragments, rather than repeating the entire path everywhere. 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={})