Bug 1237706 - Use general unpack() method to extract all kinds of supported archives. r=jlund
☠☠ backed out by c912bbffbbce ☠ ☠
authorHenrik Skupin <mail@hskupin.info>
Wed, 13 Jan 2016 23:14:09 +0100
changeset 280430 a5472dd5fafded59961360da30e166e0c1bf27fd
parent 280429 38fef54dc04e2172d0672febaef83171f466db22
child 280431 a1674bc106ab2af7d7caa5226899d8c0d0181c90
push id29915
push usercbook@mozilla.com
push dateTue, 19 Jan 2016 11:01:05 +0000
treeherdermozilla-central@b67316254602 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlund
bugs1237706
milestone46.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 1237706 - Use general unpack() method to extract all kinds of supported archives. r=jlund
testing/mozharness/mozharness/base/script.py
testing/mozharness/mozharness/mozilla/testing/talos.py
testing/mozharness/mozharness/mozilla/testing/testbase.py
testing/mozharness/mozharness/mozilla/testing/unittest.py
testing/mozharness/scripts/android_emulator_unittest.py
testing/mozharness/scripts/android_panda.py
testing/mozharness/scripts/android_panda_talos.py
testing/mozharness/scripts/androidx86_emulator_unittest.py
testing/mozharness/scripts/b2g_emulator_unittest.py
testing/mozharness/scripts/desktop_unittest.py
testing/mozharness/scripts/web_platform_tests.py
--- a/testing/mozharness/mozharness/base/script.py
+++ b/testing/mozharness/mozharness/base/script.py
@@ -40,20 +40,21 @@ if os.name == 'nt':
 try:
     import simplejson as json
     assert json
 except ImportError:
     import json
 
 from mozprocess import ProcessHandler
 from mozharness.base.config import BaseConfig
-from mozharness.base.errors import ZipErrorList
+from mozharness.base.errors import TarErrorList, ZipErrorList
 from mozharness.base.log import SimpleFileLogger, MultiFileLogger, \
     LogMixin, OutputParser, DEBUG, INFO, ERROR, FATAL
 
+
 def platform_name():
     pm = PlatformMixin()
 
     if pm._is_linux() and pm._is_64_bit():
         return 'linux64'
     elif pm._is_linux() and not pm._is_64_bit():
         return 'linux'
     elif pm._is_darwin():
@@ -446,49 +447,36 @@ class ScriptMixin(PlatformMixin):
             kwargs = {"url": url, "file_name": file_name}
 
         return self.retry(
             download_func,
             kwargs=kwargs,
             **retry_args
         )
 
-    def download_unzip(self, url, parent_dir, target_unzip_dirs=None, halt_on_failure=True):
+    def download_unpack(self, url, parent_dir, extract_dirs=None, halt_on_failure=True):
         """Generic method to download and extract a zip file.
 
         The downloaded file will always be saved to the working directory and is not getting
         deleted after extracting.
 
         Args:
             url (str): URL where the file to be downloaded is located.
             parent_dir (str): directory where the downloaded file will
                               be extracted to.
-            target_unzip_dirs (list, optional): directories inside the zip file to extract.
-                                                Defaults to `None`.
+            extract_dirs (list, optional): directories inside the archive file to extract.
+                                           Defaults to `None`.
             halt_on_failure (bool, optional): whether or not to redefine the
                                               log level as `FATAL` on errors. Defaults to True.
 
         """
         dirs = self.query_abs_dirs()
