Bug 1278698 - Make mozharness build scripts trigger artifact builds when specified via try syntax; r=armenzg
☠☠ backed out by 54d81bfbd901 ☠ ☠
authorMaja Frydrychowicz <mjzffr@gmail.com>
Fri, 02 Sep 2016 23:56:54 -0400
changeset 313407 f770a12324d77e69b830234da9392b91892600f7
parent 313406 dbeaca20e104c0a13281896dcb6a1cda63b32ba9
child 313408 4468d50fd021a9f98ae4cd48da62be2d53246dcf
push id30681
push userphilringnalda@gmail.com
push dateSat, 10 Sep 2016 07:13:06 +0000
treeherdermozilla-central@61cc64967515 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarmenzg
bugs1278698
milestone51.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 1278698 - Make mozharness build scripts trigger artifact builds when specified via try syntax; r=armenzg MozReview-Commit-ID: JDieAcHgpSy
testing/mozharness/mozharness/base/config.py
testing/mozharness/mozharness/mozilla/building/buildbase.py
testing/mozharness/mozharness/mozilla/testing/try_tools.py
testing/mozharness/scripts/fx_desktop_build.py
--- a/testing/mozharness/mozharness/base/config.py
+++ b/testing/mozharness/mozharness/base/config.py
@@ -511,17 +511,29 @@ class BaseConfig(object):
         # The idea behind the volatile_config is we don't want to save this
         # info over multiple runs.  This defaults to the action-specific
         # config options, but can be anything.
         for key in self.volatile_config.keys():
             if self._config.get(key) is not None:
                 self.volatile_config[key] = self._config[key]
                 del(self._config[key])
 
-        """Actions.
+        self.update_actions()
+        if options.list_actions:
+            self.list_actions()
+
+        # Keep? This is for saving the volatile config in the dump_config
+        self._config['volatile_config'] = self.volatile_config
+
+        self.options = options
+        self.args = args
+        return (self.options, self.args)
+
+    def update_actions(self):
+        """ Update actions after reading in config.
 
         Seems a little complex, but the logic goes:
 
         First, if default_actions is specified in the config, set our
         default actions even if the script specifies other default actions.
 
         Without any other action-specific options, run with default actions.
 
@@ -533,34 +545,25 @@ class BaseConfig(object):
 
         Finally, if we specify --no-ACTION, remove that from the list of
         actions to perform.
         """
         if self._config.get('default_actions'):
             default_actions = self.verify_actions(self._config['default_actions'])
             self.default_actions = default_actions
         self.verify_actions_order(self.default_actions)
-        if options.list_actions:
-            self.list_actions()
         self.actions = self.default_actions[:]
         if self.volatile_config['actions']:
             actions = self.verify_actions(self.volatile_config['actions'])
             self.actions = actions
         elif self.volatile_config['add_actions']:
             actions = self.verify_actions(self.volatile_config['add_actions'])
             self.actions.extend(actions)
         if self.volatile_config['no_actions']:
             actions = self.verify_actions(self.volatile_config['no_actions'])
             for action in actions:
                 if action in self.actions:
                     self.actions.remove(action)
 
-        # Keep? This is for saving the volatile config in the dump_config
-        self._config['volatile_config'] = self.volatile_config
-
-        self.options = options
-        self.args = args
-        return (self.options, self.args)
-
 
 # __main__ {{{1
 if __name__ == '__main__':
     pass
--- a/testing/mozharness/mozharness/mozilla/building/buildbase.py
+++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ -361,17 +361,18 @@ class BuildOptionParser(object):
         'api-15': 'builds/releng_sub_%s_configs/%s_api_15.py',
         'api-15-debug': 'builds/releng_sub_%s_configs/%s_api_15_debug.py',
         'api-15-gradle': 'builds/releng_sub_%s_configs/%s_api_15_gradle.py',
         'x86': 'builds/releng_sub_%s_configs/%s_x86.py',
         'api-15-partner-sample1': 'builds/releng_sub_%s_configs/%s_api_15_partner_sample1.py',
         'android-test': 'builds/releng_sub_%s_configs/%s_test.py',
         'android-checkstyle': 'builds/releng_sub_%s_configs/%s_checkstyle.py',
         'android-lint': 'builds/releng_sub_%s_configs/%s_lint.py',
-        'valgrind' : 'builds/releng_sub_%s_configs/%s_valgrind.py'
+        'valgrind' : 'builds/releng_sub_%s_configs/%s_valgrind.py',
+        'artifact': 'builds/releng_sub_%s_configs/%s_artifact.py',
     }
     build_pool_cfg_file = 'builds/build_pool_specifics.py'
     branch_cfg_file = 'builds/branch_specifics.py'
 
     @classmethod
     def _query_pltfrm_and_bits(cls, target_option, options):
         """ determine platform and bits
 
