Bug 1595982 - make mozharness::base::config.py and diskutils.py python3 compatible r=aki
authorEdwin Takahashi <egao@mozilla.com>
Wed, 20 Nov 2019 00:49:27 +0000
changeset 502719 8afa2a9e5451e996c16de0ee03ef4fa3f3905d9d
parent 502718 7dcc2fa474a86bc2064b8ffa13e35f3ee7259344
child 502720 8b0665d186e78a40fe9a811b6289aa3f2015fb9b
push id36823
push usermalexandru@mozilla.com
push dateWed, 20 Nov 2019 09:47:58 +0000
treeherdermozilla-central@79821df17239 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaki
bugs1595982
milestone72.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 1595982 - make mozharness::base::config.py and diskutils.py python3 compatible r=aki Changes: Replace `execfile` with python2/3 compatible equivalent `exec(compile(open))`. Wrap `keys()` method with `list()` to enforce a list. Compare to `six.string_types` instead of `unicode`. The usual import and code formatting. Fixes to `configure.py` should in particular fix a lot of the issues with tests running in tox-py35, since a lot of the tests get hung upon the `urllib2` import in `config.py`. Differential Revision: https://phabricator.services.mozilla.com/D53895
testing/mozharness/mozharness/base/config.py
testing/mozharness/mozharness/base/diskutils.py
--- a/testing/mozharness/mozharness/base/config.py
+++ b/testing/mozharness/mozharness/base/config.py
@@ -21,35 +21,43 @@ at a later time.
 TODO:
 
 * check_required_settings or something -- run at init, assert that
   these settings are set.
 """
 
 from __future__ import print_function
 
-from copy import deepcopy
-from optparse import OptionParser, Option, OptionGroup
 import os
+import socket
 import sys
-import urllib2
-import socket
 import time
+from copy import deepcopy
+from optparse import Option, OptionGroup, OptionParser
+
+from mozharness.base.log import CRITICAL, DEBUG, ERROR, FATAL, INFO, WARNING
+
+try:
+    from urllib2 import URLError, urlopen
+except ImportError:
+    from urllib.request import urlopen
+    from urllib.error import URLError
+
+
 try:
     import simplejson as json
 except ImportError:
     import json
 
-from mozharness.base.log import DEBUG, INFO, WARNING, ERROR, CRITICAL, FATAL
-
 
 # optparse {{{1
 class ExtendedOptionParser(OptionParser):
     """OptionParser, but with ExtendOption as the option_class.
     """
+
     def __init__(self, **kwargs):
         kwargs['option_class'] = ExtendOption
         OptionParser.__init__(self, **kwargs)
 
 
 class ExtendOption(Option):
     """from http://docs.python.org/library/optparse.html?highlight=optparse#adding-new-actions"""
     ACTIONS = Option.ACTIONS + ("extend",)
@@ -90,17 +98,17 @@ class ReadOnlyDict(dict):
     def __init__(self, dictionary):
         self._lock = False
         self.update(dictionary.copy())
 
     def _check_lock(self):
         assert not self._lock, "ReadOnlyDict is locked!"
 
     def lock(self):
-        for (k, v) in self.items():
+        for (k, v) in list(self.items()):
             self[k] = make_immutable(v)
         self._lock = True
 
     def __setitem__(self, *args):
         self._check_lock()
         return dict.__setitem__(self, *args)
 
     def __delitem__(self, *args):
@@ -126,20 +134,20 @@ class ReadOnlyDict(dict):
     def update(self, *args):
         self._check_lock()
         dict.update(self, *args)
 
     def __deepcopy__(self, memo):
         cls = self.__class__
         result = cls.__new__(cls)
         memo[id(self)] = result
-        for k, v in self.__dict__.items():
+        for k, v in list(self.__dict__.items()):
             setattr(result, k, deepcopy(v, memo))
         result._lock = False
-        for k, v in self.items():
+        for k, v in list(self.items()):
             result[k] = deepcopy(v, memo)
         return result
 
 
 DEFAULT_CONFIG_PATH = os.path.join(
     os.path.dirname(os.path.dirname(os.path.dirname(__file__))),
     "configs",
 )
