From 3150b8f333e21056da77d263b48dece98a319647 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 30 Oct 2017 17:53:51 -0600 Subject: [PATCH] Bug 1415617: Allow specifying mozconfig in mozharness as fragments, rather than repeating the entire path everywhere. r=jlund MozReview-Commit-ID: 9OCRoFpFw1G --HG-- extra : rebase_source : dafc42a8f7d143715e623e177ea7c0bc6871996a --- .../mozharness/mozilla/building/buildbase.py | 93 +++++++++++--- .../test/helper_files/mozconfig_manifest.json | 3 + .../test/test_mozilla_building_buildbase.py | 120 ++++++++++++++++++ 3 files changed, 198 insertions(+), 18 deletions(-) create mode 100644 testing/mozharness/test/helper_files/mozconfig_manifest.json create mode 100644 testing/mozharness/test/test_mozilla_building_buildbase.py diff --git a/testing/mozharness/mozharness/mozilla/building/buildbase.py b/testing/mozharness/mozharness/mozilla/building/buildbase.py index abb8e0d41e14..855e79633a55 100755 --- a/testing/mozharness/mozharness/mozilla/building/buildbase.py +++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py @@ -233,6 +233,74 @@ class CheckTestCompleteParser(OutputParser): 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 @@ -1059,26 +1127,15 @@ or run without that action (ie: --no-{action})" 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) diff --git a/testing/mozharness/test/helper_files/mozconfig_manifest.json b/testing/mozharness/test/helper_files/mozconfig_manifest.json new file mode 100644 index 000000000000..8cc8049002b6 --- /dev/null +++ b/testing/mozharness/test/helper_files/mozconfig_manifest.json @@ -0,0 +1,3 @@ +{ + "gecko_path": "path/to/mozconfig" +} diff --git a/testing/mozharness/test/test_mozilla_building_buildbase.py b/testing/mozharness/test/test_mozilla_building_buildbase.py new file mode 100644 index 000000000000..8bea13ff1e8b --- /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={})