@@ -417,23 +418,17 @@ class BuildOptionParser(object):
                     cls.platform = 'android'
                     break
             else:
                 sys.exit(error_msg % (target_option, 'platform', '--platform',
                                       '"linux", "windows", "mac", or "android"'))
         return cls.bits, cls.platform
 
     @classmethod
-    def set_build_variant(cls, option, opt, value, parser):
-        """ sets an extra config file.
-
-        This is done by either taking an existing filepath or by taking a valid
-        shortname coupled with known platform/bits.
-        """
-
+    def find_variant_cfg_path(cls, opt, value, parser):
         valid_variant_cfg_path = None
         # first let's see if we were given a valid short-name
         if cls.build_variants.get(value):
             bits, pltfrm = cls._query_pltfrm_and_bits(opt, parser.values)
             prospective_cfg_path = cls.build_variants[value] % (pltfrm, bits)
         else:
             # this is either an incomplete path or an invalid key in
             # build_variants
@@ -446,16 +441,27 @@ class BuildOptionParser(object):
             # let's take our prospective_cfg_path and see if we can
             # determine an existing file
             for path in cls.config_file_search_path:
                 if os.path.exists(os.path.join(path, prospective_cfg_path)):
                     # success! we found a config file
                     valid_variant_cfg_path = os.path.join(path,
                                                           prospective_cfg_path)
                     break