@@ -160,44 +168,55 @@ def parse_config_file(file_name, quiet=F
             if os.path.exists(os.path.join(path, file_name)):
                 file_path = os.path.join(path, file_name)
                 break
         else:
             raise IOError("Can't find %s in %s!" % (file_name, search_path))
     if file_name.endswith('.py'):
         global_dict = {}
         local_dict = {}
-        execfile(file_path, global_dict, local_dict)
+        exec(
+            compile(
+                open(
+                    file_path,
+                    "rb").read(),
+                file_path,
+                'exec'),
+            global_dict,
+            local_dict)
         config = local_dict[config_dict_name]
     elif file_name.endswith('.json'):
         fh = open(file_path)
         config = {}
         json_config = json.load(fh)
         config = dict(json_config)
         fh.close()
     else:
         raise RuntimeError(
-            "Unknown config file type %s! (config files must end in .json or .py)" % file_name)
+            "Unknown config file type %s! (config files must end in .json or .py)" %
+            file_name)
     # TODO return file_path
     return config
 
 
 def download_config_file(url, file_name):
     n = 0
     attempts = 5
     sleeptime = 60
     max_sleeptime = 5 * 60
     while True:
         if n >= attempts:
-            print("Failed to download from url %s after %d attempts, quiting..." % (url, attempts))
+            print(
+                "Failed to download from url %s after %d attempts, quiting..." %
+                (url, attempts))
             raise SystemError(-1)
         try:
-            contents = urllib2.urlopen(url, timeout=30).read()
+            contents = urlopen(url, timeout=30).read()
             break
-        except urllib2.URLError as e:
+        except URLError as e:
             print("Error downloading from url %s: %s" % (url, str(e)))
         except socket.timeout as e:
             print("Time out accessing %s: %s" % (url, str(e)))
         except socket.error as e:
             print("Socket error when accessing %s: %s" % (url, str(e)))
         print("Sleeping %d seconds before retrying" % sleeptime)
         time.sleep(sleeptime)
         sleeptime = sleeptime * 2
@@ -205,30 +224,39 @@ def download_config_file(url, file_name)
             sleeptime = max_sleeptime
         n += 1
 
     try:
         f = open(file_name, 'w')
         f.write(contents)
         f.close()
     except IOError as e:
-        print("Error writing downloaded contents to file %s: %s" % (file_name, str(e)))
+        print(
+            "Error writing downloaded contents to file %s: %s" %
+            (file_name, str(e)))
         raise SystemError(-1)
 
 
 # BaseConfig {{{1
 class BaseConfig(object):
     """Basic config setting/getting.
     """
-    def __init__(self, config=None, initial_config_file=None, config_options=None,
-                 all_actions=None, default_actions=None,
-                 volatile_config=None, option_args=None,
-                 require_config_file=False,
-                 append_env_variables_from_configs=False,
-                 usage="usage: %prog [options]"):
+
+    def __init__(
+            self,
+            config=None,
+            initial_config_file=None,
+            config_options=None,
+            all_actions=None,
+            default_actions=None,
+            volatile_config=None,
+            option_args=None,
+            require_config_file=False,
+            append_env_variables_from_configs=False,
+            usage="usage: %prog [options]"):
         self._config = {}
         self.all_cfg_files_and_dicts = []
         self.actions = []
         self.config_lock = False
         self.require_config_file = require_config_file
         # It allows to append env variables from multiple config files
         self.append_env_variables_from_configs = append_env_variables_from_configs
 
@@ -262,17 +290,18 @@ class BaseConfig(object):
             # parse sys.argv which in this case would be the command line
             # options specified to run the tests, e.g. nosetests -v. Clearly,
             # the options passed to nosetests (such as -v) should not be
             # interpreted by mozharness as mozharness options, so we specify
             # a dummy command line with no options, so that the parser does
             # not add anything from the test invocation command line
             # arguments to the mozharness options.
             if option_args is None:
-                option_args = ['dummy_mozharness_script_with_no_command_line_options.py']
+                option_args = [
+                    'dummy_mozharness_script_with_no_command_line_options.py']
         if config_options is None:
             config_options = []
         self._create_config_parser(config_options, usage)
         # we allow manually passing of option args for things like nosetests
         self.parse_args(args=option_args)
 
     def get_read_only_config(self):
         return ReadOnlyDict(self._config)
