Bug 1278698 - Make mozharness build scripts trigger artifact builds when specified via try syntax; r=armenzg
authorMaja Frydrychowicz <mjzffr@gmail.com>
Fri, 09 Sep 2016 17:16:04 -0400
changeset 313410 bd82d664a18cc6b4a2a8f1e8b31409c986558371
parent 313409 54d81bfbd90190c7e65361d53369c2ca78971bef
child 313411 ac7f8d39573545555142542bddec934a249b3217
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
@@ -73,72 +73,111 @@ class TryToolsMixin(TransferMixin):
             'reftest',
         )),
     }
 
     def _extract_try_message(self):
         msg = None
         if "try_message" in self.config and self.config["try_message"]:
             msg = self.config["try_message"]
-        else:
+        elif self._is_try():
             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
 
-    @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()
+    def _extract_try_args(self, msg):
+        """ Returns a list of args from a try message, for parsing """
         if not msg:
-            return
-
+            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
 
-        if not all_try_args:
-            self.warning('Try syntax not found in buildbot config, unable to append '
-                         'arguments from try.')
+    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)
+
+    def _is_try(self):
+        repo_path = None
+        if self.buildbot_config and 'properties' in self.buildbot_config:
+            repo_path = self.buildbot_config['properties'].get('branch')
+        return self.config.get('branch', repo_path) == 'try'
+
+    @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._is_try():
             return
 
+        msg = self._extract_try_message()
+        if not msg:
+            return
+
+        all_try_args = self._extract_try_args(msg)
+        if not all_try_args:
+            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:
--- 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()