+        return valid_variant_cfg_path, prospective_cfg_path
+
+    @classmethod
+    def set_build_variant(cls, option, opt, value, parser):
+        """ sets an extra config file.
+
+        This is done by either taking an existing filepath or by taking a valid
+        shortname coupled with known platform/bits.
+        """
+        valid_variant_cfg_path, prospective_cfg_path = cls.find_variant_cfg_path(
+            '--custom-build-variant-cfg', value, parser)
 
         if not valid_variant_cfg_path:
             # either the value was an indeterminable path or an invalid short
             # name
             sys.exit("Whoops!\n'--custom-build-variant' was passed but an "
                      "appropriate config file could not be determined. Tried "
                      "using: '%s' but it was either not:\n\t-- a valid "
                      "shortname: %s \n\t-- a valid path in %s \n\t-- a "
--- a/testing/mozharness/mozharness/mozilla/testing/try_tools.py
+++ b/testing/mozharness/mozharness/mozilla/testing/try_tools.py
@@ -78,68 +78,102 @@ class TryToolsMixin(TransferMixin):
         msg = None
         if "try_message" in self.config and self.config["try_message"]:
             msg = self.config["try_message"]
         else:
             if self.buildbot_config['sourcestamp']['changes']:
                 msg = self.buildbot_config['sourcestamp']['changes'][-1]['comments']
 
             if msg is None or len(msg) == 1024:
-                # This commit message was potentially truncated, get the full message
+                # This commit message was potentially truncated or not available in
+                # buildbot_config (e.g. if running in TaskCluster), get the full message
                 # from hg.
                 props = self.buildbot_config['properties']
-                rev = props['revision']
-                repo = props['repo_path']
-                url = 'https://hg.mozilla.org/%s/json-pushes?changeset=%s&full=1' % (repo, rev)
+                repo_url = 'https://hg.mozilla.org/%s/'
+                if 'revision' in props and 'repo_path' in props:
+                    rev = props['revision']
+                    repo_path = props['repo_path']
+                else:
+                    # In TaskCluster we have no buildbot props, rely on env vars instead
+                    rev = os.environ.get('GECKO_HEAD_REV')
+                    repo_path = self.config.get('branch')
+                if repo_path:
+                    repo_url = repo_url % repo_path
+                else:
+                    repo_url = os.environ.get('GECKO_HEAD_REPOSITORY',
+                                              repo_url % 'try')
+                if not repo_url.endswith('/'):
+                    repo_url += '/'
+
+                url = '{}json-pushes?changeset={}&full=1'.format(repo_url, rev)
 
                 pushinfo = self.load_json_from_url(url)
                 for k, v in pushinfo.items():
                     if isinstance(v, dict) and 'changesets' in v:
                         msg = v['changesets'][-1]['desc']
 
             if not msg and 'try_syntax' in self.buildbot_config['properties']:
                 # If we don't find try syntax in the usual place, check for it in an
                 # alternate property available to tools using self-serve.
                 msg = self.buildbot_config['properties']['try_syntax']
+        if not msg:
+            self.warning('Try message not found.')
+        return msg
 
-        return msg
+    def _extract_try_args(self, msg):
+        """ Returns a list of args from a try message, for parsing """
+        if not msg:
+            return None
+        all_try_args = None
+        for line in msg.splitlines():
+            if 'try: ' in line:
+                # Autoland adds quotes to try strings that will confuse our
+                # args later on.
+                if line.startswith('"') and line.endswith('"'):
+                    line = line[1:-1]
+                # Allow spaces inside of [filter expressions]
+                try_message = line.strip().split('try: ', 1)
+                all_try_args = re.findall(r'(?:\[.*?\]|\S)+', try_message[1])
+                break
+        if not all_try_args:
+            self.warning('Try syntax not found in: %s.' % msg )
+        return all_try_args
+
+    def try_message_has_flag(self, flag, message=None):
+        """
+        Returns True if --`flag` is present in message.
+        """
+        parser = argparse.ArgumentParser()
+        parser.add_argument('--' + flag, action='store_true')
+        message = message or self._extract_try_message()
+        if not message:
+            return False
+        msg_list = self._extract_try_args(message)
+        args, _ = parser.parse_known_args(msg_list)
+        return getattr(args, flag, False)
 
     @PostScriptAction('download-and-extract')
     def set_extra_try_arguments(self, action, success=None):
         """Finds a commit message and parses it for extra arguments to pass to the test
         harness command line and test paths used to filter manifests.
 
         Extracting arguments from a commit message taken directly from the try_parser.
         """
         if (not self.buildbot_config or 'properties' not in self.buildbot_config or
                 self.buildbot_config['properties'].get('branch') != 'try'):
             return
 
         msg = self._extract_try_message()
         if not msg:
             return
 
-        all_try_args = None
-        for line in msg.splitlines():
-            if 'try: ' in line:
-                # Autoland adds quotes to try strings that will confuse our
-                # args later on.
-                if line.startswith('"') and line.endswith('"'):
-                    line = line[1:-1]
-                # Allow spaces inside of [filter expressions]
-                try_message = line.strip().split('try: ', 1)
-                all_try_args = re.findall(r'(?:\[.*?\]|\S)+', try_message[1])
-                break
-
+        all_try_args = self._extract_try_args(msg)
         if not all_try_args:
-            self.warning('Try syntax not found in buildbot config, unable to append '
-                         'arguments from try.')
             return
 
-
         parser = argparse.ArgumentParser(
             description=('Parse an additional subset of arguments passed to try syntax'
                          ' and forward them to the underlying test harness command.'))
 
         label_dict = {}
         def label_from_val(val):
             if val in label_dict:
                 return label_dict[val]
--- a/testing/mozharness/scripts/fx_desktop_build.py
+++ b/testing/mozharness/scripts/fx_desktop_build.py
@@ -8,30 +8,34 @@
 
 script harness to build nightly firefox within Mozilla's build environment
 and developer machines alike
 
 author: Jordan Lund
 
 """
 
+import copy
+import pprint
 import sys
 import os
 
 # load modules from parent dir
 sys.path.insert(1, os.path.dirname(sys.path[0]))
 
 from mozharness.mozilla.building.buildbase import BUILD_BASE_CONFIG_OPTIONS, \
-    BuildingConfig, BuildScript
+    BuildingConfig, BuildOptionParser, BuildScript
+from mozharness.base.config import parse_config_file
+from mozharness.mozilla.testing.try_tools import TryToolsMixin, try_config_options
 
 
-class FxDesktopBuild(BuildScript, object):
+class FxDesktopBuild(BuildScript, TryToolsMixin, object):
     def __init__(self):
         buildscript_kwargs = {
-            'config_options': BUILD_BASE_CONFIG_OPTIONS,
+            'config_options': BUILD_BASE_CONFIG_OPTIONS + copy.deepcopy(try_config_options),
             'all_actions': [
                 'get-secrets',
                 'clobber',
                 'clone-tools',
                 'checkout-sources',
                 'setup-mock',
                 'build',
                 'upload-files',  # upload from BB to TC
@@ -115,18 +119,50 @@ class FxDesktopBuild(BuildScript, object
                     platform_for_log_url += '-pgo'
                 # postrun.py uses stage_platform buildbot prop as part of the log url
                 self.set_buildbot_property('stage_platform',
                                            platform_for_log_url,
                                            write_to_file=True)
             else:
                 self.fatal("'stage_platform' not determined and is required in your config")
 
+            if self.try_message_has_flag('artifact'):
+                self.info('Artifact build requested in try syntax.')
+                self._update_build_variant(rw_config)
 
     # helpers
+    def _update_build_variant(self, rw_config, variant='artifact'):
+        """ Intended for use in _pre_config_lock """
+        c = self.config
+        variant_cfg_path, _ = BuildOptionParser.find_variant_cfg_path(
+            '--custom-build-variant-cfg',
+            variant,
+            rw_config.config_parser
+        )
+        if not variant_cfg_path:
+            self.fatal('Could not find appropriate config file for variant %s' % variant)
+        # Update other parts of config to keep dump-config accurate
+        # Only dump-config is affected because most config info is set during
+        # initial parsing
+        variant_cfg_dict = parse_config_file(variant_cfg_path)
+        rw_config.all_cfg_files_and_dicts.append((variant_cfg_path, variant_cfg_dict))
+        c.update({
+            'build_variant': variant,
+            'config_files': c['config_files'] + [variant_cfg_path]
+        })
+
+        self.info("Updating self.config with the following from {}:".format(variant_cfg_path))
+        self.info(pprint.pformat(variant_cfg_dict))
+        c.update(variant_cfg_dict)
+        # replace rw_config as well to set actions as in BaseScript
+        rw_config.set_config(c, overwrite=True)
+        rw_config.update_actions()
+        self.actions = tuple(rw_config.actions)
+        self.all_actions = tuple(rw_config.all_actions)
+
 
     def query_abs_dirs(self):
         if self.abs_dirs:
             return self.abs_dirs
         c = self.config
         abs_dirs = super(FxDesktopBuild, self).query_abs_dirs()
         if not c.get('app_ini_path'):
             self.fatal('"app_ini_path" is needed in your config for this '
@@ -158,12 +194,15 @@ class FxDesktopBuild(BuildScript, object
 
         # Actions {{{2
         # read_buildbot_config in BuildingMixin
         # clobber in BuildingMixin -> PurgeMixin
         # if Linux config:
         # reset_mock in BuildingMixing -> MockMixin
         # setup_mock in BuildingMixing (overrides MockMixin.mock_setup)
 
+    def set_extra_try_arguments(self, action, success=None):
+        """ Override unneeded method from TryToolsMixin """
+        pass
 
 if __name__ == '__main__':
     fx_desktop_build = FxDesktopBuild()
     fx_desktop_build.run_and_exit()