{"thread":{"id":"57065","subject":"[PATCH v2 1/3] git-p4: remove support for Python 2","startedAt":"2021-12-10T15:31:37Z","lastAt":"2021-12-13T00:11:59Z","messageCount":9,"participants":["Joel Holdsworth","Luke Diamand","Elijah Newren","Andrew Oakley","Ævar Arnfjörð Bjarmason","brian m. carlson"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"443819","messageId":"20211210153101.35433-2-jholdsworth@nvidia.com","threadId":"57065","inReplyTo":"20211210153101.35433-1-jholdsworth@nvidia.com","subject":"[PATCH v2 1/3] git-p4: remove support for Python 2","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-10T15:30:59Z","receivedAt":"2021-12-10T15:31:37Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"git-p4 previously contained seperate code-paths for Python 2 and 3 to\nabstract away the differences in string handling behaviour between the\ntwo platforms.\n\nThis patch removes the Python 2 code-paths within this abstraction\nwithout removing the abstractions themselves. These will be removed in\nlater patches to further modernise the script.\n\nThe motivation for this change is that there is a family of issues with\ngit-p4's handling of incoming text data when it contains bytes which\ncannot be decoded into UTF-8 characters. For text files created in\nWindows, CP1252 Smart Quote Characters (0x93 and 0x94) are seen fairly\nfrequently. These codes are invalid in UTF-8, so if the script\nencounters any file or file name containing them, on Python 2 the\nsymbols will be corrupted, and on Python 3 the script will fail with an\nexception.\n\nIn order to address these issues it will be necessary to overhaul\ngit-p4's handling of incoming data. Keeping a clean separation between\nencoded bytes and decoded text is much easier to do in Python 3. If\nPython 2 support must be maintained, this will require careful testing\nof the separate code paths for each platform, which is unreasonable\ngiven that Python 2 is now thoroughly deprecated.\n\nThe minimum supported Python version has been set to 3.6. This version\nis no longer supported by the Python project, however at the current\ntime it is still available for use in RHEL 8. No features from newer\nversions of Python are currently required.\n\nSigned-off-by: Joel Holdsworth <jholdsworth@nvidia.com>\n---\n git-p4.py | 90 ++++++++++++++++++-------------------------------------\n 1 file changed, 29 insertions(+), 61 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 2b4500226a..e3fe86e4f2 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1,4 +1,4 @@\n-#!/usr/bin/env python\n+#!/usr/bin/env python3\n #\n # git-p4.py -- A tool for bidirectional operation between a Perforce depot and git.\n #\n@@ -16,8 +16,9 @@\n # pylint: disable=too-many-branches,too-many-nested-blocks\n #\n import sys\n-if sys.version_info.major < 3 and sys.version_info.minor < 7:\n-    sys.stderr.write(\"git-p4: requires Python 2.7 or later.\\n\")\n+if (sys.version_info.major < 3 or\n+    (sys.version_info.major == 3 and sys.version_info.minor < 6)):\n+    sys.stderr.write(\"git-p4: requires Python 3.6 or later.\\n\")\n     sys.exit(1)\n import os\n import optparse\n@@ -36,16 +37,6 @@\n import errno\n import glob\n \n-# On python2.7 where raw_input() and input() are both availble,\n-# we want raw_input's semantics, but aliased to input for python3\n-# compatibility\n-# support basestring in python3\n-try:\n-    if raw_input and input:\n-        input = raw_input\n-except:\n-    pass\n-\n verbose = False\n \n # Only labels/tags matching this will be imported/exported\n@@ -176,35 +167,16 @@ def prompt(prompt_text):\n         if response in choices:\n             return response\n \n-# We need different encoding/decoding strategies for text data being passed\n-# around in pipes depending on python version\n-if bytes is not str:\n-    # For python3, always encode and decode as appropriate\n-    def decode_text_stream(s):\n-        return s.decode() if isinstance(s, bytes) else s\n-    def encode_text_stream(s):\n-        return s.encode() if isinstance(s, str) else s\n-else:\n-    # For python2.7, pass read strings as-is, but also allow writing unicode\n-    def decode_text_stream(s):\n-        return s\n-    def encode_text_stream(s):\n-        return s.encode('utf_8') if isinstance(s, unicode) else s\n+def decode_text_stream(s):\n+    return s.decode() if isinstance(s, bytes) else s\n+def encode_text_stream(s):\n+    return s.encode() if isinstance(s, str) else s\n \n def decode_path(path):\n     \"\"\"Decode a given string (bytes or otherwise) using configured path encoding options\n     \"\"\"\n     encoding = gitConfig('git-p4.pathEncoding') or 'utf_8'\n-    if bytes is not str:\n-        return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path\n-    else:\n-        try:\n-            path.decode('ascii')\n-        except:\n-            path = path.decode(encoding, errors='replace')\n-            if verbose:\n-                print('Path with non-ASCII characters detected. Used {} to decode: {}'.format(encoding, path))\n-        return path\n+    return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path\n \n def run_git_hook(cmd, param=[]):\n     \"\"\"Execute a hook if the hook exists.\"\"\"\n@@ -289,8 +261,8 @@ def write_pipe(c, stdin):\n \n def p4_write_pipe(c, stdin):\n     real_cmd = p4_build_cmd(c)\n-    if bytes is not str and isinstance(stdin, str):\n-        stdin = encode_text_stream(stdin)\n+    if isinstance(stdin, str):\n+        stdin = stdin.encode()\n     return write_pipe(real_cmd, stdin)\n \n def read_pipe_full(c):\n@@ -762,21 +734,18 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,\n     result = []\n     try:\n         while True:\n-            entry = marshal.load(p4.stdout)\n-            if bytes is not str:\n-                # Decode unmarshalled dict to use str keys and values, except for:\n-                #   - `data` which may contain arbitrary binary data\n-                #   - `depotFile[0-9]*`, `path`, or `clientFile` which may contain non-UTF8 encoded text\n-                decoded_entry = {}\n-                for key, value in entry.items():\n-                    key = key.decode()\n-                    if isinstance(value, bytes) and not (key in ('data', 'path', 'clientFile') or key.startswith('depotFile')):\n-                        value = value.decode()\n-                    decoded_entry[key] = value\n-                # Parse out data if it's an error response\n-                if decoded_entry.get('code') == 'error' and 'data' in decoded_entry:\n-                    decoded_entry['data'] = decoded_entry['data'].decode()\n-                entry = decoded_entry\n+            # Decode unmarshalled dict to use str keys and values, except for:\n+            #   - `data` which may contain arbitrary binary data\n+            #   - `depotFile[0-9]*`, `path`, or `clientFile` which may contain non-UTF8 encoded text\n+            entry = {}\n+            for key, value in marshal.load(p4.stdout).items():\n+                key = key.decode()\n+                if isinstance(value, bytes) and not (key in ('data', 'path', 'clientFile') or key.startswith('depotFile')):\n+                    value = value.decode()\n+                entry[key] = value\n+            # Parse out data if it's an error response\n+            if entry.get('code') == 'error' and 'data' in entry:\n+                entry['data'] = entry['data'].decode()\n             if skip_info:\n                 if 'code' in entry and entry['code'] == 'info':\n                     continue\n@@ -3840,14 +3809,13 @@ def openStreams(self):\n         self.gitStream = self.importProcess.stdin\n         self.gitError = self.importProcess.stderr\n \n-        if bytes is not str:\n-            # Wrap gitStream.write() so that it can be called using `str` arguments\n-            def make_encoded_write(write):\n-                def encoded_write(s):\n-                    return write(s.encode() if isinstance(s, str) else s)\n-                return encoded_write\n+        # Wrap gitStream.write() so that it can be called using `str` arguments\n+        def make_encoded_write(write):\n+            def encoded_write(s):\n+                return write(s.encode() if isinstance(s, str) else s)\n+            return encoded_write\n \n-            self.gitStream.write = make_encoded_write(self.gitStream.write)\n+        self.gitStream.write = make_encoded_write(self.gitStream.write)\n \n     def closeStreams(self):\n         if self.gitStream is None:\n-- \n2.33.0\n\n"},{"id":"443820","messageId":"20211210153101.35433-1-jholdsworth@nvidia.com","threadId":"57065","inReplyTo":null,"subject":"[PATCH v2 0/3] Transition git-p4.py to support Python 3 only","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-10T15:30:58Z","receivedAt":"2021-12-10T15:31:39Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The git-p4.py script currently implements code paths for both Python 2\nand 3.\n\nPython 2 was discontinued in 2020, and there is no longer any officially\nsupported interpreter. Further development of git-p4.py will require\nwould-be developers to test their changes with all supported dialects of\nthe language. However, if there is no longer any supported runtime\nenvironment available, this places an unreasonable burden on the Git\nproject to maintain support for an obselete dialect of the language.\n\nThe patch-set removes all Python 2-specific code paths.\n\nThis second revisision of the patch-set is more tightly focussed on the\ntask of retiring Python 2, and excludes previously submitted patches\nthat contain other tidy-ups and bug fixes.\n\nJoel Holdsworth (3):\n  git-p4: remove support for Python 2\n  git-p4: eliminate decode_stream and encode_stream\n  git-p4: add \"Nvidia Corporation\" to copyright header\n\n git-p4.py | 132 +++++++++++++++++++-----------------------------------\n 1 file changed, 45 insertions(+), 87 deletions(-)\n\n-- \n2.33.0\n\n"},{"id":"443821","messageId":"20211210153101.35433-3-jholdsworth@nvidia.com","threadId":"57065","inReplyTo":"20211210153101.35433-1-jholdsworth@nvidia.com","subject":"[PATCH v2 2/3] git-p4: eliminate decode_stream and encode_stream","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-10T15:31:00Z","receivedAt":"2021-12-10T15:31:40Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The decode_stream and encode_stream functions previously abstracted away\nthe differences in string encode/decode behaviour between Python 2 and\nPython 3.\n\nGiven that Python 2 is no longer supported, and the code paths for it\nhave been removed, the abstraction is no longer necessary, and the\nscript can therefore be simplified by eliminating these functions.\n\nSigned-off-by: Joel Holdsworth <jholdsworth@nvidia.com>\n---\n git-p4.py | 49 +++++++++++++++++++------------------------------\n 1 file changed, 19 insertions(+), 30 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex e3fe86e4f2..5568d44c72 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -167,11 +167,6 @@ def prompt(prompt_text):\n         if response in choices:\n             return response\n \n-def decode_text_stream(s):\n-    return s.decode() if isinstance(s, bytes) else s\n-def encode_text_stream(s):\n-    return s.encode() if isinstance(s, str) else s\n-\n def decode_path(path):\n     \"\"\"Decode a given string (bytes or otherwise) using configured path encoding options\n     \"\"\"\n@@ -276,7 +271,7 @@ def read_pipe_full(c):\n     expand = not isinstance(c, list)\n     p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n     (out, err) = p.communicate()\n-    return (p.returncode, out, decode_text_stream(err))\n+    return (p.returncode, out, err.decode())\n \n def read_pipe(c, ignore_error=False, raw=False):\n     \"\"\" Read output from  command. Returns the output text on\n@@ -288,22 +283,17 @@ def read_pipe(c, ignore_error=False, raw=False):\n     (retcode, out, err) = read_pipe_full(c)\n     if retcode != 0:\n         if ignore_error:\n-            out = \"\"\n+            out = b\"\"\n         else:\n             die('Command failed: %s\\nError: %s' % (str(c), err))\n-    if not raw:\n-        out = decode_text_stream(out)\n-    return out\n+    return out if raw else out.decode()\n \n def read_pipe_text(c):\n     \"\"\" Read output from a command with trailing whitespace stripped.\n         On error, returns None.\n     \"\"\"\n     (retcode, out, err) = read_pipe_full(c)\n-    if retcode != 0:\n-        return None\n-    else:\n-        return decode_text_stream(out).rstrip()\n+    return out.decode().rstrip() if retcode == 0 else None\n \n def p4_read_pipe(c, ignore_error=False, raw=False):\n     real_cmd = p4_build_cmd(c)\n@@ -316,7 +306,7 @@ def read_pipe_lines(c):\n     expand = not isinstance(c, list)\n     p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)\n     pipe = p.stdout\n-    val = [decode_text_stream(line) for line in pipe.readlines()]\n+    val = [line.decode() for line in pipe.readlines()]\n     if pipe.close() or p.wait():\n         die('Command failed: %s' % str(c))\n     return val\n@@ -346,7 +336,7 @@ def p4_has_move_command():\n     cmd = p4_build_cmd([\"move\", \"-k\", \"@from\", \"@to\"])\n     p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)\n     (out, err) = p.communicate()\n-    err = decode_text_stream(err)\n+    err = err.decode()\n     # return code will be 1 in either case\n     if err.find(\"Invalid option\") >= 0:\n         return False\n@@ -721,7 +711,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,\n             stdin_file.write(stdin)\n         else:\n             for i in stdin:\n-                stdin_file.write(encode_text_stream(i))\n+                stdin_file.write(i.encode() if isinstance(i, str) else i)\n                 stdin_file.write(b'\\n')\n         stdin_file.flush()\n         stdin_file.seek(0)\n@@ -963,8 +953,7 @@ def branch_exists(branch):\n \n     cmd = [ \"git\", \"rev-parse\", \"--symbolic\", \"--verify\", branch ]\n     p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)\n-    out, _ = p.communicate()\n-    out = decode_text_stream(out)\n+    out = p.communicate()[0].decode()\n     if p.returncode:\n         return False\n     # expect exactly one line of output: the branch name\n@@ -1349,7 +1338,7 @@ def generatePointer(self, contentFile):\n             ['git', 'lfs', 'pointer', '--file=' + contentFile],\n             stdout=subprocess.PIPE\n         )\n-        pointerFile = decode_text_stream(pointerProcess.stdout.read())\n+        pointerFile = pointerProcess.stdout.read().decode()\n         if pointerProcess.wait():\n             os.remove(contentFile)\n             die('git-lfs pointer command failed. Did you install the extension?')\n@@ -2148,7 +2137,7 @@ def applyCommit(self, id):\n         tmpFile = os.fdopen(handle, \"w+b\")\n         if self.isWindows:\n             submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n-        tmpFile.write(encode_text_stream(submitTemplate))\n+        tmpFile.write(submitTemplate.encode())\n         tmpFile.close()\n \n         submitted = False\n@@ -2204,8 +2193,8 @@ def applyCommit(self, id):\n                         return False\n \n                 # read the edited message and submit\n-                tmpFile = open(fileName, \"rb\")\n-                message = decode_text_stream(tmpFile.read())\n+                with open(fileName, \"r\") as tmpFile:\n+                    message = tmpFile.read()\n                 tmpFile.close()\n                 if self.isWindows:\n                     message = message.replace(\"\\r\\n\", \"\\n\")\n@@ -2905,7 +2894,7 @@ def splitFilesIntoBranches(self, commit):\n         return branches\n \n     def writeToGitStream(self, gitMode, relPath, contents):\n-        self.gitStream.write(encode_text_stream(u'M {} inline {}\\n'.format(gitMode, relPath)))\n+        self.gitStream.write('M {} inline {}\\n'.format(gitMode, relPath))\n         self.gitStream.write('data %d\\n' % sum(len(d) for d in contents))\n         for d in contents:\n             self.gitStream.write(d)\n@@ -2947,7 +2936,7 @@ def streamOneP4File(self, file, contents):\n             git_mode = \"120000\"\n             # p4 print on a symlink sometimes contains \"target\\n\";\n             # if it does, remove the newline\n-            data = ''.join(decode_text_stream(c) for c in contents)\n+            data = ''.join(c.decode() for c in contents)\n             if not data:\n                 # Some version of p4 allowed creating a symlink that pointed\n                 # to nothing.  This causes p4 errors when checking out such\n@@ -3001,9 +2990,9 @@ def streamOneP4File(self, file, contents):\n         pattern = p4_keywords_regexp_for_type(type_base, type_mods)\n         if pattern:\n             regexp = re.compile(pattern, re.VERBOSE)\n-            text = ''.join(decode_text_stream(c) for c in contents)\n+            text = ''.join(c.decode() for c in contents)\n             text = regexp.sub(r'$\\1$', text)\n-            contents = [ encode_text_stream(text) ]\n+            contents = [text.encode()]\n \n         if self.largeFileSystem:\n             (git_mode, contents) = self.largeFileSystem.processContent(git_mode, relPath, contents)\n@@ -3015,7 +3004,7 @@ def streamOneP4Deletion(self, file):\n         if verbose:\n             sys.stdout.write(\"delete %s\\n\" % relPath)\n             sys.stdout.flush()\n-        self.gitStream.write(encode_text_stream(u'D {}\\n'.format(relPath)))\n+        self.gitStream.write('D {}\\n'.format(relPath))\n \n         if self.largeFileSystem and self.largeFileSystem.isLargeFile(relPath):\n             self.largeFileSystem.removeLargeFile(relPath)\n@@ -3115,9 +3104,9 @@ def streamP4FilesCbSelf(entry):\n                 if 'shelved_cl' in f:\n                     # Handle shelved CLs using the \"p4 print file@=N\" syntax to print\n                     # the contents\n-                    fileArg = f['path'] + encode_text_stream('@={}'.format(f['shelved_cl']))\n+                    fileArg = f['path'] + '@={}'.format(f['shelved_cl']).encode()\n                 else:\n-                    fileArg = f['path'] + encode_text_stream('#{}'.format(f['rev']))\n+                    fileArg = f['path'] + '#{}'.format(f['rev']).encode()\n \n                 fileArgs.append(fileArg)\n \n-- \n2.33.0\n\n"},{"id":"443822","messageId":"20211210153101.35433-4-jholdsworth@nvidia.com","threadId":"57065","inReplyTo":"20211210153101.35433-1-jholdsworth@nvidia.com","subject":"[PATCH v2 3/3] git-p4: add \"Nvidia Corporation\" to copyright header","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-10T15:31:01Z","receivedAt":"2021-12-10T15:31:43Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The inclusion of the coorporate copyright is a stipulation of the\ncompany code release process.\n---\n git-p4.py | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5568d44c72..17e18265dc 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -5,6 +5,7 @@\n # Author: Simon Hausmann <simon@lst.de>\n # Copyright: 2007 Simon Hausmann <simon@lst.de>\n #            2007 Trolltech ASA\n+#            2021 Nvidia Corporation\n # License: MIT <http://www.opensource.org/licenses/mit-license.php>\n #\n # pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except\n-- \n2.33.0\n\n"},{"id":"443831","messageId":"CAE5ih7-nAOviVmuDbAWXONcY-FkR6xUDu_vTZhWz8_RTpDpsMg@mail.gmail.com","threadId":"57065","inReplyTo":"20211210153101.35433-4-jholdsworth@nvidia.com","subject":"Re: [PATCH v2 3/3] git-p4: add \"Nvidia Corporation\" to copyright header","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2021-12-10T18:57:47Z","receivedAt":"2021-12-10T18:58:02Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Fri, 10 Dec 2021 at 15:31, Joel Holdsworth <jholdsworth@nvidia.com> wrote:\n>\n> The inclusion of the coorporate copyright is a stipulation of the\n> company code release process.\n\nThis doesn't seem right to me.\n\nIt seems very odd that there's a patch that just adds a copyright\nnotice, with no further notice.\n\nWhat code does this cover and is now copyrighted? What are the license\nterms of the copyright holder? I think these things need to be made\nclear before this could be accepted.\n\n> ---\n>  git-p4.py | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 5568d44c72..17e18265dc 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -5,6 +5,7 @@\n>  # Author: Simon Hausmann <simon@lst.de>\n>  # Copyright: 2007 Simon Hausmann <simon@lst.de>\n>  #            2007 Trolltech ASA\n> +#            2021 Nvidia Corporation\n>  # License: MIT <http://www.opensource.org/licenses/mit-license.php>\n>  #\n>  # pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except\n> --\n> 2.33.0\n>\n"},{"id":"443894","messageId":"CABPp-BEyBLzWY2andDXZV6AgkQpnt1sp_rSThy84=qXMt2D8nA@mail.gmail.com","threadId":"57065","inReplyTo":"20211210153101.35433-4-jholdsworth@nvidia.com","subject":"Re: [PATCH v2 3/3] git-p4: add \"Nvidia Corporation\" to copyright header","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-12-11T21:19:18Z","receivedAt":"2021-12-11T21:19:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Dec 10, 2021 at 12:30 PM Joel Holdsworth <jholdsworth@nvidia.com> wrote:\n>\n> The inclusion of the coorporate copyright is a stipulation of the\n> company code release process.\n> ---\n>  git-p4.py | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 5568d44c72..17e18265dc 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -5,6 +5,7 @@\n>  # Author: Simon Hausmann <simon@lst.de>\n>  # Copyright: 2007 Simon Hausmann <simon@lst.de>\n>  #            2007 Trolltech ASA\n> +#            2021 Nvidia Corporation\n>  # License: MIT <http://www.opensource.org/licenses/mit-license.php>\n>  #\n>  # pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except\n> --\n> 2.33.0\n\nCan we just get rid of all these copyright notices from all files in\nGit?  They're obviously out-of-date and not even close to an accurate\nindicator of authorship.  For example, builtin/branch.c has:\n\n * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>\n * Based on git-branch.sh by Junio C Hamano.\n\nKristian only authored 1 patch for this file (though that one patch\nwas submitted and attributed to Lars Hjemli in c31820c26b (\"Make\ngit-branch a builtin\", 2006-10-23) with a note in the commit message\nabout Kristian being the real author).  I added a simple replace\nobject to change the author attribution on Kristian's commit back to\nhim, and then...\n\nLooking at shortlog, there are 86 different authors, with the top 7\nhaving this many commits:\n\n$ git shortlog -sn --no-merges -- builtin/branch.c builtin-branch.c | head -n 7\n    42 Junio C Hamano\n    37 Jeff King\n    30 Nguyễn Thái Ngọc Duy\n    16 Karthik Nayak\n    12 René Scharfe\n    11 Lars Hjemli\n    11 Ævar Arnfjörð Bjarmason\n\nLooking at git blame, there are 61 different authors who have lines of\ncode surviving until today, with the top 7 being:\n\n$ git blame -C -C builtin/branch.c | awk '{print $3 \" \" $4}' | sort |\nuniq -c | sort -rn | head -n 7\n    139 (Junio C\n    129 (Nguyễn Thái\n    122 (Karthik Nayak\n     40 (Jeff King\n     38 (Kristian H�gsberg\n     36 (Sahil Dua\n     35 (René Scharfe\n\nSo the copyright notice is horribly misleading at best.  It also seems\nlike the wrong way to figure out the answer to _any_ question I can\nthink of.  (Some examples: \"Who can review my changes to this file?\",\n\"Who do I need to contact for permission to relicense?\", \"Who should I\npraise for doing the work of making this code so great for me?\", etc.)\n-- in all cases, shortlog, log, and blame are better tools.\n\nCan we just git rid of these lines entirely?\n"},{"id":"443902","messageId":"20211212175054.5d3c11af@ado-tr.dyn.home.arpa","threadId":"57065","inReplyTo":"20211210153101.35433-2-jholdsworth@nvidia.com","subject":"Re: [PATCH v2 1/3] git-p4: remove support for Python 2","fromName":"Andrew Oakley","fromEmail":"andrew@adoakley.name","sentAt":"2021-12-12T17:50:54Z","receivedAt":"2021-12-12T18:17:09Z","isPatch":true,"sender":{"key":"andrew@adoakley.name","avatar":"https://avatars.githubusercontent.com/u/1107440?v=4"},"body":"On Fri, 10 Dec 2021 15:30:59 +0000\nJoel Holdsworth <jholdsworth@nvidia.com> wrote:\n> The motivation for this change is that there is a family of issues\n> with git-p4's handling of incoming text data when it contains bytes\n> which cannot be decoded into UTF-8 characters. For text files created\n> in Windows, CP1252 Smart Quote Characters (0x93 and 0x94) are seen\n> fairly frequently. These codes are invalid in UTF-8, so if the script\n> encounters any file or file name containing them, on Python 2 the\n> symbols will be corrupted, and on Python 3 the script will fail with\n> an exception.\n\nAs I've pointed out previously, peforce fails to store the encoding of\ntext like commit messages.  With Windows perforce clients, the encoding\nused seems to be based on the current code page on the client which\nmade the commit.  If you're part of a global organisation with people\nin different locales making commits then you will find that there is\nnot a consistent encoding for commit messages.\n\nGiven that you don't know the encoding of the text, what's the best\nthing to do with the data?  Options I can see are:\n\n- Feed the raw bytes directly into git.  The i18n.commitEncoding config\n  option can be set by the user if they want to attempt to decode the\n  commit messages in something other than UTF-8.\n- Attempt to detect the encoding somehow, feed the raw bytes directly\n  into git and set the encoding on the commit.\n- Attempt to dedect the encoding somehow and reencode everything into\n  UTF-8.\n\nRight now, if you use python2 then you get the behaviour as described\nin the first of these options.  It doesn't \"corrupt\" anything, it just\ntransfers the bytes from perforce into git.  If you use python3 then\ngit-p4 is unusable because it throws exceptions trying to decode things.\n\nIt's not clear to me how \"attempt to detect the encoding somehow\" would\nwork.  The first option therefore seems like the best choice.\n\nI think that this is the problem which really needs solving.  Dropping\nsupport for python2 doesn't make the issue go away (although it might\nmake it slightly easier to write the code).  I think that the python2\ncompatibility should be maintained at least until the encoding problems\nhave been solved for python3.\n\nI previously wrote some patches which attempt to move in what I think is\nthe right direction, but unfortunately they never got upstreamed:\n\nhttps://lore.kernel.org/git/20210412085251.51475-1-andrew@adoakley.name/\n\nYour comments elsewhere that git-p4 could benifit from some clean-up\nseem accurate to me, and it would be good to see that kind of change.\n"},{"id":"443936","messageId":"211212.86k0g9a1mz.gmgdl@evledraar.gmail.com","threadId":"57065","inReplyTo":"20211212175054.5d3c11af@ado-tr.dyn.home.arpa","subject":"Re: [PATCH v2 1/3] git-p4: remove support for Python 2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-12T22:01:27Z","receivedAt":"2021-12-12T22:39:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Dec 12 2021, Andrew Oakley wrote:\n\n> On Fri, 10 Dec 2021 15:30:59 +0000\n> Joel Holdsworth <jholdsworth@nvidia.com> wrote:\n>> The motivation for this change is that there is a family of issues\n>> with git-p4's handling of incoming text data when it contains bytes\n>> which cannot be decoded into UTF-8 characters. For text files created\n>> in Windows, CP1252 Smart Quote Characters (0x93 and 0x94) are seen\n>> fairly frequently. These codes are invalid in UTF-8, so if the script\n>> encounters any file or file name containing them, on Python 2 the\n>> symbols will be corrupted, and on Python 3 the script will fail with\n>> an exception.\n>\n> As I've pointed out previously, peforce fails to store the encoding of\n> text like commit messages.  With Windows perforce clients, the encoding\n> used seems to be based on the current code page on the client which\n> made the commit.  If you're part of a global organisation with people\n> in different locales making commits then you will find that there is\n> not a consistent encoding for commit messages.\n>\n> Given that you don't know the encoding of the text, what's the best\n> thing to do with the data?  Options I can see are:\n>\n> - Feed the raw bytes directly into git.  The i18n.commitEncoding config\n>   option can be set by the user if they want to attempt to decode the\n>   commit messages in something other than UTF-8.\n> - Attempt to detect the encoding somehow, feed the raw bytes directly\n>   into git and set the encoding on the commit.\n> - Attempt to dedect the encoding somehow and reencode everything into\n>   UTF-8.\n>\n> Right now, if you use python2 then you get the behaviour as described\n> in the first of these options.  It doesn't \"corrupt\" anything, it just\n> transfers the bytes from perforce into git.  If you use python3 then\n> git-p4 is unusable because it throws exceptions trying to decode things.\n>\n> [...]\n>\n> I think that this is the problem which really needs solving.  Dropping\n> support for python2 doesn't make the issue go away (although it might\n> make it slightly easier to write the code).  I think that the python2\n> compatibility should be maintained at least until the encoding problems\n> have been solved for python3.\n>\n> I previously wrote some patches which attempt to move in what I think is\n> the right direction, but unfortunately they never got upstreamed:\n>\n> https://lore.kernel.org/git/20210412085251.51475-1-andrew@adoakley.name/\n>\n> Your comments elsewhere that git-p4 could benifit from some clean-up\n> seem accurate to me, and it would be good to see that kind of change.\n\nThis summary makes sense, i.e. if the original SCM doesn't have a\ndeclared or consistent encoding then having no \"encoding\" header etc. in\ngit likewise makes sense, and we should be trying to handle it in our\noutput layer.\n\n[Snipped from above]:\n\n> It's not clear to me how \"attempt to detect the encoding somehow\" would\n> work.  The first option therefore seems like the best choice.\n\nThis really isn't possible to do in the general case, but you can get\npretty far with heuristics.\n\nThe best way to do this that I'm aware of is Mozilla's character\ndetection library:\n\n    https://www-archive.mozilla.org/projects/intl/chardetinterface\n\nHere's an old (maybe/probably not up-to-date) copy of its sources that\nI've worked with:\n\n    https://metacpan.org/release/JGMYERS/Encode-Detect-1.01/source/src?sort=[[2,1]]\n\nI.e. if you get arbitrary text the best you can do if you're going in\nblind is to have various character/language frequency tables to try to\nguess at what encoding is the most plausible, but even then you might\nstill be wrong.\n\nI'd think for git-p4 (and git-svn, how do we handle this there?) a\nsensible first approximation would be to use UTF-8, and if we encounter\ndata that doesn't conform die.\n\nThen offer the user to manually configure a \"fallback\" encoding. I think\nmost real-world projects only have two of those, e.g. some old latin1\ndata before a UTF-8 migration.\n\nAnd maybe start with a \"don't even try\" mode, which AFAICT is what\nyou're describing Python 2 doing.\n\nIf we change the Python 3 code to do what the Python 2 code does now,\nwill we pull the rug from under users who have only ever used the Python\n3 code, and are relying on those semantics somehow?\n"},{"id":"443938","messageId":"YbaPy8UhzIwRuNYm@camp.crustytoothpaste.net","threadId":"57065","inReplyTo":"CABPp-BEyBLzWY2andDXZV6AgkQpnt1sp_rSThy84=qXMt2D8nA@mail.gmail.com","subject":"Re: [PATCH v2 3/3] git-p4: add \"Nvidia Corporation\" to copyright header","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-12-13T00:11:55Z","receivedAt":"2021-12-13T00:11:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-12-11 at 21:19:18, Elijah Newren wrote:\n> On Fri, Dec 10, 2021 at 12:30 PM Joel Holdsworth <jholdsworth@nvidia.com> wrote:\n> >\n> > The inclusion of the coorporate copyright is a stipulation of the\n> > company code release process.\n> > ---\n> >  git-p4.py | 1 +\n> >  1 file changed, 1 insertion(+)\n> >\n> > diff --git a/git-p4.py b/git-p4.py\n> > index 5568d44c72..17e18265dc 100755\n> > --- a/git-p4.py\n> > +++ b/git-p4.py\n> > @@ -5,6 +5,7 @@\n> >  # Author: Simon Hausmann <simon@lst.de>\n> >  # Copyright: 2007 Simon Hausmann <simon@lst.de>\n> >  #            2007 Trolltech ASA\n> > +#            2021 Nvidia Corporation\n> >  # License: MIT <http://www.opensource.org/licenses/mit-license.php>\n> >  #\n> >  # pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except\n> > --\n> > 2.33.0\n> \n> Can we just git rid of these lines entirely?\n\nIn the case of the MIT License, it is a condition of the license that\nthe copyright notices be preserved, so no, we cannot remove them.\nSpecifically, the first paragraph, which grants permissions states that\nthey are \"subject to the following conditions\", one of which is as\nfollows:\n\n  The above copyright notice and this permission notice shall be\n  included in all copies or substantial portions of the Software.\n\nThe other is the total exclusion of warranty or liability.\n\nAs for the rest of the codebase, the GPL v2 states that the exercise of\ncopying and distribution is permitted, \"provided that you conspicuously\nand appropriately publish on each copy an appropriate copyright notice\nand disclaimer of warranty.\"  I am not an attorney, but I'm pretty sure\nthat it would not be permissible to remove a copyright notice unless the\ncode to which it referred were no longer present and that would not\ncount as publishing an appropriate copyright notice.\n\nHowever, that doesn't mean we need to add to them, but I will state that\nas a contributor who primarily contributes on his own time, I don't\nthink it's unreasonable for a contributor to request that a copyright\nnotice be applied where applicable as attribution, since that's the only\ncompensation one receives for one's contributions.  Such copyright\nnotices could live in a central file for convenience, however.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"}]}