diff mbox series

[3/6] git-p4: Removed support for Python 2

Message ID 20211209201029.136886-4-jholdsworth@nvidia.com (mailing list archive)
State Superseded
Headers show
Series Transition git-p4.py to support Python 3 only | expand

Commit Message

Joel Holdsworth Dec. 9, 2021, 8:10 p.m. UTC
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
 git-p4.py | 89 +++++++++++++++++--------------------------------------
 1 file changed, 28 insertions(+), 61 deletions(-)

Comments

Junio C Hamano Dec. 9, 2021, 10:44 p.m. UTC | #1
Joel Holdsworth <jholdsworth@nvidia.com> writes:

> Subject: Re: [PATCH 3/6] git-p4: Removed support for Python 2

"Removed" -> "remove".

Losing unused/no longer usable code is good.

> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
>  git-p4.py | 89 +++++++++++++++++--------------------------------------
>  1 file changed, 28 insertions(+), 61 deletions(-)

In these 28 new/replacement lines, there is nothing that deserves
any mention in the proposed log message?
Randall S. Becker Dec. 9, 2021, 11:07 p.m. UTC | #2
On December 9, 2021 5:45 PM, Junio C Hamano wrote:
> Joel Holdsworth <jholdsworth@nvidia.com> writes:
> 
> > Subject: Re: [PATCH 3/6] git-p4: Removed support for Python 2
> 
> "Removed" -> "remove".
> 
> Losing unused/no longer usable code is good.
> 
> > Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> > ---
> >  git-p4.py | 89
> > +++++++++++++++++--------------------------------------
> >  1 file changed, 28 insertions(+), 61 deletions(-)
> 
> In these 28 new/replacement lines, there is nothing that deserves any
> mention in the proposed log message?

Just a reminder that Python 2 is the only option available on some older
(still supported) platforms for the next year or so.

Sincerely,
Randall
David Aguilar Dec. 10, 2021, 3:25 a.m. UTC | #3
On Thu, Dec 9, 2021 at 1:13 PM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
>
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
>  git-p4.py | 89 +++++++++++++++++--------------------------------------
>  1 file changed, 28 insertions(+), 61 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 32f30e5f9a..b5d4fc1176 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3
>  #
>  # git-p4.py -- A tool for bidirectional operation between a Perforce depot and git.
>  #
> @@ -16,8 +16,8 @@
>  # pylint: disable=too-many-branches,too-many-nested-blocks
>  #
>  import sys
> -if sys.version_info.major < 3 and sys.version_info.minor < 7:
> -    sys.stderr.write("git-p4: requires Python 2.7 or later.\n")
> +if sys.version_info.major < 3 or (sys.version_info.major == 3 and sys.version_info.minor < 7):
> +    sys.stderr.write("git-p4: requires Python 3.7 or later.\n")
>      sys.exit(1)
>  import os
>  import optparse


There are pretty large user bases on centos/rhel 7+8 where python3.6
is the default version.

If we don't necessarily require any features from 3.7 then it might be
worth lowering this constraint to allow 3.6 or maybe even 3.4 to
maximize the number of users that would benefit.

I realize these python versions are retired according to the python
core team, but I tend to be a little more sympathetic to users when
it doesn't have any impact on the code.

--
David
Joel Holdsworth Dec. 10, 2021, 10:44 a.m. UTC | #4
> There are pretty large user bases on centos/rhel 7+8 where python3.6 is the
> default version.
> 
> If we don't necessarily require any features from 3.7 then it might be worth
> lowering this constraint to allow 3.6 or maybe even 3.4 to maximize the
> number of users that would benefit.
> 
> I realize these python versions are retired according to the python core
> team, but I tend to be a little more sympathetic to users when it doesn't
> have any impact on the code.

I don't mind 3.6 - I don't need any features from 3.7 or newer.

I draw the line at Python 2, though.
diff mbox series

Patch

diff --git a/git-p4.py b/git-p4.py
index 32f30e5f9a..b5d4fc1176 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1,4 +1,4 @@ 
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 # git-p4.py -- A tool for bidirectional operation between a Perforce depot and git.
 #
@@ -16,8 +16,8 @@ 
 # pylint: disable=too-many-branches,too-many-nested-blocks
 #
 import sys
-if sys.version_info.major < 3 and sys.version_info.minor < 7:
-    sys.stderr.write("git-p4: requires Python 2.7 or later.\n")
+if sys.version_info.major < 3 or (sys.version_info.major == 3 and sys.version_info.minor < 7):
+    sys.stderr.write("git-p4: requires Python 3.7 or later.\n")
     sys.exit(1)
 import os
 import optparse
@@ -36,16 +36,6 @@ 
 import errno
 import glob
 