@@ -280,35 +309,44 @@ class BaseConfig(object):
     def _create_config_parser(self, config_options, usage):
         self.config_parser = ExtendedOptionParser(usage=usage)
         self.config_parser.add_option(
             "--work-dir", action="store", dest="work_dir",
             type="string", default="build",
             help="Specify the work_dir (subdir of base_work_dir)"
         )
         self.config_parser.add_option(
-            "--base-work-dir", action="store", dest="base_work_dir",
-            type="string", default=os.getcwd(),
-            help="Specify the absolute path of the parent of the working directory"
-        )
+            "--base-work-dir",
+            action="store",
+            dest="base_work_dir",
+            type="string",
+            default=os.getcwd(),
+            help="Specify the absolute path of the parent of the working directory")
         self.config_parser.add_option(
-            "--extra-config-path", action='extend', dest="config_paths",
-            type="string", help="Specify additional paths to search for config files.",
+            "--extra-config-path",
+            action='extend',
+            dest="config_paths",
+            type="string",
+            help="Specify additional paths to search for config files.",
         )
         self.config_parser.add_option(
             "-c", "--config-file", "--cfg", action="extend",
             dest="config_files", default=[], type="string",
             help="Specify a config file; can be repeated",
         )
         self.config_parser.add_option(
-            "-C", "--opt-config-file", "--opt-cfg", action="extend",
-            dest="opt_config_files", type="string", default=[],
+            "-C",
+            "--opt-config-file",
+            "--opt-cfg",
+            action="extend",
+            dest="opt_config_files",
+            type="string",
+            default=[],
             help="Specify an optional config file, like --config-file but with no "
-                 "error if the file is missing; can be repeated"
-        )
+            "error if the file is missing; can be repeated")
         self.config_parser.add_option(
             "--dump-config", action="store_true",
             dest="dump_config",
             help="List and dump the config generated from this run to "
                  "a JSON file."
         )
         self.config_parser.add_option(
             "--dump-config-hierarchy", action="store_true",
@@ -417,17 +455,18 @@ class BaseConfig(object):
 
     def verify_actions_order(self, action_list):
         try:
             indexes = [self.all_actions.index(elt) for elt in action_list]
             sorted_indexes = sorted(indexes)
             for i in range(len(indexes)):
                 if indexes[i] != sorted_indexes[i]:
                     print(("Action %s comes in different order in %s\n" +
-                           "than in %s") % (action_list[i], action_list, self.all_actions))
+                           "than in %s") %
+                          (action_list[i], action_list, self.all_actions))
                     raise SystemExit(-1)
         except ValueError as e:
             print("Invalid action found: " + str(e))
             raise SystemExit(-1)
 
     def list_actions(self):
         print("Actions available:")
         for a in self.all_actions:
@@ -475,17 +514,18 @@ class BaseConfig(object):
                     print(
                         "WARNING: optional config file not found %s" % cf
                     )
                 else:
                     raise
 
         if 'EXTRA_MOZHARNESS_CONFIG' in os.environ:
             env_config = json.loads(os.environ['EXTRA_MOZHARNESS_CONFIG'])
-            all_cfg_files_and_dicts.append(("[EXTRA_MOZHARENSS_CONFIG]", env_config))
+            all_cfg_files_and_dicts.append(
+                ("[EXTRA_MOZHARENSS_CONFIG]", env_config))
 
         return all_cfg_files_and_dicts
 
     def parse_args(self, args=None):
         """Parse command line arguments in a generic way.
         Return the parser object after adding the basic options, so
         child objects can manipulate it.
         """
@@ -512,44 +552,44 @@ class BaseConfig(object):
             options.config_files + options.opt_config_files, options=options
         ))
         config = {}
         if (self.append_env_variables_from_configs
                 or options.append_env_variables_from_configs):
             # We only append values from various configs for the 'env' entry
             # For everything else we follow the standard behaviour
             for i, (c_file, c_dict) in enumerate(self.all_cfg_files_and_dicts):
-                for v in c_dict.keys():
+                for v in list(c_dict.keys()):
                     if v == 'env' and v in config:
                         config[v].update(c_dict[v])
                     else:
                         config[v] = c_dict[v]
         else:
             for i, (c_file, c_dict) in enumerate(self.all_cfg_files_and_dicts):
                 config.update(c_dict)
         # assign or update self._config depending on if it exists or not
         #    NOTE self._config will be passed to ReadOnlyConfig's init -- a
         #    dict subclass with immutable locking capabilities -- and serve
         #    as the keys/values that make up that instance. Ultimately,
         #    this becomes self.config during BaseScript's init
         self.set_config(config)
 
