Restore py2/py3 compatibility with basestring

A previous CL replaced "basestring" with "str", because basestring
does not exist in Python 3. However, this broke Python 2's ability
to interoperate with unicode strings. This CL introduces a workaround
(defining basestring to be equivalent to string, if it doesn't exist
already), and restores the references to basestring. This workaround
can be fixed when we're 100% on Python 3.

It also undoes some unnecessary and harder-to-read formatting changes.

Bug: 942522
Change-Id: I4a31ee46dc048134c2e4832b6c44ea00ce341899
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1572441
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

gclient.py

@@ -117,6 +117,14 @@ import subprocess2
 import setup_color
 
 
+# TODO(crbug.com/953884): Remove this when python3 migration is done.
+try:
+  basestring
+except NameError:
+  # pylint: disable=redefined-builtin
+  basestring = str
+
+
 # Singleton object to represent an unset cache_dir (as opposed to a disabled
 # one, e.g. if a spec explicitly says `cache_dir = None`.)
 UNSET_CACHE_DIR = object()
@@ -132,7 +140,7 @@ def ToGNString(value, allow_dicts = True):
   allow_dicts indicates if this function will allow converting dictionaries
   to GN scopes. This is only possible at the top level, you can't nest a
   GN scope in a list, so this should be set to False for recursive calls."""
-  if isinstance(value, str):
+  if isinstance(value, basestring):
     if value.find('\n') >= 0:
       raise GNException("Trying to print a string with a newline in it.")
     return '"' + \
@@ -290,7 +298,7 @@ class DependencySettings(object):
     self._custom_hooks = custom_hooks or []
 
     # Post process the url to remove trailing slashes.
-    if isinstance(self.url, str):
+    if isinstance(self.url, basestring):
       # urls are sometime incorrectly written as proto://host/path/@rev. Replace
       # it to proto://host/path@rev.
       self.set_url(self.url.replace('/@', '@'))
@@ -432,7 +440,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     self._OverrideUrl()
     # This is inherited from WorkItem.  We want the URL to be a resource.
-    if self.url and isinstance(self.url, str):
+    if self.url and isinstance(self.url, basestring):
       # The url is usually given to gclient either as https://blah@123
       # or just https://blah.  The @123 portion is irrelevant.
       self.resources.append(self.url.split('@')[0])
@@ -452,7 +460,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
                    self.url, parsed_url)
       self.set_url(parsed_url)
 
-    elif isinstance(self.url, str):
+    elif isinstance(self.url, basestring):
       parsed_url = urlparse.urlparse(self.url)
       if (not parsed_url[0] and
           not re.match(r'^\w+\@[\w\.-]+\:[\w\/]+', parsed_url[2])):
@@ -741,7 +749,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     if 'recursedeps' in local_scope:
       for ent in local_scope['recursedeps']:
-        if isinstance(ent, str):
+        if isinstance(ent, basestring):
           self.recursedeps[ent] = self.deps_file
         else:  # (depname, depsfilename)
           self.recursedeps[ent[0]] = ent[1]
@@ -1008,7 +1016,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     variables = self.get_vars()
     for arg in self._gn_args:
       value = variables[arg]
-      if isinstance(value, str):
+      if isinstance(value, basestring):
         value = gclient_eval.EvaluateCondition(value, variables)
       lines.append('%s = %s' % (arg, ToGNString(value)))
     with open(os.path.join(self.root.root_dir, self._gn_args_file), 'w') as f:
@@ -1387,21 +1395,6 @@ it or fix the checkout.
     except SyntaxError as e:
       gclient_utils.SyntaxErrorToError('.gclient', e)
 
-    # Supporting Unicode URLs in both Python 2 and 3 is annoying. Try to
-    # convert to ASCII in case the URL doesn't actually have any Unicode
-    # characters, otherwise raise an error.
-    # This isn't an issue on Python 3 because everything's Unicode anyway.
-    if sys.version_info.major == 2:
-      try:
-        url = config_dict['solutions'][0]['url']
-        if isinstance(url, unicode):
-          config_dict['solutions'][0]['url'] = url.encode('ascii')
-      except UnicodeEncodeError:
-        raise gclient_utils.Error(
-            "Invalid .gclient file. The url mustn't be unicode.")
-      except KeyError:
-        pass
-
     # Append any target OS that is not already being enforced to the tuple.
     target_os = config_dict.get('target_os', [])
     if config_dict.get('target_os_only', False):

gclient_eval.py

@@ -19,6 +19,14 @@ else:
   from io import StringIO
 
 
+# TODO(crbug.com/953884): Remove this when python3 migration is done.
+try:
+  basestring
+except NameError:
+  # pylint: disable=redefined-builtin
+  basestring = str
+
+
 class _NodeDict(collections.MutableMapping):
   """Dict-like type that also stores information on AST nodes and tokens."""
   def __init__(self, data, tokens=None):
@@ -78,35 +86,30 @@ def _NodeDictSchema(dict_schema):
 
 # See https://github.com/keleshev/schema for docs how to configure schema.
 _GCLIENT_DEPS_SCHEMA = _NodeDictSchema({
-    schema.Optional(str):
+    schema.Optional(basestring):
         schema.Or(
             None,
-            str,
+            basestring,
             _NodeDictSchema({
                 # Repo and revision to check out under the path
                 # (same as if no dict was used).
-                'url':
-                    schema.Or(None, str),
+                'url': schema.Or(None, basestring),
 
                 # Optional condition string. The dep will only be processed
                 # if the condition evaluates to True.
-                schema.Optional('condition'):
-                    str,
-                schema.Optional('dep_type', default='git'):
-                    str,
+                schema.Optional('condition'): basestring,
+                schema.Optional('dep_type', default='git'): basestring,
             }),
             # CIPD package.
             _NodeDictSchema({
                 'packages': [
                     _NodeDictSchema({
-                        'package': str,
-                        'version': str,
+                        'package': basestring,
+                        'version': basestring,
                     })
                 ],
-                schema.Optional('condition'):
-                    str,
-                schema.Optional('dep_type', default='cipd'):
-                    str,
+                schema.Optional('condition'): basestring,
+                schema.Optional('dep_type', default='cipd'): basestring,
             }),
         ),
 })
@@ -114,26 +117,22 @@ _GCLIENT_DEPS_SCHEMA = _NodeDictSchema({
 _GCLIENT_HOOKS_SCHEMA = [
     _NodeDictSchema({
         # Hook action: list of command-line arguments to invoke.
-        'action': [str],
+        'action': [basestring],
 
         # Name of the hook. Doesn't affect operation.
-        schema.Optional('name'):
-            str,
+        schema.Optional('name'): basestring,
 
         # Hook pattern (regex). Originally intended to limit some hooks to run
         # only when files matching the pattern have changed. In practice, with
         # git, gclient runs all the hooks regardless of this field.
-        schema.Optional('pattern'):
-            str,
+        schema.Optional('pattern'): basestring,
 
         # Working directory where to execute the hook.
-        schema.Optional('cwd'):
-            str,
+        schema.Optional('cwd'): basestring,
 
         # Optional condition string. The hook will only be run
         # if the condition evaluates to True.
-        schema.Optional('condition'):
-            str,
+        schema.Optional('condition'): basestring,
     })
 ]
 
@@ -142,7 +141,7 @@ _GCLIENT_SCHEMA = schema.Schema(
         # List of host names from which dependencies are allowed (whitelist).
         # NOTE: when not present, all hosts are allowed.
         # NOTE: scoped to current DEPS file, not recursive.
-        schema.Optional('allowed_hosts'): [schema.Optional(str)],
+        schema.Optional('allowed_hosts'): [schema.Optional(basestring)],
 
         # Mapping from paths to repo and revision to check out under that path.
         # Applying this mapping to the on-disk checkout is the main purpose
@@ -152,95 +151,87 @@ _GCLIENT_SCHEMA = schema.Schema(
         #
         #   Var(): allows variable substitution (either from 'vars' dict below,
         #          or command-line override)
-        schema.Optional('deps'):
-            _GCLIENT_DEPS_SCHEMA,
+        schema.Optional('deps'): _GCLIENT_DEPS_SCHEMA,
 
         # Similar to 'deps' (see above) - also keyed by OS (e.g. 'linux').
         # Also see 'target_os'.
-        schema.Optional('deps_os'):
-            _NodeDictSchema({
-                schema.Optional(str): _GCLIENT_DEPS_SCHEMA,
-            }),
+        schema.Optional('deps_os'): _NodeDictSchema({
+            schema.Optional(basestring): _GCLIENT_DEPS_SCHEMA,
+        }),
 
         # Dependency to get gclient_gn_args* settings from. This allows these
         # values to be set in a recursedeps file, rather than requiring that
         # they exist in the top-level solution.
-        schema.Optional('gclient_gn_args_from'):
-            str,
+        schema.Optional('gclient_gn_args_from'): basestring,
 
         # Path to GN args file to write selected variables.
-        schema.Optional('gclient_gn_args_file'):
-            str,
+        schema.Optional('gclient_gn_args_file'): basestring,
 
         # Subset of variables to write to the GN args file (see above).
-        schema.Optional('gclient_gn_args'): [schema.Optional(str)],
+        schema.Optional('gclient_gn_args'): [schema.Optional(basestring)],
 
         # Hooks executed after gclient sync (unless suppressed), or explicitly
         # on gclient hooks. See _GCLIENT_HOOKS_SCHEMA for details.
         # Also see 'pre_deps_hooks'.
-        schema.Optional('hooks'):
-            _GCLIENT_HOOKS_SCHEMA,
+        schema.Optional('hooks'): _GCLIENT_HOOKS_SCHEMA,
 
         # Similar to 'hooks', also keyed by OS.
-        schema.Optional('hooks_os'):
-            _NodeDictSchema({
-                schema.Optional(str): _GCLIENT_HOOKS_SCHEMA
-            }),
+        schema.Optional('hooks_os'): _NodeDictSchema({
+            schema.Optional(basestring): _GCLIENT_HOOKS_SCHEMA
+        }),
 
         # Rules which #includes are allowed in the directory.
         # Also see 'skip_child_includes' and 'specific_include_rules'.
-        schema.Optional('include_rules'): [schema.Optional(str)],
+        schema.Optional('include_rules'): [schema.Optional(basestring)],
 
         # Hooks executed before processing DEPS. See 'hooks' for more details.
-        schema.Optional('pre_deps_hooks'):
-            _GCLIENT_HOOKS_SCHEMA,
+        schema.Optional('pre_deps_hooks'): _GCLIENT_HOOKS_SCHEMA,
 
         # Recursion limit for nested DEPS.
-        schema.Optional('recursion'):
-            int,
+        schema.Optional('recursion'): int,
 
         # Whitelists deps for which recursion should be enabled.
         schema.Optional('recursedeps'): [
-            schema.Optional(schema.Or(str, (str, str), [str, str])),
+            schema.Optional(schema.Or(
+                basestring,
+                (basestring, basestring),
+                [basestring, basestring]
+            )),
         ],
 
         # Blacklists directories for checking 'include_rules'.
-        schema.Optional('skip_child_includes'): [schema.Optional(str)],
+        schema.Optional('skip_child_includes'): [schema.Optional(basestring)],
 
         # Mapping from paths to include rules specific for that path.
         # See 'include_rules' for more details.
-        schema.Optional('specific_include_rules'):
-            _NodeDictSchema({
-                schema.Optional(str): [str]
-            }),
+        schema.Optional('specific_include_rules'): _NodeDictSchema({
+            schema.Optional(basestring): [basestring]
+        }),
 
         # List of additional OS names to consider when selecting dependencies
         # from deps_os.
-        schema.Optional('target_os'): [schema.Optional(str)],
+        schema.Optional('target_os'): [schema.Optional(basestring)],
 
         # For recursed-upon sub-dependencies, check out their own dependencies
         # relative to the parent's path, rather than relative to the .gclient
         # file.
-        schema.Optional('use_relative_paths'):
-            bool,
+        schema.Optional('use_relative_paths'): bool,
 
         # For recursed-upon sub-dependencies, run their hooks relative to the
         # parent's path instead of relative to the .gclient file.
-        schema.Optional('use_relative_hooks'):
-            bool,
+        schema.Optional('use_relative_hooks'): bool,
 
         # Variables that can be referenced using Var() - see 'deps'.
-        schema.Optional('vars'):
-            _NodeDictSchema({
-                schema.Optional(str): schema.Or(str, bool),
-            }),
+        schema.Optional('vars'): _NodeDictSchema({
+            schema.Optional(basestring): schema.Or(basestring, bool),
+        }),
     }))
 
 
 def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
   """Safely evaluates a single expression. Returns the result."""
   _allowed_names = {'None': None, 'True': True, 'False': False}
-  if isinstance(node_or_string, str):
+  if isinstance(node_or_string, basestring):
     node_or_string = ast.parse(node_or_string, filename=filename, mode='eval')
   if isinstance(node_or_string, ast.Expression):
     node_or_string = node_or_string.body
@@ -284,7 +275,7 @@ def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
             'Var takes exactly one argument (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
       arg = _convert(node.args[0])
-      if not isinstance(arg, str):
+      if not isinstance(arg, basestring):
         raise ValueError(
             'Var\'s argument must be a variable name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
@@ -410,7 +401,7 @@ def ExecLegacy(content, filename='<unknown>', vars_override=None,
     return local_scope
 
   def _DeepFormat(node):
-    if isinstance(node, str):
+    if isinstance(node, basestring):
       return node.format(**vars_dict)
     elif isinstance(node, dict):
       return {k.format(**vars_dict): _DeepFormat(v) for k, v in node.items()}
@@ -562,7 +553,7 @@ def EvaluateCondition(condition, variables, referenced_variables=None):
 
         # Allow using "native" types, without wrapping everything in strings.
         # Note that schema constraints still apply to variables.
-        if not isinstance(value, str):
+        if not isinstance(value, basestring):
           return value
 
         # Recursively evaluate the variable reference.

gclient_scm.py

@@ -597,8 +597,7 @@ class GitWrapper(SCMWrapper):
     # but whose branch(s) are the same as official repos.
     if (current_url.rstrip(b'/') != url.rstrip('/') and url != 'git://foo' and
         subprocess2.capture(
-            ['git', 'config',
-             'remote.%s.gclient-auto-fix-url' % self.remote],
+            ['git', 'config', 'remote.%s.gclient-auto-fix-url' % self.remote],
             cwd=self.checkout_path).strip() != 'False'):
       self.Print('_____ switching %s to a new upstream' % self.relpath)
       if not (options.force or options.reset):

subprocess2.py

@@ -28,6 +28,14 @@ import threading
 if sys.version_info.major == 2:
   codecs.lookup('string-escape')
 
+# TODO(crbug.com/953884): Remove this when python3 migration is done.
+try:
+  basestring
+except NameError:
+  # pylint: disable=redefined-builtin
+  basestring = str
+
+
 # Constants forwarded from subprocess.
 PIPE = subprocess.PIPE
 STDOUT = subprocess.STDOUT
@@ -214,8 +222,7 @@ class Popen(subprocess.Popen):
       # the list.
       kwargs['shell'] = bool(sys.platform=='win32')
 
-    if isinstance(args, str) or (sys.version_info.major == 2 and
-                                 isinstance(args, unicode)):
+    if isinstance(args, basestring):
       tmp_str = args
     elif isinstance(args, (list, tuple)):
       tmp_str = ' '.join(args)
@@ -458,8 +465,7 @@ def communicate(args, timeout=None, nag_timer=None, nag_max=None, **kwargs):
   """
   stdin = kwargs.pop('stdin', None)
   if stdin is not None:
-    if isinstance(stdin, str) or (sys.version_info.major == 2 and
-                                  isinstance(stdin, unicode)):
+    if isinstance(stdin, basestring):
       # When stdin is passed as an argument, use it as the actual input data and
       # set the Popen() parameter accordingly.
       kwargs['stdin'] = PIPE