-# On python2.7 where raw_input() and input() are both availble,
-# we want raw_input's semantics, but aliased to input for python3
-# compatibility
-# support basestring in python3
-try:
-    if raw_input and input:
-        input = raw_input
-except:
-    pass
-
 verbose = False
 
 # Only labels/tags matching this will be imported/exported
@@ -173,35 +163,16 @@  def prompt(prompt_text):
         if response in choices:
             return response
 
-# We need different encoding/decoding strategies for text data being passed
-# around in pipes depending on python version
-if bytes is not str:
-    # For python3, always encode and decode as appropriate
-    def decode_text_stream(s):
-        return s.decode() if isinstance(s, bytes) else s
-    def encode_text_stream(s):
-        return s.encode() if isinstance(s, str) else s
-else:
-    # For python2.7, pass read strings as-is, but also allow writing unicode
-    def decode_text_stream(s):
-        return s
-    def encode_text_stream(s):
-        return s.encode('utf_8') if isinstance(s, unicode) else s
+def decode_text_stream(s):
+    return s.decode() if isinstance(s, bytes) else s
+def encode_text_stream(s):
+    return s.encode() if isinstance(s, str) else s
 
 def decode_path(path):
     """Decode a given string (bytes or otherwise) using configured path encoding options
     """
     encoding = gitConfig('git-p4.pathEncoding') or 'utf_8'
-    if bytes is not str:
-        return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path
-    else:
-        try:
-            path.decode('ascii')
-        except:
-            path = path.decode(encoding, errors='replace')
-            if verbose:
-                print('Path with non-ASCII characters detected. Used {} to decode: {}'.format(encoding, path))
-        return path
+    return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path
 
 def run_git_hook(cmd, param=[]):
     """Execute a hook if the hook exists."""
@@ -285,8 +256,8 @@  def write_pipe(c, stdin, *k, **kw):
 
 def p4_write_pipe(c, stdin):
     real_cmd = p4_build_cmd(c)
-    if bytes is not str and isinstance(stdin, str):
-        stdin = encode_text_stream(stdin)
+    if isinstance(stdin, str):
+        stdin = stdin.encode()
     return write_pipe(real_cmd, stdin)
 
 def read_pipe_full(c, *k, **kw):
@@ -745,21 +716,18 @@  def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
     result = []
     try:
         while True:
-            entry = marshal.load(p4.stdout)
-            if bytes is not str:
-                # Decode unmarshalled dict to use str keys and values, except for:
-                #   - `data` which may contain arbitrary binary data
-                #   - `depotFile[0-9]*`, `path`, or `clientFile` which may contain non-UTF8 encoded text
-                decoded_entry = {}
-                for key, value in entry.items():
-                    key = key.decode()
-                    if isinstance(value, bytes) and not (key in ('data', 'path', 'clientFile') or key.startswith('depotFile')):
-                        value = value.decode()
-                    decoded_entry[key] = value
-                # Parse out data if it's an error response
-                if decoded_entry.get('code') == 'error' and 'data' in decoded_entry:
-                    decoded_entry['data'] = decoded_entry['data'].decode()
-                entry = decoded_entry
+            # Decode unmarshalled dict to use str keys and values, except for:
+            #   - `data` which may contain arbitrary binary data
+            #   - `depotFile[0-9]*`, `path`, or `clientFile` which may contain non-UTF8 encoded text
+            entry = {}
+            for key, value in marshal.load(p4.stdout).items():
+                key = key.decode()
+                if isinstance(value, bytes) and not (key in ('data', 'path', 'clientFile') or key.startswith('depotFile')):
+                    value = value.decode()
+                entry[key] = value
+            # Parse out data if it's an error response
+            if entry.get('code') == 'error' and 'data' in entry:
+                entry['data'] = entry['data'].decode()
             if skip_info:
                 if 'code' in entry and entry['code'] == 'info':
                     continue
@@ -3822,14 +3790,13 @@  def openStreams(self):
         self.gitStream = self.importProcess.stdin
         self.gitError = self.importProcess.stderr
 
-        if bytes is not str:
-            # Wrap gitStream.write() so that it can be called using `str` arguments
-            def make_encoded_write(write):
-                def encoded_write(s):
-                    return write(s.encode() if isinstance(s, str) else s)
-                return encoded_write
+        # Wrap gitStream.write() so that it can be called using `str` arguments
+        def make_encoded_write(write):
+            def encoded_write(s):
+                return write(s.encode() if isinstance(s, str) else s)
+            return encoded_write
 
-            self.gitStream.write = make_encoded_write(self.gitStream.write)
+        self.gitStream.write = make_encoded_write(self.gitStream.write)
 
     def closeStreams(self):
         if self.gitStream is None: