Re: [PATCH 00/13] git-p4: python3 compatibility

[Ben Keene <seraphire@gmail.com>]

Here's a patch I have on my tree that I would offer -
it removes references to basestring and should be a drop in patch.


 From 1cc3c0f8570adb1ef2bacc0009aac979a3263d70 Mon Sep 17 00:00:00 2001
From: Ben Keene <seraphire@gmail.com>
Date: Tue, 3 Dec 2019 16:36:26 -0500
Subject: [PATCH] git-p4: change the expansion test from basestring to list

Python 3 handles strings differently than Python 2.7. Since Python 2
is reaching it's end of life, a series of changes are being submitted to
enable python 3.5 and following support. The current code fails basic
tests under python 3.5.

Some codepaths can represent a command line the program
internally prepares to execute either as a single string
(i.e. each token properly quoted, concatenated with $IFS) or
as a list of argv[] elements, and there are 9 places where
we say "if X is isinstance(_, basestring), then do this
thing to handle X as a command line in a single string; if
not, X is a command line in a list form".

This does not work well with Python 3, as there is no
basestring (everything is Unicode now), and even with Python
2, it was not an ideal way to tell the two cases apart,
because an internally formed command line could have been in
a single Unicode string.

Flip the check to say "if X is not a list, then handle X as
a command line in a single string; otherwise treat it as a
command line in a list form".

This will get rid of references to 'basestring', to migrate
the code ready for Python 3.

Thanks-to: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ben Keene <seraphire@gmail.com>
---
  git-p4.py | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 25d8012e23..d322ae20ef 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -98,7 +98,7 @@ def p4_build_cmd(cmd):
          # Provide a way to not pass this option by setting 
git-p4.retries to 0
          real_cmd += ["-r", str(retries)]

-    if isinstance(cmd,basestring):
+    if not isinstance(cmd, list):
          real_cmd = ' '.join(real_cmd) + ' ' + cmd
      else:
          real_cmd += cmd
@@ -192,7 +192,7 @@ def write_pipe(c, stdin):
      if verbose:
          sys.stderr.write('Writing pipe: %s\n' % str(c))

-    expand = isinstance(c,basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdin=subprocess.PIPE, shell=expand)
      pipe = p.stdin
      val = pipe.write(stdin)
@@ -214,7 +214,7 @@ def read_pipe_full(c):
      if verbose:
          sys.stderr.write('Reading pipe: %s\n' % str(c))

-    expand = isinstance(c,basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE, shell=expand)
      (out, err) = p.communicate()
      return (p.returncode, out, decode_text_stream(err))
@@ -254,7 +254,7 @@ def read_pipe_lines(c):
      if verbose:
          sys.stderr.write('Reading pipe: %s\n' % str(c))

-    expand = isinstance(c, basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
      pipe = p.stdout
      val = [decode_text_stream(line) for line in pipe.readlines()]
@@ -297,7 +297,7 @@ def p4_has_move_command():
      return True

  def system(cmd, ignore_error=False):
-    expand = isinstance(cmd,basestring)
+    expand = not isinstance(cmd, list)
      if verbose:
          sys.stderr.write("executing %s\n" % str(cmd))
      retcode = subprocess.call(cmd, shell=expand)
@@ -309,7 +309,7 @@ def system(cmd, ignore_error=False):
  def p4_system(cmd):
      """Specifically invoke p4 as the system command. """
      real_cmd = p4_build_cmd(cmd)
-    expand = isinstance(real_cmd, basestring)
+    expand = not isinstance(real_cmd, list)
      retcode = subprocess.call(real_cmd, shell=expand)
      if retcode:
          raise CalledProcessError(retcode, real_cmd)
@@ -547,7 +547,7 @@ def getP4OpenedType(file):
  # Return the set of all p4 labels
  def getP4Labels(depotPaths):
      labels = set()
-    if isinstance(depotPaths,basestring):
+    if not isinstance(depotPaths, list):
          depotPaths = [depotPaths]

      for l in p4CmdList(["labels"] + ["%s..." % p for p in depotPaths]):
@@ -633,7 +633,7 @@ def isModeExecChanged(src_mode, dst_mode):
  def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
          errors_as_exceptions=False):

-    if isinstance(cmd,basestring):
+    if not isinstance(cmd, list):
          cmd = "-G " + cmd
          expand = True
      else:
@@ -650,7 +650,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', 
cb=None, skip_info=False,
      stdin_file = None
      if stdin is not None:
          stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', 
mode=stdin_mode)
-        if isinstance(stdin,basestring):
+        if not isinstance(stdin, list):
              stdin_file.write(stdin)
          else:
              for i in stdin:
-- 
2.24.1.windows.2