-        zipfile = self.download_file(url, parent_dir=dirs['abs_work_dir'],
+        archive = self.download_file(url, parent_dir=dirs['abs_work_dir'],
                                      error_level=FATAL)
-
-        command = self.query_exe('unzip', return_type='list')
-        # Always overwrite to not get an input in a hidden pipe if files already exist
-        command.extend(['-q', '-o', zipfile, '-d', parent_dir])
-        if target_unzip_dirs:
-            command.extend(target_unzip_dirs)
-        # TODO error_list: http://www.info-zip.org/mans/unzip.html#DIAGNOSTICS
-        # unzip return code 11 is 'no matching files were found'
-        self.run_command(command,
-                         error_list=ZipErrorList,
-                         halt_on_failure=halt_on_failure,
-                         fatal_exit_code=3,
-                         success_codes=[0, 11],
-                         )
+        self.unpack(archive, parent_dir, extract_dirs, halt_on_failure)
 
     def load_json_url(self, url, error_level=None, *args, **kwargs):
         """ Returns a json object from a url (it retries). """
         contents = self._retry_download(
             url=url, error_level=error_level, *args, **kwargs
         )
         return json.loads(contents.read())
 
@@ -1390,36 +1378,65 @@ class ScriptMixin(PlatformMixin):
         except OSError:
             try:
                 open(file_name, 'w').close()
             except IOError as e:
                 msg = "I/O error(%s): %s" % (e.errno, e.strerror)
                 self.log(msg, error_level=error_level)
         os.utime(file_name, times)
 
-    def unpack(self, filename, extract_to):
-        '''
+    def unpack(self, filename, extract_to,
+               extract_dirs=None, halt_on_failure=True):
+        """
         This method allows us to extract a file regardless of its extension
 
         Args:
             filename (str): filename of the compressed file.
             extract_to (str): where to extract the compressed file.
-        '''
-        # XXX: Make sure that filename has a extension of one of our supported file formats
+            extract_dirs (list, optional): directories inside the archive file to extract.
+                                           Defaults to `None`.
+            halt_on_failure (bool, optional): whether or not to redefine the
+                                              log level as `FATAL` on errors. Defaults to True.
+
+        """
         m = re.search('\.tar\.(bz2|gz)$', filename)
         if m:
+            # tar does not automatically create the target folder
+            if not os.path.exists(extract_to):
+                os.makedirs(extract_to)
+
             command = self.query_exe('tar', return_type='list')
-            tar_cmd = "jxfv"
-            if m.group(1) == "gz":
-                tar_cmd = "zxfv"
-            command.extend([tar_cmd, filename, "-C", extract_to])
-            self.run_command(command, halt_on_failure=True)
+            tar_args = 'zxfv' if m.group(1) == 'gz' else 'jxfv'
+            command.extend([tar_args, filename, '-C', extract_to])
+            if extract_dirs:
+                command.append('--wildcards')
+                command.extend(extract_dirs)
+            self.run_command(command,
+                             error_list=TarErrorList,
+                             fatal_exit_code=3,
+                             halt_on_failure=halt_on_failure,
+                             )
+
+        elif filename.endswith('.zip'):
+            command = self.query_exe('unzip', return_type='list')
+            # Always overwrite to not get an input in a hidden pipe if files already exist
+            command.extend(['-q', '-o', filename, '-d', extract_to])
+            if extract_dirs:
+                command.extend(extract_dirs)
+            # TODO error_list: http://www.info-zip.org/mans/unzip.html#DIAGNOSTICS
+            # unzip return code 11 is 'no matching files were found'
+            self.run_command(command,
+                             error_list=ZipErrorList,
+                             fatal_exit_code=3,
+                             halt_on_failure=halt_on_failure,
+                             success_codes=[0, 11],
+                             )
+
         else:
-            # XXX implement
-            pass
+            raise NotImplementedError('No extraction method found for: %s' % filename)
 
 
 def PreScriptRun(func):
     """Decorator for methods that will be called before script execution.
 
     Each method on a BaseScript having this decorator will be called at the
     beginning of BaseScript.run().
 
--- a/testing/mozharness/mozharness/mozilla/testing/talos.py
+++ b/testing/mozharness/mozharness/mozilla/testing/talos.py
@@ -269,23 +269,23 @@ class Talos(TestingMixin, MercurialScrip
         if c.get('run_local'):
             self.talos_path = os.path.dirname(self.talos_json)
 
         src_talos_webdir = os.path.join(self.talos_path, 'talos')
 
         if self.query_pagesets_url():
             self.info("Downloading pageset...")
             src_talos_pageset = os.path.join(src_talos_webdir, 'tests')
-            self.download_unzip(self.pagesets_url, src_talos_pageset)
+            self.download_unpack(self.pagesets_url, src_talos_pageset)
 
     # Action methods. {{{1
     # clobber defined in BaseScript
     # read_buildbot_config defined in BuildbotMixin
 
-    def download_and_extract(self, target_unzip_dirs=None, suite_categories=None):
+    def download_and_extract(self, extract_dirs=None, suite_categories=None):
         return super(Talos, self).download_and_extract(
             suite_categories=['common', 'talos']
         )
 
     def create_virtualenv(self, **kwargs):
         """VirtualenvMixin.create_virtualenv() assuemes we're using
         self.config['virtualenv_modules']. Since we are installing
         talos from its source, we have to wrap that method here."""
--- a/testing/mozharness/mozharness/mozilla/testing/testbase.py
+++ b/testing/mozharness/mozharness/mozilla/testing/testbase.py
@@ -391,17 +391,17 @@ 2. running via buildbot and running the 
                 self.fatal("There was an error reading test package requirements from %s "
                            "requirements: `%s` - error: `%s`" % (source,
                                                                  package_requirements or 'None',
                                                                  err or 'No error'))
         self.info("Using the following test package requirements:\n%s" %
                   pprint.pformat(package_requirements))
         return package_requirements
 
-    def _download_test_packages(self, suite_categories, target_unzip_dirs):
+    def _download_test_packages(self, suite_categories, extract_dirs):
         # Some platforms define more suite categories/names than others.
         # This is a difference in the convention of the configs more than
         # to how these tests are run, so we pave over these differences here.
         aliases = {
             'robocop': 'mochitest',
             'mochitest-chrome': 'mochitest',
             'mochitest-gl': 'mochitest',
             'webapprt': 'mochitest',
@@ -425,31 +425,31 @@ 2. running via buildbot and running the 
                 # If we don't harness specific requirements, assume the common zip
                 # has everything we need to run tests for this suite.
                 target_packages = package_requirements['common']
 
             self.info("Downloading packages: %s for test suite category: %s" %
                       (target_packages, category))
             for file_name in target_packages:
                 target_dir = test_install_dir
-                unzip_dirs = target_unzip_dirs
+                unpack_dirs = extract_dirs
                 if "jsshell-" in file_name or file_name == "target.jsshell.zip":
                     self.info("Special-casing the jsshell zip file")
-                    unzip_dirs = None
+                    unpack_dirs = None
                     target_dir = dirs['abs_test_bin_dir']
                 url = self.query_build_dir_url(file_name)
-                self.download_unzip(url, target_dir,
-                                     target_unzip_dirs=unzip_dirs)
+                self.download_unpack(url, target_dir,
+                                     extract_dirs=unpack_dirs)
 
-    def _download_test_zip(self, target_unzip_dirs=None):
+    def _download_test_zip(self, extract_dirs=None):
         dirs = self.query_abs_dirs()
         test_install_dir = dirs.get('abs_test_install_dir',
                                     os.path.join(dirs['abs_work_dir'], 'tests'))
-        self.download_unzip(self.test_url, test_install_dir,
-                             target_unzip_dirs=target_unzip_dirs)
+        self.download_unpack(self.test_url, test_install_dir,
+                             extract_dirs=extract_dirs)
 
     def structured_output(self, suite_category):
         """Defines whether structured logging is in use in this configuration. This
         may need to be replaced with data from a different config at the resolution
         of bug 1070041 and related bugs.
         """
         return ('structured_suites' in self.config and
                 suite_category in self.config['structured_suites'])
@@ -486,19 +486,19 @@ 2. running via buildbot and running the 
         if self.config.get('download_symbols') == 'ondemand':
             self.symbols_path = self.symbols_url
             return
         if not self.symbols_path:
             self.symbols_path = os.path.join(dirs['abs_work_dir'], 'symbols')
 
         self.set_buildbot_property("symbols_url", self.symbols_url,
                                    write_to_file=True)
-        self.download_unzip(self.symbols_url, self.symbols_path)
+        self.download_unpack(self.symbols_url, self.symbols_path)
 
-    def download_and_extract(self, target_unzip_dirs=None, suite_categories=None):
+    def download_and_extract(self, extract_dirs=None, suite_categories=None):
         """
         download and extract test zip / download installer
         """
         # Swap plain http for https when we're downloading from ftp
         # See bug 957502 and friends
         from_ = "http://ftp.mozilla.org"
         to_ = "https://ftp-ssl.mozilla.org"
         for attr in 'symbols_url', 'installer_url', 'test_packages_url', 'test_url':
@@ -511,27 +511,27 @@ 2. running via buildbot and running the 
         if 'test_url' in self.config:
             # A user has specified a test_url directly, any test_packages_url will
             # be ignored.
             if self.test_packages_url:
                 self.error('Test data will be downloaded from "%s", the specified test '
                            ' package data at "%s" will be ignored.' %
                            (self.config.get('test_url'), self.test_packages_url))
 
-            self._download_test_zip(target_unzip_dirs)
+            self._download_test_zip(extract_dirs)
         else:
             if not self.test_packages_url:
                 # The caller intends to download harness specific packages, but doesn't know
                 # where the packages manifest is located. This is the case when the
                 # test package manifest isn't set as a buildbot property, which is true
                 # for some self-serve jobs and platforms using parse_make_upload.
                 self.test_packages_url = self.query_build_dir_url('test_packages.json')
 
             suite_categories = suite_categories or ['common']
-            self._download_test_packages(suite_categories, target_unzip_dirs)
+            self._download_test_packages(suite_categories, extract_dirs)
 
         self._download_installer()
         if self.config.get('download_symbols'):
             self._download_and_extract_symbols()
 
     # create_virtualenv is in VirtualenvMixin.
 
     def preflight_install(self):
--- a/testing/mozharness/mozharness/mozilla/testing/unittest.py
+++ b/testing/mozharness/mozharness/mozilla/testing/unittest.py
@@ -216,17 +216,17 @@ class EmulatorMixin(object):
             unzip_cmd = [unzip, '-q', os.path.join(dirs['abs_work_dir'], "emulator.zip")]
             self.run_command(unzip_cmd, cwd=dirs['abs_emulator_dir'], halt_on_failure=True,
                              fatal_exit_code=3)
 
     def install_emulator(self):
         dirs = self.query_abs_dirs()
         self.mkdir_p(dirs['abs_emulator_dir'])
         if self.config.get('emulator_url'):
-            self.download_unzip(self.config['emulator_url'], dirs['abs_emulator_dir'])
+            self.download_unpack(self.config['emulator_url'], dirs['abs_emulator_dir'])
         elif self.config.get('emulator_manifest'):
             manifest_path = self.create_tooltool_manifest(self.config['emulator_manifest'])
             do_unzip = True
             if 'unpack' in self.config['emulator_manifest']:
                 do_unzip = False
             self.install_emulator_from_tooltool(manifest_path, do_unzip)
         elif self.buildbot_config:
             props = self.buildbot_config.get('properties')
--- a/testing/mozharness/scripts/android_emulator_unittest.py
+++ b/testing/mozharness/scripts/android_emulator_unittest.py
@@ -622,17 +622,17 @@ class AndroidEmulatorTest(BlobUploadMixi
         """
         suite_category = self.test_suite_definitions[self.test_suite]['category']
         super(AndroidEmulatorTest, self).download_and_extract(suite_categories=[suite_category])
         dirs = self.query_abs_dirs()
         if self.test_suite.startswith('robocop'):
             robocop_url = self.installer_url[:self.installer_url.rfind('/')] + '/robocop.apk'
             self.info("Downloading robocop...")
             self.download_file(robocop_url, 'robocop.apk', dirs['abs_work_dir'], error_level=FATAL)
-        self.download_unzip(self.host_utils_url, dirs['abs_xre_dir'])
+        self.download_unpack(self.host_utils_url, dirs['abs_xre_dir'])
 
     def install(self):
         """
         Install APKs on the emulator
         """
         assert self.installer_path is not None, \
             "Either add installer_path to the config or use --installer-path."
 
--- a/testing/mozharness/scripts/android_panda.py
+++ b/testing/mozharness/scripts/android_panda.py
@@ -378,17 +378,17 @@ class PandaTest(TestingMixin, MercurialS
         out, err = self.logcat_proc.communicate()
         self.info("logcat.py output:\n%s\n%s\n" % (out, err))
 
     def _download_unzip_hostutils(self):
         c = self.config
         dirs = self.query_abs_dirs()
         self.host_utils_url = c['hostutils_url']
         # get the zip and extract it
-        self.download_unzip(self.host_utils_url, dirs['abs_hostutils_dir'])
+        self.download_unpack(self.host_utils_url, dirs['abs_hostutils_dir'])
 
     def _install_app(self):
         c = self.config
         base_work_dir = c['base_work_dir']
         cmd = ['python', self.config.get("install_app_path"), self.device_ip, 'build/' + str(self.filename_apk), self.app_name]
         self.run_command(cmd, cwd=base_work_dir, halt_on_failure=True, fatal_exit_code=3)
 
     def _download_robocop_apk(self):
--- a/testing/mozharness/scripts/android_panda_talos.py
+++ b/testing/mozharness/scripts/android_panda_talos.py
@@ -294,17 +294,17 @@ class PandaTalosTest(TestingMixin, Mercu
         self.mkdir_p(dirs['abs_talosbuild_dir'])
         robocop_url = self.installer_url.rsplit("/", 1)[0] + "/robocop.apk"
         self.mkdir_p(dirs['abs_talosdatatalos_dir'])
 
         #download and extract apk to /builds/panda-0nnn/talos-data
         self.rmtree(dirs['abs_talosdata_dir'])
         self.mkdir_p(dirs['abs_talosdata_dir'])
         self.mkdir_p(dirs['abs_symbols_dir'])
-        self.download_unzip(self.installer_url,
+        self.download_unpack(self.installer_url,
                              dirs['abs_fennec_dir'])
         #this is ugly but you can't specify a file in download_unzip to extract the file to, by default it's the abs_work_dir
         #should think of a better way
         self.download_file(self.installer_url,
                            parent_dir=dirs['abs_talosdata_dir'],
                            error_level=FATAL)
 
         #download and extract fennec_ids.txt to /builds/panda-0nnn/talos-data
@@ -312,20 +312,20 @@ class PandaTalosTest(TestingMixin, Mercu
                            parent_dir=dirs['abs_talosdata_dir'],
                            error_level=FATAL)
         #download and extract robocop.apk to /builds/panda-0nnn/talos-data/build
         self.download_file(robocop_url, file_name='robocop.apk',
                            parent_dir=dirs['abs_talosbuild_dir'],
                            error_level=FATAL)
         self.symbols_url = self.query_symbols_url()
 
-        self.download_unzip(self.symbols_url,
+        self.download_unpack(self.symbols_url,
                              dirs['abs_symbols_dir'])
 
-        self.download_unzip(self.config['retry_url'],
+        self.download_unpack(self.config['retry_url'],
                              dirs['abs_talosdata_dir'])
 
         taloscode = self.config.get("talos_from_code_url")
 
         talos_from_code_url = (taloscode % (self.repo_path, self.revision))
 
         self.download_file(talos_from_code_url, file_name='talos_from_code.py',
                            parent_dir=dirs['abs_talosdata_dir'],
--- a/testing/mozharness/scripts/androidx86_emulator_unittest.py
+++ b/testing/mozharness/scripts/androidx86_emulator_unittest.py
@@ -676,17 +676,17 @@ class AndroidEmulatorTest(BlobUploadMixi
         super(AndroidEmulatorTest, self).download_and_extract(suite_categories=suite_categories)
         dirs = self.query_abs_dirs()
 
         for suite_name in self.test_suites:
             if suite_name.startswith('robocop'):
                 self._download_robocop_apk()
                 break
 
-        self.download_unzip(self.host_utils_url, dirs['abs_xre_dir'])
+        self.download_unpack(self.host_utils_url, dirs['abs_xre_dir'])
 
     def install(self):
         assert self.installer_path is not None, \
             "Either add installer_path to the config or use --installer-path."
 
         emulator_index = 0
         for suite_name in self.test_suites:
             emulator = self.emulators[emulator_index]
--- a/testing/mozharness/scripts/b2g_emulator_unittest.py
+++ b/testing/mozharness/scripts/b2g_emulator_unittest.py
@@ -203,17 +203,17 @@ class B2GEmulatorTest(TestingMixin, VCSM
 
         self.mkdir_p(dirs['abs_emulator_dir'])
         tar = self.query_exe('tar', return_type='list')
         self.run_command(tar + ['zxf', self.installer_path],
                          cwd=dirs['abs_emulator_dir'],
                          error_list=TarErrorList,
                          halt_on_failure=True, fatal_exit_code=3)
 
-        self.download_unzip(self.config['xre_url'],
+        self.download_unpack(self.config['xre_url'],
                              dirs['abs_xre_dir'])
 
         if self.config.get('busybox_url'):
             self.download_file(self.config['busybox_url'],
                                file_name='busybox',
                                parent_dir=dirs['abs_work_dir'])
             self.busybox_path = os.path.join(dirs['abs_work_dir'], 'busybox')
 
--- a/testing/mozharness/scripts/desktop_unittest.py
+++ b/testing/mozharness/scripts/desktop_unittest.py
@@ -470,30 +470,30 @@ class DesktopUnittest(TestingMixin, Merc
 
     def download_and_extract(self):
         """
         download and extract test zip / download installer
         optimizes which subfolders to extract from tests zip
         """
         c = self.config
 
-        target_unzip_dirs = None
+        extract_dirs = None
         if c['specific_tests_zip_dirs']:
-            target_unzip_dirs = list(c['minimum_tests_zip_dirs'])
+            extract_dirs = list(c['minimum_tests_zip_dirs'])
             for category in c['specific_tests_zip_dirs'].keys():
                 if c['run_all_suites'] or self._query_specified_suites(category) \
                         or 'run-tests' not in self.actions:
-                    target_unzip_dirs.extend(c['specific_tests_zip_dirs'][category])
+                    extract_dirs.extend(c['specific_tests_zip_dirs'][category])
 
         if c.get('run_all_suites'):
             target_categories = SUITE_CATEGORIES
         else:
             target_categories = [cat for cat in SUITE_CATEGORIES
                                  if self._query_specified_suites(cat) is not None]
-        super(DesktopUnittest, self).download_and_extract(target_unzip_dirs=target_unzip_dirs,
+        super(DesktopUnittest, self).download_and_extract(extract_dirs=extract_dirs,
                                                           suite_categories=target_categories)
 
     # pull defined in VCSScript.
     # preflight_run_tests defined in TestingMixin.
 
     def run_tests(self):
         self._run_category_suites('mochitest')
         self._run_category_suites('reftest')
--- a/testing/mozharness/scripts/web_platform_tests.py
+++ b/testing/mozharness/scripts/web_platform_tests.py
@@ -160,22 +160,22 @@ class WebPlatformTest(TestingMixin, Merc
                                       str_format_values=str_format_values))
         cmd.extend(self.query_tests_args(try_tests,
                                          str_format_values=str_format_values))
 
         return cmd
 
     def download_and_extract(self):
         super(WebPlatformTest, self).download_and_extract(
-            target_unzip_dirs=["bin/*",
-                               "config/*",
-                               "mozbase/*",
-                               "marionette/*",
-                               "tools/wptserve/*",
-                               "web-platform/*"],
+            extract_dirs=["bin/*",
+                          "config/*",
+                          "mozbase/*",
+                          "marionette/*",
+                          "tools/wptserve/*",
+                          "web-platform/*"],
             suite_categories=["web-platform"])
 
     def run_tests(self):
         dirs = self.query_abs_dirs()
         cmd = self._query_cmd()
 
         parser = StructuredOutputParser(config=self.config,
                                         log_obj=self.log_obj)