-        for key in defaults.keys():
+        for key in list(defaults.keys()):
             value = getattr(options, key)
             if value is None:
                 continue
             # Don't override config_file defaults with config_parser defaults
             if key in defaults and value == defaults[key] and key in self._config:
                 continue
             self._config[key] = value
 
         # 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():
+        for key in list(self.volatile_config.keys()):
             if self._config.get(key) is not None:
                 self.volatile_config[key] = self._config[key]
                 del(self._config[key])
 
         self.update_actions()
         if options.list_actions:
             self.list_actions()
 
@@ -575,17 +615,18 @@ class BaseConfig(object):
 
         Otherwise, if we specify --add-action ACTION, we want to add an
         action to the list.
 
         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'])
+            default_actions = self.verify_actions(
+                self._config['default_actions'])
             self.default_actions = default_actions
         self.verify_actions_order(self.default_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'])
--- a/testing/mozharness/mozharness/base/diskutils.py
+++ b/testing/mozharness/mozharness/base/diskutils.py
@@ -20,19 +20,22 @@
     try:
         file_size = convert_to(file_size, from_unit='bytes', to_unit='GB')
     except DiskutilsError as e:
         # manage the exception e.g: log.error(e)
         pass
 
 """
 import ctypes
+import logging
 import os
 import sys
-import logging
+
+from six import string_types
+
 from mozharness.base.log import INFO, numeric_log_level
 
 # use mozharness log
 log = logging.getLogger(__name__)
 
 
 class DiskutilsError(Exception):
     """Exception thrown by Diskutils module"""
@@ -54,23 +57,27 @@ def convert_to(size, from_unit, to_unit)
              'MB': 1024 * 1024,
              'GB': 1024 * 1024 * 1024,
              'TB': 1024 * 1024 * 1024 * 1024}
     try:
         df = sizes[to_unit]
         sf = sizes[from_unit]
         return size * sf / df
     except KeyError:
-        raise DiskutilsError('conversion error: Invalid source or destination format')
+        raise DiskutilsError(
+            'conversion error: Invalid source or destination format')
     except TypeError:
-        raise DiskutilsError('conversion error: size (%s) is not a number' % size)
+        raise DiskutilsError(
+            'conversion error: size (%s) is not a number' %
+            size)
 
 
 class DiskInfo(object):
     """Stores basic information about the disk"""
+
     def __init__(self):
         self.unit = 'bytes'
         self.free = 0
         self.used = 0
         self.total = 0
 
     def __str__(self):
         string = ['Disk space info (in %s)' % self.unit]
@@ -79,17 +86,20 @@ class DiskInfo(object):
         string += ['free: %s' % self.free]
         return " ".join(string)
 
     def _to(self, unit):
         from_unit = self.unit
         to_unit = unit
         self.free = convert_to(self.free, from_unit=from_unit, to_unit=to_unit)
         self.used = convert_to(self.used, from_unit=from_unit, to_unit=to_unit)
-        self.total = convert_to(self.total, from_unit=from_unit, to_unit=to_unit)
+        self.total = convert_to(
+            self.total,
+            from_unit=from_unit,
+            to_unit=to_unit)
         self.unit = unit
 
 
 class DiskSize(object):
     """DiskSize object
     """
     @staticmethod
     def _posix_size(path):
@@ -111,17 +121,17 @@ class DiskSize(object):
         # DLL call
         disk_info = DiskInfo()
         dummy = ctypes.c_ulonglong()  # needed by the dll call but not used
         total = ctypes.c_ulonglong()  # stores the total space value
         free = ctypes.c_ulonglong()   # stores the free space value
         # depending on path format (unicode or not) and python version (2 or 3)
         # we need to call GetDiskFreeSpaceExW or GetDiskFreeSpaceExA
         called_function = ctypes.windll.kernel32.GetDiskFreeSpaceExA
-        if isinstance(path, unicode) or sys.version_info >= (3,):
+        if isinstance(path, string_types) or sys.version_info >= (3,):
             called_function = ctypes.windll.kernel32.GetDiskFreeSpaceExW
         # we're ready for the dll call. On error it returns 0
         if called_function(path,
                            ctypes.byref(dummy),
                            ctypes.byref(total),
                            ctypes.byref(free)) != 0:
             # success, we can use the values returned by the dll call
             disk_info.free = free.value