{"thread":{"id":"52399","subject":"[PATCH 00/13] git-p4: python3 compatibility","startedAt":"2019-12-07T00:33:47Z","lastAt":"2019-12-13T20:39:38Z","messageCount":33,"participants":["Yang Zhao","Denton Liu","Ben Keene","Johannes Schindelin","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"387653","messageId":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":null,"subject":"[PATCH 00/13] git-p4: python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:18Z","receivedAt":"2019-12-07T00:33:47Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"This patchset adds python3 compatibility to git-p4.\n\nWhile some clean-up refactoring would have been nice, I specifically avoided\nmaking any major changes to the internal API, aiming to have passing tests\nwith as few changes as possible.\n\nCI results can be seen from this GitHub PR: https://github.com/git/git/pull/673\n\n(As of writing, the CI pipelines are intermittently failing due to reasons\nthat appear unrelated to code. I do have python3 tests passing locally on\na Gentoo host.)\n\nYang Zhao (13):\n ci: also run linux-gcc pipeline with python-3.7 environment\n git-p4: make python-2.7 the oldest supported version\n git-p4: simplify python version detection\n git-p4: decode response from p4 to str for python3\n git-p4: properly encode/decode communication with git for python 3\n git-p4: convert path to unicode before processing them\n git-p4: open .gitp4-usercache.txt in text mode\n git-p4: use marshal format version 2 when sending to p4\n git-p4: fix freezing while waiting for fast-import progress\n git-p4: use functools.reduce instead of reduce\n git-p4: use dict.items() iteration for python3 compatibility\n git-p4: simplify regex pattern generation for parsing diff-tree\n git-p4: use python3's input() everywhere\n\n azure-pipelines.yml | 11 +++\n git-p4.py | 195 ++++++++++++++++++++++++++++----------------\n 2 files changed, 136 insertions(+), 70 deletions(-)\n\n-- \n2.21.0.windows.1\n\n"},{"id":"387654","messageId":"20191207003333.3228-2-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:19Z","receivedAt":"2019-12-07T00:33:48Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"git-p4.py includes support for python-3, but this was not previously\nvalidated in CI. Lets actually do that.\n\nThere is no tangible benefit to repeating python-3 tests for all\nenvironments, so only limit it to linux-gcc for now.\n\nSigned-off-by: Yang Zhao \n---\n\nI assert that we don't need to run python3 tests on more platforms,\nbut is this actually reasonable?\n\n azure-pipelines.yml | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex 37ed7e06c6..d5f9413248 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -331,7 +331,18 @@ jobs:\n displayName: linux-gcc\n condition: succeeded()\n pool: Hosted Ubuntu 1604\n+ strategy:\n+ matrix:\n+ python27:\n+ python.version: '2.7'\n+ python37:\n+ python.version: '3.7'\n steps:\n+ - task: UsePythonVersion@0\n+ inputs:\n+ versionSpec: '$(python.version)'\n+ - bash: |\n+ echo \"##vso[task.setvariable variable=python_path]$(which python)\"\n - bash: |\n test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n \n-- \n2.21.0.windows.1\n\n"},{"id":"387655","messageId":"20191207003333.3228-3-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 02/13] git-p4: make python-2.7 the oldest supported version","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:20Z","receivedAt":"2019-12-07T00:33:49Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Python-2.6 and earlier have been end-of-life'd for many years now, and\nwe actually already use 2.7-only features in the code. Make the version\ncheck reflect current realities.\n---\n git-p4.py | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 60c73b6a37..d8f88884db 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -8,9 +8,8 @@\n # License: MIT \n #\n import sys\n-if sys.hexversion < 0x02040000:\n- # The limiter is the subprocess module\n- sys.stderr.write(\"git-p4: requires Python 2.4 or later.\\n\")\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 sys.exit(1)\n import os\n import optparse\n-- \n2.21.0.windows.1\n\n"},{"id":"387656","messageId":"20191207003333.3228-4-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 03/13] git-p4: simplify python version detection","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:21Z","receivedAt":"2019-12-07T00:33:51Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Instead of type shenanigans, just check the version object.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 11 +----------\n 1 file changed, 1 insertion(+), 10 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex d8f88884db..ebeef35a92 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -27,18 +27,9 @@\n import errno\n \n # support basestring in python3\n-try:\n- unicode = unicode\n-except NameError:\n- # 'unicode' is undefined, must be Python 3\n- str = str\n- unicode = str\n- bytes = bytes\n+if sys.version_info.major >= 3:\n basestring = (str,bytes)\n else:\n- # 'unicode' exists, must be Python 2\n- str = str\n- unicode = unicode\n bytes = str\n basestring = basestring\n \n-- \n2.21.0.windows.1\n\n"},{"id":"387657","messageId":"20191207003333.3228-5-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 04/13] git-p4: decode response from p4 to str for python3","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:22Z","receivedAt":"2019-12-07T00:33:54Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"The marshalled dict in the response given on STDOUT by p4 uses `str` for\nkeys and string values. When run using python3, these values are\ndeserialized as `bytes`, leading to a whole host of problems as the rest\nof the code assumes `str` is used throughout.\n\nThis patch changes the deserialization behaviour such that, as much as\npossible, text output from p4 is decoded to native unicode strings.\nExceptions are made for the field `data` as it is usually arbitrary\nbinary data. `depotFile[0-9]*`, `path`, and `clientFile` are also exempt\nas they contain path information which may not be UTF-8 encoding\ncompatible, and must survive round-trip back to p4.\n\nSigned-off-by: Yang Zhao \n\nSQUASH: use unicode string internally throughout\n---\n git-p4.py | 61 ++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 45 insertions(+), 16 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex ebeef35a92..6720c7b24a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -157,6 +157,19 @@ def die(msg):\n sys.stderr.write(msg + \"\\n\")\n sys.exit(1)\n \n+# We need different encoding/decoding strategies for text data being passed\n+# around in pipes depending on python version\n+if sys.version_info.major >= 3:\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+ 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+\n def write_pipe(c, stdin):\n if verbose:\n sys.stderr.write('Writing pipe: %s\\n' % str(c))\n@@ -186,7 +199,7 @@ def read_pipe_full(c):\n expand = isinstance(c,basestring)\n p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)\n (out, err) = p.communicate()\n- return (p.returncode, out, err)\n+ return (p.returncode, out, decode_text_stream(err))\n \n def read_pipe(c, ignore_error=False):\n \"\"\" Read output from command. Returns the output text on\n@@ -209,11 +222,11 @@ def read_pipe_text(c):\n if retcode != 0:\n return None\n else:\n- return out.rstrip()\n+ return decode_text_stream(out).rstrip()\n \n-def p4_read_pipe(c, ignore_error=False):\n+def p4_read_pipe(c, ignore_error=False, raw=False):\n real_cmd = p4_build_cmd(c)\n- return read_pipe(real_cmd, ignore_error)\n+ return read_pipe(real_cmd, ignore_error, raw=raw)\n \n def read_pipe_lines(c):\n if verbose:\n@@ -222,7 +235,7 @@ def read_pipe_lines(c):\n expand = isinstance(c, basestring)\n p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)\n pipe = p.stdout\n- val = pipe.readlines()\n+ val = [decode_text_stream(line) for line in pipe.readlines()]\n if pipe.close() or p.wait():\n die('Command failed: %s' % str(c))\n \n@@ -253,6 +266,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 # return code will be 1 in either case\n if err.find(\"Invalid option\") >= 0:\n return False\n@@ -633,6 +647,20 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,\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 if skip_info:\n if 'code' in entry and entry['code'] == 'info':\n continue\n@@ -850,6 +878,7 @@ def branch_exists(branch):\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 if p.returncode:\n return False\n # expect exactly one line of output: the branch name\n@@ -1993,7 +2022,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(submitTemplate)\n+ tmpFile.write(encode_text_stream(submitTemplate))\n tmpFile.close()\n \n if self.prepare_p4_only:\n@@ -2040,11 +2069,11 @@ def applyCommit(self, id):\n if self.edit_template(fileName):\n # read the edited message and submit\n tmpFile = open(fileName, \"rb\")\n- message = tmpFile.read()\n+ message = decode_text_stream(tmpFile.read())\n tmpFile.close()\n if self.isWindows:\n message = message.replace(\"\\r\\n\", \"\\n\")\n- submitTemplate = message[:message.index(separatorLine)]\n+ submitTemplate = encode_text_stream(message[:message.index(separatorLine)])\n \n if update_shelve:\n p4_write_pipe(['shelve', '-r', '-i'], submitTemplate)\n@@ -2145,7 +2174,7 @@ def exportGitTags(self, gitTags):\n print(\"Not creating p4 label %s for tag due to option\" \\\n \" --prepare-p4-only\" % name)\n else:\n- p4_write_pipe([\"label\", \"-i\"], labelTemplate)\n+ p4_write_pipe([\"label\", \"-i\"], encode_text_stream(labelTemplate))\n \n # Use the label\n p4_system([\"tag\", \"-l\", name] +\n@@ -2469,7 +2498,7 @@ def append(self, view_line):\n \n def convert_client_path(self, clientFile):\n # chop off //client/ part to make it relative\n- if not clientFile.startswith(self.client_prefix):\n+ if not decode_path(clientFile).startswith(self.client_prefix):\n die(\"No prefix '%s' on clientFile '%s'\" %\n (self.client_prefix, clientFile))\n return clientFile[len(self.client_prefix):]\n@@ -2729,7 +2758,7 @@ def splitFilesIntoBranches(self, commit):\n return branches\n \n def writeToGitStream(self, gitMode, relPath, contents):\n- self.gitStream.write('M %s inline %s\\n' % (gitMode, relPath))\n+ self.gitStream.write(encode_text_stream(u'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@@ -2770,7 +2799,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(contents)\n+ data = ''.join(decode_text_stream(c) 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@@ -2824,7 +2853,7 @@ 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(contents)\n+ text = ''.join(decode_text_stream(c) for c in contents)\n text = regexp.sub(r'$\\1$', text)\n contents = [ text ]\n \n@@ -2839,7 +2868,7 @@ def streamOneP4Deletion(self, file):\n if verbose:\n sys.stdout.write(\"delete %s\\n\" % relPath)\n sys.stdout.flush()\n- self.gitStream.write(\"D %s\\n\" % relPath)\n+ self.gitStream.write(encode_text_stream(u'D {}\\n'.format(relPath)))\n \n if self.largeFileSystem and self.largeFileSystem.isLargeFile(relPath):\n self.largeFileSystem.removeLargeFile(relPath)\n@@ -2939,9 +2968,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 = '%s@=%d' % (f['path'], f['shelved_cl'])\n+ fileArg = f['path'] + encode_text_stream('@={}'.format(f['shelved_cl']))\n else:\n- fileArg = '%s#%s' % (f['path'], f['rev'])\n+ fileArg = f['path'] + encode_text_stream('#{}'.format(f['rev']))\n \n fileArgs.append(fileArg)\n \n-- \n2.21.0.windows.1\n\n"},{"id":"387658","messageId":"20191207003333.3228-6-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 05/13] git-p4: properly encode/decode communication with git for python 3","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:23Z","receivedAt":"2019-12-07T00:33:54Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Under python3, calls to write() on the stream to `git fast-import` must\nbe encoded. This patch wraps the IO object such that this encoding is\ndone transparently.\n\nConversely, any text data read from subprocesses must also be decoded\nbefore running through the rest of the pipeline.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 19 ++++++++++++++++---\n 1 file changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 6720c7b24a..fefa716b17 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -201,10 +201,12 @@ def read_pipe_full(c):\n (out, err) = p.communicate()\n return (p.returncode, out, decode_text_stream(err))\n \n-def read_pipe(c, ignore_error=False):\n+def read_pipe(c, ignore_error=False, raw=False):\n \"\"\" Read output from command. Returns the output text on\n success. On failure, terminates execution, unless\n ignore_error is True, when it returns an empty string.\n+\n+ If raw is True, do not attempt to decode output text.\n \"\"\"\n (retcode, out, err) = read_pipe_full(c)\n if retcode != 0:\n@@ -212,6 +214,8 @@ def read_pipe(c, ignore_error=False):\n out = \"\"\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 \n def read_pipe_text(c):\n@@ -238,7 +242,6 @@ def read_pipe_lines(c):\n val = [decode_text_stream(line) for line in pipe.readlines()]\n if pipe.close() or p.wait():\n die('Command failed: %s' % str(c))\n-\n return val\n \n def p4_read_pipe_lines(c):\n@@ -634,7 +637,8 @@ 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(i + '\\n')\n+ stdin_file.write(encode_text_stream(i))\n+ stdin_file.write(b'\\n')\n stdin_file.flush()\n stdin_file.seek(0)\n \n@@ -3556,6 +3560,15 @@ 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+\n+ self.gitStream.write = make_encoded_write(self.gitStream.write)\n+\n def closeStreams(self):\n self.gitStream.close()\n if self.importProcess.wait() != 0:\n-- \n2.21.0.windows.1\n\n"},{"id":"387659","messageId":"20191207003333.3228-8-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 06/13] git-p4: open .gitp4-usercache.txt in text mode","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:25Z","receivedAt":"2019-12-07T00:33:56Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Opening .gitp4-usercache.txt in text mode makes python 3 happy without\nexplicitly adding encoding and decoding.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c7d543b18e..bd3118e0e8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1402,14 +1402,14 @@ def getUserMapFromPerforceServer(self):\n for (key, val) in self.users.items():\n s += \"%s\\t%s\\n\" % (key.expandtabs(1), val.expandtabs(1))\n \n- open(self.getUserCacheFilename(), \"wb\").write(s)\n+ open(self.getUserCacheFilename(), 'w').write(s)\n self.userMapFromPerforceServer = True\n \n def loadUserMapFromCache(self):\n self.users = {}\n self.userMapFromPerforceServer = False\n try:\n- cache = open(self.getUserCacheFilename(), \"rb\")\n+ cache = open(self.getUserCacheFilename(), 'r')\n lines = cache.readlines()\n cache.close()\n for line in lines:\n-- \n2.21.0.windows.1\n\n"},{"id":"387660","messageId":"20191207003333.3228-7-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 06/13] git-p4: convert path to unicode before processing them","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:24Z","receivedAt":"2019-12-07T00:33:57Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"P4 allows essentially arbitrary encoding for path data while we would\nperfer to be dealing only with unicode strings. Since path data need to\nsurvive round-trip back to p4, this patch implements the general policy\nthat we store path data as-is, but decode them to unicode before doing\nany non-trivial processing.\n\nA new `decode_path()` method is provided that generally does the correct\nconversion, taking into account `git-p4.pathEncoding` configuration.\n\nFor python2.7, path strings will be left as-is if it only contains ASCII\ncharacters.\n\nFor python3, decoding is always done so that we have str objects.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 67 +++++++++++++++++++++++++++++++++++--------------------\n 1 file changed, 43 insertions(+), 24 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex fefa716b17..088924fbe1 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -170,6 +170,21 @@ def decode_text_stream(s):\n def encode_text_stream(s):\n return s.encode('utf_8') if isinstance(s, unicode) 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+\n def write_pipe(c, stdin):\n if verbose:\n sys.stderr.write('Writing pipe: %s\\n' % str(c))\n@@ -715,7 +730,8 @@ def p4Where(depotPath):\n if \"depotFile\" in entry:\n # Search for the base client side depot path, as long as it starts with the branch's P4 path.\n # The base path always ends with \"/...\".\n- if entry[\"depotFile\"].find(depotPath) == 0 and entry[\"depotFile\"][-4:] == \"/...\":\n+ entry_path = decode_path(entry['depotFile'])\n+ if entry_path.find(depotPath) == 0 and entry_path[-4:] == \"/...\":\n output = entry\n break\n elif \"data\" in entry:\n@@ -730,11 +746,11 @@ def p4Where(depotPath):\n return \"\"\n clientPath = \"\"\n if \"path\" in output:\n- clientPath = output.get(\"path\")\n+ clientPath = decode_path(output['path'])\n elif \"data\" in output:\n data = output.get(\"data\")\n- lastSpace = data.rfind(\" \")\n- clientPath = data[lastSpace + 1:]\n+ lastSpace = data.rfind(b\" \")\n+ clientPath = decode_path(data[lastSpace + 1:])\n \n if clientPath.endswith(\"...\"):\n clientPath = clientPath[:-3]\n@@ -2511,7 +2527,7 @@ def update_client_spec_path_cache(self, files):\n \"\"\" Caching file paths by \"p4 where\" batch query \"\"\"\n \n # List depot file paths exclude that already cached\n- fileArgs = [f['path'] for f in files if f['path'] not in self.client_spec_path_cache]\n+ fileArgs = [f['path'] for f in files if decode_path(f['path']) not in self.client_spec_path_cache]\n \n if len(fileArgs) == 0:\n return # All files in cache\n@@ -2526,16 +2542,18 @@ def update_client_spec_path_cache(self, files):\n if \"unmap\" in res:\n # it will list all of them, but only one not unmap-ped\n continue\n+ depot_path = decode_path(res['depotFile'])\n if gitConfigBool(\"core.ignorecase\"):\n- res['depotFile'] = res['depotFile'].lower()\n- self.client_spec_path_cache[res['depotFile']] = self.convert_client_path(res[\"clientFile\"])\n+ depot_path = depot_path.lower()\n+ self.client_spec_path_cache[depot_path] = self.convert_client_path(res[\"clientFile\"])\n \n # not found files or unmap files set to \"\"\n for depotFile in fileArgs:\n+ depotFile = decode_path(depotFile)\n if gitConfigBool(\"core.ignorecase\"):\n depotFile = depotFile.lower()\n if depotFile not in self.client_spec_path_cache:\n- self.client_spec_path_cache[depotFile] = \"\"\n+ self.client_spec_path_cache[depotFile] = b''\n \n def map_in_client(self, depot_path):\n \"\"\"Return the relative location in the client where this\n@@ -2653,7 +2671,7 @@ def isPathWanted(self, path):\n elif path.lower() == p.lower():\n return False\n for p in self.depotPaths:\n- if p4PathStartsWith(path, p):\n+ if p4PathStartsWith(path, decode_path(p)):\n return True\n return False\n \n@@ -2662,7 +2680,7 @@ def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n- found = self.isPathWanted(path)\n+ found = self.isPathWanted(decode_path(path))\n if not found:\n fnum = fnum + 1\n continue\n@@ -2696,7 +2714,7 @@ def stripRepoPath(self, path, prefixes):\n if self.useClientSpec:\n # branch detection moves files up a level (the branch name)\n # from what client spec interpretation gives\n- path = self.clientSpecDirs.map_in_client(path)\n+ path = decode_path(self.clientSpecDirs.map_in_client(path))\n if self.detectBranches:\n for b in self.knownBranches:\n if p4PathStartsWith(path, b + \"/\"):\n@@ -2730,14 +2748,15 @@ def splitFilesIntoBranches(self, commit):\n branches = {}\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n- path = commit[\"depotFile%s\" % fnum]\n+ raw_path = commit[\"depotFile%s\" % fnum]\n+ path = decode_path(raw_path)\n found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\n- file[\"path\"] = path\n+ file[\"path\"] = raw_path\n file[\"rev\"] = commit[\"rev%s\" % fnum]\n file[\"action\"] = commit[\"action%s\" % fnum]\n file[\"type\"] = commit[\"type%s\" % fnum]\n@@ -2746,7 +2765,7 @@ def splitFilesIntoBranches(self, commit):\n # start with the full relative path where this file would\n # go in a p4 client\n if self.useClientSpec:\n- relPath = self.clientSpecDirs.map_in_client(path)\n+ relPath = decode_path(self.clientSpecDirs.map_in_client(path))\n else:\n relPath = self.stripRepoPath(path, self.depotPaths)\n \n@@ -2784,14 +2803,15 @@ def encodeWithUTF8(self, path):\n # - helper for streamP4Files\n \n def streamOneP4File(self, file, contents):\n- relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n- relPath = self.encodeWithUTF8(relPath)\n+ file_path = file['depotFile']\n+ relPath = self.stripRepoPath(decode_path(file_path), self.branchPrefixes)\n+\n if verbose:\n if 'fileSize' in self.stream_file:\n size = int(self.stream_file['fileSize'])\n else:\n size = 0 # deleted files don't get a fileSize apparently\n- sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n+ sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file_path, relPath, size/1024/1024))\n sys.stdout.flush()\n \n (type_base, type_mods) = split_p4_type(file[\"type\"])\n@@ -2809,7 +2829,7 @@ def streamOneP4File(self, file, contents):\n # to nothing. This causes p4 errors when checking out such\n # a change, and errors here too. Work around it by ignoring\n # the bad symlink; hopefully a future change fixes it.\n- print(\"\\nIgnoring empty symlink in %s\" % file['depotFile'])\n+ print(\"\\nIgnoring empty symlink in %s\" % file_path)\n return\n elif data[-1] == '\\n':\n contents = [data[:-1]]\n@@ -2828,7 +2848,7 @@ def streamOneP4File(self, file, contents):\n # just the native \"NT\" type.\n #\n try:\n- text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])\n+ text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (decode_path(file['depotFile']), file['change'])], raw=True)\n except Exception as e:\n if 'Translation of file content failed' in str(e):\n type_base = 'binary'\n@@ -2836,7 +2856,7 @@ def streamOneP4File(self, file, contents):\n raise e\n else:\n if p4_version_string().find('/NT') >= 0:\n- text = text.replace('\\r\\n', '\\n')\n+ text = text.replace(b'\\r\\n', b'\\n')\n contents = [ text ]\n \n if type_base == \"apple\":\n@@ -2867,8 +2887,7 @@ def streamOneP4File(self, file, contents):\n self.writeToGitStream(git_mode, relPath, contents)\n \n def streamOneP4Deletion(self, file):\n- relPath = self.stripRepoPath(file['path'], self.branchPrefixes)\n- relPath = self.encodeWithUTF8(relPath)\n+ relPath = self.stripRepoPath(decode_path(file['path']), self.branchPrefixes)\n if verbose:\n sys.stdout.write(\"delete %s\\n\" % relPath)\n sys.stdout.flush()\n@@ -3055,8 +3074,8 @@ def commit(self, details, files, branch, parent = \"\", allow_empty=False):\n if self.clientSpecDirs:\n self.clientSpecDirs.update_client_spec_path_cache(files)\n \n- files = [f for f in files\n- if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]\n+ files = [f for (f, path) in ((f, decode_path(f['path'])) for f in files)\n+ if self.inClientSpec(path) and self.hasBranchPrefix(path)]\n \n if gitConfigBool('git-p4.keepEmptyCommits'):\n allow_empty = True\n-- \n2.21.0.windows.1\n\n"},{"id":"387662","messageId":"20191207003333.3228-9-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 07/13] git-p4: convert path to unicode before processing them","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:26Z","receivedAt":"2019-12-07T00:33:58Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"P4 allows essentially arbitrary encoding for path data while we would\nperfer to be dealing only with unicode strings. Since path data need to\nsurvive round-trip back to p4, this patch implements the general policy\nthat we store path data as-is, but decode them to unicode before doing\nany non-trivial processing.\n\nA new `decode_path()` method is provided that generally does the correct\nconversion, taking into account `git-p4.pathEncoding` configuration.\n\nFor python2.7, path strings will be left as-is if it only contains ASCII\ncharacters.\n\nFor python3, decoding is always done so that we have str objects.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 67 +++++++++++++++++++++++++++++++++++--------------------\n 1 file changed, 43 insertions(+), 24 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex bd3118e0e8..650c11eb62 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -170,6 +170,21 @@ def decode_text_stream(s):\n def encode_text_stream(s):\n return s.encode('utf_8') if isinstance(s, unicode) 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+\n def write_pipe(c, stdin):\n if verbose:\n sys.stderr.write('Writing pipe: %s\\n' % str(c))\n@@ -720,7 +735,8 @@ def p4Where(depotPath):\n if \"depotFile\" in entry:\n # Search for the base client side depot path, as long as it starts with the branch's P4 path.\n # The base path always ends with \"/...\".\n- if entry[\"depotFile\"].find(depotPath) == 0 and entry[\"depotFile\"][-4:] == \"/...\":\n+ entry_path = decode_path(entry['depotFile'])\n+ if entry_path.find(depotPath) == 0 and entry_path[-4:] == \"/...\":\n output = entry\n break\n elif \"data\" in entry:\n@@ -735,11 +751,11 @@ def p4Where(depotPath):\n return \"\"\n clientPath = \"\"\n if \"path\" in output:\n- clientPath = output.get(\"path\")\n+ clientPath = decode_path(output['path'])\n elif \"data\" in output:\n data = output.get(\"data\")\n- lastSpace = data.rfind(\" \")\n- clientPath = data[lastSpace + 1:]\n+ lastSpace = data.rfind(b\" \")\n+ clientPath = decode_path(data[lastSpace + 1:])\n \n if clientPath.endswith(\"...\"):\n clientPath = clientPath[:-3]\n@@ -2516,7 +2532,7 @@ def update_client_spec_path_cache(self, files):\n \"\"\" Caching file paths by \"p4 where\" batch query \"\"\"\n \n # List depot file paths exclude that already cached\n- fileArgs = [f['path'] for f in files if f['path'] not in self.client_spec_path_cache]\n+ fileArgs = [f['path'] for f in files if decode_path(f['path']) not in self.client_spec_path_cache]\n \n if len(fileArgs) == 0:\n return # All files in cache\n@@ -2531,16 +2547,18 @@ def update_client_spec_path_cache(self, files):\n if \"unmap\" in res:\n # it will list all of them, but only one not unmap-ped\n continue\n+ depot_path = decode_path(res['depotFile'])\n if gitConfigBool(\"core.ignorecase\"):\n- res['depotFile'] = res['depotFile'].lower()\n- self.client_spec_path_cache[res['depotFile']] = self.convert_client_path(res[\"clientFile\"])\n+ depot_path = depot_path.lower()\n+ self.client_spec_path_cache[depot_path] = self.convert_client_path(res[\"clientFile\"])\n \n # not found files or unmap files set to \"\"\n for depotFile in fileArgs:\n+ depotFile = decode_path(depotFile)\n if gitConfigBool(\"core.ignorecase\"):\n depotFile = depotFile.lower()\n if depotFile not in self.client_spec_path_cache:\n- self.client_spec_path_cache[depotFile] = \"\"\n+ self.client_spec_path_cache[depotFile] = b''\n \n def map_in_client(self, depot_path):\n \"\"\"Return the relative location in the client where this\n@@ -2658,7 +2676,7 @@ def isPathWanted(self, path):\n elif path.lower() == p.lower():\n return False\n for p in self.depotPaths:\n- if p4PathStartsWith(path, p):\n+ if p4PathStartsWith(path, decode_path(p)):\n return True\n return False\n \n@@ -2667,7 +2685,7 @@ def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n- found = self.isPathWanted(path)\n+ found = self.isPathWanted(decode_path(path))\n if not found:\n fnum = fnum + 1\n continue\n@@ -2701,7 +2719,7 @@ def stripRepoPath(self, path, prefixes):\n if self.useClientSpec:\n # branch detection moves files up a level (the branch name)\n # from what client spec interpretation gives\n- path = self.clientSpecDirs.map_in_client(path)\n+ path = decode_path(self.clientSpecDirs.map_in_client(path))\n if self.detectBranches:\n for b in self.knownBranches:\n if p4PathStartsWith(path, b + \"/\"):\n@@ -2735,14 +2753,15 @@ def splitFilesIntoBranches(self, commit):\n branches = {}\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n- path = commit[\"depotFile%s\" % fnum]\n+ raw_path = commit[\"depotFile%s\" % fnum]\n+ path = decode_path(raw_path)\n found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\n- file[\"path\"] = path\n+ file[\"path\"] = raw_path\n file[\"rev\"] = commit[\"rev%s\" % fnum]\n file[\"action\"] = commit[\"action%s\" % fnum]\n file[\"type\"] = commit[\"type%s\" % fnum]\n@@ -2751,7 +2770,7 @@ def splitFilesIntoBranches(self, commit):\n # start with the full relative path where this file would\n # go in a p4 client\n if self.useClientSpec:\n- relPath = self.clientSpecDirs.map_in_client(path)\n+ relPath = decode_path(self.clientSpecDirs.map_in_client(path))\n else:\n relPath = self.stripRepoPath(path, self.depotPaths)\n \n@@ -2789,14 +2808,15 @@ def encodeWithUTF8(self, path):\n # - helper for streamP4Files\n \n def streamOneP4File(self, file, contents):\n- relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n- relPath = self.encodeWithUTF8(relPath)\n+ file_path = file['depotFile']\n+ relPath = self.stripRepoPath(decode_path(file_path), self.branchPrefixes)\n+\n if verbose:\n if 'fileSize' in self.stream_file:\n size = int(self.stream_file['fileSize'])\n else:\n size = 0 # deleted files don't get a fileSize apparently\n- sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n+ sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file_path, relPath, size/1024/1024))\n sys.stdout.flush()\n \n (type_base, type_mods) = split_p4_type(file[\"type\"])\n@@ -2814,7 +2834,7 @@ def streamOneP4File(self, file, contents):\n # to nothing. This causes p4 errors when checking out such\n # a change, and errors here too. Work around it by ignoring\n # the bad symlink; hopefully a future change fixes it.\n- print(\"\\nIgnoring empty symlink in %s\" % file['depotFile'])\n+ print(\"\\nIgnoring empty symlink in %s\" % file_path)\n return\n elif data[-1] == '\\n':\n contents = [data[:-1]]\n@@ -2833,7 +2853,7 @@ def streamOneP4File(self, file, contents):\n # just the native \"NT\" type.\n #\n try:\n- text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])\n+ text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (decode_path(file['depotFile']), file['change'])], raw=True)\n except Exception as e:\n if 'Translation of file content failed' in str(e):\n type_base = 'binary'\n@@ -2841,7 +2861,7 @@ def streamOneP4File(self, file, contents):\n raise e\n else:\n if p4_version_string().find('/NT') >= 0:\n- text = text.replace('\\r\\n', '\\n')\n+ text = text.replace(b'\\r\\n', b'\\n')\n contents = [ text ]\n \n if type_base == \"apple\":\n@@ -2872,8 +2892,7 @@ def streamOneP4File(self, file, contents):\n self.writeToGitStream(git_mode, relPath, contents)\n \n def streamOneP4Deletion(self, file):\n- relPath = self.stripRepoPath(file['path'], self.branchPrefixes)\n- relPath = self.encodeWithUTF8(relPath)\n+ relPath = self.stripRepoPath(decode_path(file['path']), self.branchPrefixes)\n if verbose:\n sys.stdout.write(\"delete %s\\n\" % relPath)\n sys.stdout.flush()\n@@ -3060,8 +3079,8 @@ def commit(self, details, files, branch, parent = \"\", allow_empty=False):\n if self.clientSpecDirs:\n self.clientSpecDirs.update_client_spec_path_cache(files)\n \n- files = [f for f in files\n- if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]\n+ files = [f for (f, path) in ((f, decode_path(f['path'])) for f in files)\n+ if self.inClientSpec(path) and self.hasBranchPrefix(path)]\n \n if gitConfigBool('git-p4.keepEmptyCommits'):\n allow_empty = True\n-- \n2.21.0.windows.1\n\n"},{"id":"387661","messageId":"20191207003333.3228-10-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 07/13] git-p4: open .gitp4-usercache.txt in text mode","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:27Z","receivedAt":"2019-12-07T00:33:59Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Opening .gitp4-usercache.txt in text mode makes python 3 happy without\nexplicitly adding encoding and decoding.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 088924fbe1..af563cf23d 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1413,14 +1413,14 @@ def getUserMapFromPerforceServer(self):\n for (key, val) in self.users.items():\n s += \"%s\\t%s\\n\" % (key.expandtabs(1), val.expandtabs(1))\n \n- open(self.getUserCacheFilename(), \"wb\").write(s)\n+ open(self.getUserCacheFilename(), 'w').write(s)\n self.userMapFromPerforceServer = True\n \n def loadUserMapFromCache(self):\n self.users = {}\n self.userMapFromPerforceServer = False\n try:\n- cache = open(self.getUserCacheFilename(), \"rb\")\n+ cache = open(self.getUserCacheFilename(), 'r')\n lines = cache.readlines()\n cache.close()\n for line in lines:\n-- \n2.21.0.windows.1\n\n"},{"id":"387663","messageId":"20191207003333.3228-12-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 09/13] git-p4: fix freezing while waiting for fast-import progress","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:29Z","receivedAt":"2019-12-07T00:34:00Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"As part of its importing process, git-p4 sends a `checkpoint` followed\nimmediately by `progress` to fast-import in to force synchronization.\nDue to buffering, it is possible for the `progress` command to not be\nflushed before git-p4 proceeds to wait for the corresponding response.\nThis causes the script to freeze completely, and is consistently\nobservable at least on python-3.6.9.\n\nMake sure this command sequence is completely flushed before waiting.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c2a3de59e7..1007b936c8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2659,6 +2659,7 @@ def __init__(self):\n def checkpoint(self):\n self.gitStream.write(\"checkpoint\\n\\n\")\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n+ self.gitStream.flush()\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n-- \n2.21.0.windows.1\n\n"},{"id":"387664","messageId":"20191207003333.3228-11-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 08/13] git-p4: use marshal format version 2 when sending to p4","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:28Z","receivedAt":"2019-12-07T00:34:00Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"p4 does not appear to understand marshal format version 3 and above.\nVersion 2 was the latest supported by python-2.7.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex af563cf23d..c2a3de59e7 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1697,7 +1697,8 @@ def modifyChangelistUser(self, changelist, newUser):\n c = changes[0]\n if c['User'] == newUser: return # nothing to do\n c['User'] = newUser\n- input = marshal.dumps(c)\n+ # p4 does not understand format version 3 and above\n+ input = marshal.dumps(c, 2)\n \n result = p4CmdList(\"change -f -i\", stdin=input)\n for r in result:\n-- \n2.21.0.windows.1\n\n"},{"id":"387666","messageId":"20191207003333.3228-13-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 10/13] git-p4: use functools.reduce instead of reduce","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:30Z","receivedAt":"2019-12-07T00:34:01Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"For python3, reduce() has been moved to functools.reduce(). This is\nalso available in python2.7.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 1007b936c8..c888e4825a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -13,6 +13,7 @@\n sys.exit(1)\n import os\n import optparse\n+import functools\n import marshal\n import subprocess\n import tempfile\n@@ -1176,7 +1177,7 @@ def pushFile(self, localLargeFile):\n assert False, \"Method 'pushFile' required in \" + self.__class__.__name__\n \n def hasLargeFileExtension(self, relPath):\n- return reduce(\n+ return functools.reduce(\n lambda a, b: a or b,\n [relPath.endswith('.' + e) for e in gitConfigList('git-p4.largeFileExtensions')],\n False\n-- \n2.21.0.windows.1\n\n"},{"id":"387665","messageId":"20191207003333.3228-14-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 11/13] git-p4: use dict.items() iteration for python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:31Z","receivedAt":"2019-12-07T00:34:02Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Python3 uses dict.items() instead of .iteritems() to provide\niteratoration over dict. Although items() is technically less efficient\nfor python2.7 (allocates a new list instead of simply iterating), the\namount of data involved is very small and the penalty negligible.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c888e4825a..867a8d42ef 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1763,7 +1763,7 @@ def prepareSubmitTemplate(self, changelist=None):\n break\n if not change_entry:\n die('Failed to decode output of p4 change -o')\n- for key, value in change_entry.iteritems():\n+ for key, value in change_entry.items():\n if key.startswith('File'):\n if 'depot-paths' in settings:\n if not [p for p in settings['depot-paths']\n-- \n2.21.0.windows.1\n\n"},{"id":"387667","messageId":"20191207003333.3228-16-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 13/13] git-p4: use python3's input() everywhere","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:33Z","receivedAt":"2019-12-07T00:34:05Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"Python3 deprecates raw_input() from 2.7 and replaced it with input().\nSince we do not need 2.7's input() semantics, `raw_input()` is aliased\nto `input()` for easy forward compatability.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex f975f197a5..97a9def657 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -33,6 +33,8 @@\n else:\n bytes = str\n basestring = basestring\n+ # We want python3's input() semantics\n+ input = raw_input\n \n try:\n from subprocess import CalledProcessError\n@@ -1819,7 +1821,7 @@ def edit_template(self, template_file):\n return True\n \n while True:\n- response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n+ response = input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n if response == 'y':\n return True\n if response == 'n':\n@@ -2390,7 +2392,7 @@ def run(self, args):\n # prompt for what to do, or use the option/variable\n if self.conflict_behavior == \"ask\":\n print(\"What do you want to do?\")\n- response = raw_input(\"[s]kip this commit but apply\"\n+ response = input(\"[s]kip this commit but apply\"\n \" the rest, or [q]uit? \")\n if not response:\n continue\n-- \n2.21.0.windows.1\n\n"},{"id":"387668","messageId":"20191207003333.3228-15-yang.zhao@skyboxlabs.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"[PATCH 12/13] git-p4: simplify regex pattern generation for parsing diff-tree","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T00:33:32Z","receivedAt":"2019-12-07T00:34:06Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"It is not clear why a generator was used to create the regex used to\nparse git-diff-tree output; I assume an early implementation required\nit, but is not part of the mainline change.\n\nSimply use a lazily initialized global instead.\n\nSigned-off-by: Yang Zhao \n---\n git-p4.py | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 867a8d42ef..f975f197a5 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -562,12 +562,7 @@ def getGitTags():\n gitTags.add(tag)\n return gitTags\n \n-def diffTreePattern():\n- # This is a simple generator for the diff tree regex pattern. This could be\n- # a class variable if this and parseDiffTreeEntry were a part of a class.\n- pattern = re.compile(':(\\d+) (\\d+) (\\w+) (\\w+) ([A-Z])(\\d+)?\\t(.*?)((\\t(.*))|$)')\n- while True:\n- yield pattern\n+_diff_tree_pattern = None\n \n def parseDiffTreeEntry(entry):\n \"\"\"Parses a single diff tree entry into its component elements.\n@@ -588,7 +583,11 @@ def parseDiffTreeEntry(entry):\n \n If the pattern is not matched, None is returned.\"\"\"\n \n- match = diffTreePattern().next().match(entry)\n+ global _diff_tree_pattern\n+ if not _diff_tree_pattern:\n+ _diff_tree_pattern = re.compile(':(\\d+) (\\d+) (\\w+) (\\w+) ([A-Z])(\\d+)?\\t(.*?)((\\t(.*))|$)')\n+\n+ match = _diff_tree_pattern.match(entry)\n if match:\n return {\n 'src_mode': match.group(1),\n-- \n2.21.0.windows.1\n\n"},{"id":"387669","messageId":"20191207010938.GA75094@generichostname","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-12-07T01:09:38Z","receivedAt":"2019-12-07T01:09:18Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Yang,\n\nOn Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:\n> This patchset adds python3 compatibility to git-p4.\n> \n> While some clean-up refactoring would have been nice, I specifically avoided\n> making any major changes to the internal API, aiming to have passing tests\n> with as few changes as possible.\n> \n> CI results can be seen from this GitHub PR: https://github.com/git/git/pull/673\n> \n> (As of writing, the CI pipelines are intermittently failing due to reasons\n> that appear unrelated to code. I do have python3 tests passing locally on\n> a Gentoo host.)\n\nCurrently, there's a competing effort to do the same thing[1] by Ben\nKeene (CC'd). Like the last time[2] two competing topics arose at the\nsame time, I'm going to make the same suggestion.\n\nWould it be possible for both of you to join forces?\n\nThanks,\n\nDenton\n\n[1]: https://lore.kernel.org/git/pull.463.v4.git.1575498577.gitgitgadget@gmail.com/\n[2]: https://lore.kernel.org/git/xmqq5zs7oexn.fsf@gitster-ct.c.googlers.com/\n"},{"id":"387670","messageId":"CABvFv3+viMXJO0z5HAQbCya7MU9tWd7P_LxUhu66T74XGN99yA@mail.gmail.com","threadId":"52399","inReplyTo":"20191207010938.GA75094@generichostname","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T07:29:20Z","receivedAt":"2019-12-07T07:29:38Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Fri, Dec 6, 2019 at 5:09 PM Denton Liu wrote:\n> On Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:\n> > This patchset adds python3 compatibility to git-p4.\n> > ...\n>\n> Currently, there's a competing effort to do the same thing[1] by Ben\n> Keene (CC'd). Like the last time[2] two competing topics arose at the\n> same time, I'm going to make the same suggestion.\n>\n> Would it be possible for both of you to join forces?\n\nYes, I do believe we are aware of each other's efforts. I had submitted\nan RFC patch set around the time Ben was preparing his own patchset.\n\nI have not reviewed Ben's first patchset as I did not feel that I understood\nthe systems well enough at the time. I've briefly skimmed through Ben's latest\niteration and it would appear the general approach is very similar, but there's\nmore added abstractions and just general code change in his version.\n\nRegardless, I'm open to working together.\n\nIdeally, I would prefer we land something minimal and working in mainline soon,\nthen further collaborate on changes that clean up code and enable more features.\n\nMy end-game is to have P4 Streams working in git-p4, and maybe LFS-like support\nthat uses p4 as the backend. It would be great to not be the only one\nspending effort\nin that direction.\n\nYang\n"},{"id":"387671","messageId":"CABvFv3+06yTTaF6VQ=DpRV5U5wDBBmrQc2ZNXDEmvoGw6R_o0Q@mail.gmail.com","threadId":"52399","inReplyTo":"20191207003333.3228-1-yang.zhao@skyboxlabs.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T07:34:51Z","receivedAt":"2019-12-07T07:35:10Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Fri, Dec 6, 2019 at 4:33 PM Yang Zhao wrote:\n> This patchset adds python3 compatibility to git-p4.\n>\n> While some clean-up refactoring would have been nice, I specifically avoided\n> making any major changes to the internal API, aiming to have passing tests\n> with as few changes as possible.\n>\n> CI results can be seen from this GitHub PR: https://github.com/git/git/pull/673\n\nLooks like p4 LFS tests are failing for python3. Looks like it's just\nmore bytes vs str.\n\nWill have to enable LFS tests in my own environment.\n\n-- \nYang Zhao\n"},{"id":"387676","messageId":"b21d153a-02f9-b9a1-7388-59b5a882d4f2@gmail.com","threadId":"52399","inReplyTo":"CABvFv3+viMXJO0z5HAQbCya7MU9tWd7P_LxUhu66T74XGN99yA@mail.gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Ben Keene","fromEmail":"seraphire@gmail.com","sentAt":"2019-12-07T16:21:01Z","receivedAt":"2019-12-07T16:21:06Z","isPatch":true,"sender":{"key":"seraphire@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22523774?v=4"},"body":"\nOn 12/7/2019 2:29 AM, Yang Zhao wrote:\n> On Fri, Dec 6, 2019 at 5:09 PM Denton Liu wrote:\n>> On Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:\n>>> This patchset adds python3 compatibility to git-p4.\n>>> ...\n>> Currently, there's a competing effort to do the same thing[1] by Ben\n>> Keene (CC'd). Like the last time[2] two competing topics arose at the\n>> same time, I'm going to make the same suggestion.\n>>\n>> Would it be possible for both of you to join forces?\n> Yes, I do believe we are aware of each other's efforts. I had submitted\n> an RFC patch set around the time Ben was preparing his own patchset.\n> I have not reviewed Ben's first patchset as I did not feel that I understood\n> the systems well enough at the time. I've briefly skimmed through Ben's latest\n> iteration and it would appear the general approach is very similar, but there's\n> more added abstractions and just general code change in his version.\n>\n> Regardless, I'm open to working together.\n\nI am also open to working together, and could really use the help, as I'm\nnot a python developer.\n\nI have taken all the suggestions from my first patch set and have reworked\nmy code and commits and will submit them now for review.  With the smaller\npatches and cleaner commit messages I hope that it will make it easier\nto see what I've done so far and what is still open work.\n\n> Ideally, I would prefer we land something minimal and working in mainline soon,\n> then further collaborate on changes that clean up code and enable more features.\n>\n> My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support\n> that uses p4 as the backend. It would be great to not be the only one\n> spending effort\n> in that direction.\n>\n> Yang\n\n\nI have similar goals.  I would love to get the smallest set of non-breaking\nchanges in that allows the program to basically work with Python 3.5+.\n\nMy rush has been because I need to use git-p4 for work and have been \nworking\non the project at the office.  Once I reach a point where I am able to\ngenerally work (when t9800 is complete) I'll really not be free to spend \ntoo\nmuch work time on the project, but I am eager to see this through!\n\nAs far as status, the last time I ran tests, python 2.7 passed all the tests\nand Python 3.5 passed some of the tests.  I know it is not passing t9801\nat this time and I'm trying to find out why.\n\nSo, Yang, I am very interested in working together.\n\n\nKindest regards,\n\nBen Keene\n\n\n"},{"id":"387698","messageId":"CABvFv3Jf9i06OmBqOC2zfS+7Sm88PRYa19_rB8rELtMoN2E8CQ@mail.gmail.com","threadId":"52399","inReplyTo":"b21d153a-02f9-b9a1-7388-59b5a882d4f2@gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-07T19:59:47Z","receivedAt":"2019-12-07T20:00:04Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Sat, Dec 7, 2019 at 8:21 AM Ben Keene wrote:\n> On 12/7/2019 2:29 AM, Yang Zhao wrote:\n> > Ideally, I would prefer we land something minimal and working in mainline soon,\n> > then further collaborate on changes that clean up code and enable more features.\n> >\n> > My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support\n> > that uses p4 as the backend. It would be great to not be the only one\n> > spending effort\n> > in that direction.\n>\n> I have similar goals. I would love to get the smallest set of non-breaking\n> changes in that allows the program to basically work with Python 3.5+.\n>\n> My rush has been because I need to use git-p4 for work and have been\n> working\n> on the project at the office. Once I reach a point where I am able to\n> generally work (when t9800 is complete) I'll really not be free to spend\n> too\n> much work time on the project, but I am eager to see this through!\n\nI'm in a similar situation, but we use p4 Streams and so I actually need further\ndevelopment before being able to make a full switch. I am given more liberty\nin terms of how much work time I can dedicate to this, though.\n\nGiven the situation, can you give my patch set a try in your work environment?\nIt is currently passing everything except t9824-git-p4-git-lfs.\n\nIf you're OK with it, I would prefer that we work from my version as a base and\nadd some of your quality-of-life enhancements on top. I can do the merges myself\nif you are pressed for time.\n\nThanks,\nYang\n"},{"id":"387783","messageId":"afa761cf-9c0e-cdcc-9c32-be88c5507042@gmail.com","threadId":"52399","inReplyTo":"CABvFv3Jf9i06OmBqOC2zfS+7Sm88PRYa19_rB8rELtMoN2E8CQ@mail.gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Ben Keene","fromEmail":"seraphire@gmail.com","sentAt":"2019-12-09T15:03:14Z","receivedAt":"2019-12-09T15:03:18Z","isPatch":true,"sender":{"key":"seraphire@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22523774?v=4"},"body":"\nOn 12/7/2019 2:59 PM, Yang Zhao wrote:\n> On Sat, Dec 7, 2019 at 8:21 AM Ben Keene wrote:\n>> On 12/7/2019 2:29 AM, Yang Zhao wrote:\n>>> Ideally, I would prefer we land something minimal and working in mainline soon,\n>>> then further collaborate on changes that clean up code and enable more features.\n>>>\n>>> My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support\n>>> that uses p4 as the backend. It would be great to not be the only one\n>>> spending effort\n>>> in that direction.\n>> I have similar goals. I would love to get the smallest set of non-breaking\n>> changes in that allows the program to basically work with Python 3.5+.\n>>\n>> My rush has been because I need to use git-p4 for work and have been\n>> working\n>> on the project at the office. Once I reach a point where I am able to\n>> generally work (when t9800 is complete) I'll really not be free to spend\n>> too\n>> much work time on the project, but I am eager to see this through!\n> I'm in a similar situation, but we use p4 Streams and so I actually need further\n> development before being able to make a full switch. I am given more liberty\n> in terms of how much work time I can dedicate to this, though.\n>\n> Given the situation, can you give my patch set a try in your work environment?\n> It is currently passing everything except t9824-git-p4-git-lfs.\n\nI downloaded your code and it looks like it works for Python 2.7.  I'm \nseeing errors with the following tests:\n\n* 9816.5\n\n     Traceback (most recent call last):\n     File \"/home/bkeene/git/git-p4\", line 4227, in \n         main()\n     File \"/home/bkeene/git/git-p4\", line 4221, in main\n         if not cmd.run(args):\n     File \"/home/bkeene/git/git-p4\", line 2381, in run\n         ok = self.applyCommit(commit)\n     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n         p4_write_pipe(['submit', '-i'], submitTemplate)\n     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n         return write_pipe(real_cmd, stdin)\n     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n         die('Command failed: %s' % str(c))\n     File \"/home/bkeene/git/git-p4\", line 158, in die\n         raise Exception(msg)\n     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n\n* 9816.6\n\n     Traceback (most recent call last):\n     File \"/home/bkeene/git/git-p4\", line 4227, in \n         main()\n     File \"/home/bkeene/git/git-p4\", line 4221, in main\n         if not cmd.run(args):\n     File \"/home/bkeene/git/git-p4\", line 2381, in run\n         ok = self.applyCommit(commit)\n     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n         p4_write_pipe(['submit', '-i'], submitTemplate)\n     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n         return write_pipe(real_cmd, stdin)\n     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n         die('Command failed: %s' % str(c))\n     File \"/home/bkeene/git/git-p4\", line 158, in die\n         raise Exception(msg)\n     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n\n* 9816.7\n\n  Traceback (most recent call last):\n    File \"/home/bkeene/git/git-p4\", line 4227, in \n      main()\n    File \"/home/bkeene/git/git-p4\", line 4221, in main\n      if not cmd.run(args):\n    File \"/home/bkeene/git/git-p4\", line 2381, in run\n      ok = self.applyCommit(commit)\n    File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n      p4_write_pipe(['submit', '-i'], submitTemplate)\n    File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n      return write_pipe(real_cmd, stdin)\n    File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n      die('Command failed: %s' % str(c))\n    File \"/home/bkeene/git/git-p4\", line 158, in die\n      raise Exception(msg)\n  Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n\n* 9816.9\n\n     Traceback (most recent call last):\n     File \"/home/bkeene/git/git-p4\", line 4227, in \n         main()\n     File \"/home/bkeene/git/git-p4\", line 4221, in main\n         if not cmd.run(args):\n     File \"/home/bkeene/git/git-p4\", line 2381, in run\n         ok = self.applyCommit(commit)\n     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n         p4_write_pipe(['submit', '-i'], submitTemplate)\n     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n         return write_pipe(real_cmd, stdin)\n     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n         die('Command failed: %s' % str(c))\n     File \"/home/bkeene/git/git-p4\", line 158, in die\n         raise Exception(msg)\n     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n\n* 9810.16\n\n     Traceback (most recent call last):\n     File \"/home/bkeene/git/git-p4\", line 4227, in \n         main()\n     File \"/home/bkeene/git/git-p4\", line 4221, in main\n         if not cmd.run(args):\n     File \"/home/bkeene/git/git-p4\", line 2436, in run\n         rebase.rebase()\n     File \"/home/bkeene/git/git-p4\", line 3913, in rebase\n         system(\"git rebase %s\" % upstream)\n     File \"/home/bkeene/git/git-p4\", line 305, in system\n         raise CalledProcessError(retcode, cmd)\n     subprocess.CalledProcessError: Command 'git rebase \nremotes/p4/master' returned non-zero exit status 1\n\nThe last test was a breaking test that stopped the test make.\n\n\n> If you're OK with it, I would prefer that we work from my version as a base and\n> add some of your quality-of-life enhancements on top. I can do the merges myself\n> if you are pressed for time.\n>\n> Thanks,\n> Yang\n\nI am not a Python developer and my code is further behind than yours, so\nit makes complete sense to use yours as the base.\n\n\n"},{"id":"387816","messageId":"ec301179-f9dc-4148-8634-2abc9263af5f@gmail.com","threadId":"52399","inReplyTo":"afa761cf-9c0e-cdcc-9c32-be88c5507042@gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Ben Keene","fromEmail":"seraphire@gmail.com","sentAt":"2019-12-09T18:54:53Z","receivedAt":"2019-12-09T18:54:59Z","isPatch":true,"sender":{"key":"seraphire@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22523774?v=4"},"body":"\nOn 12/9/2019 10:03 AM, Ben Keene wrote:\n>\n> On 12/7/2019 2:59 PM, Yang Zhao wrote:\n>> On Sat, Dec 7, 2019 at 8:21 AM Ben Keene wrote:\n>>> On 12/7/2019 2:29 AM, Yang Zhao wrote:\n>>>> Ideally, I would prefer we land something minimal and working in \n>>>> mainline soon,\n>>>> then further collaborate on changes that clean up code and enable \n>>>> more features.\n>>>>\n>>>> My end-game is to have P4 Streams working in git-p4, and maybe \n>>>> LFS-like support\n>>>> that uses p4 as the backend. It would be great to not be the only one\n>>>> spending effort\n>>>> in that direction.\n>>> I have similar goals.  I would love to get the smallest set of \n>>> non-breaking\n>>> changes in that allows the program to basically work with Python 3.5+.\n>>>\n>>> My rush has been because I need to use git-p4 for work and have been\n>>> working\n>>> on the project at the office.  Once I reach a point where I am able to\n>>> generally work (when t9800 is complete) I'll really not be free to \n>>> spend\n>>> too\n>>> much work time on the project, but I am eager to see this through!\n>> I'm in a similar situation, but we use p4 Streams and so I actually \n>> need further\n>> development before being able to make a full switch. I am given more \n>> liberty\n>> in terms of how much work time I can dedicate to this, though.\n>>\n>> Given the situation, can you give my patch set a try in your work \n>> environment?\n>> It is currently passing everything except t9824-git-p4-git-lfs.\n>\n> I downloaded your code and it looks like it works for Python 2.7. I'm \n> seeing errors with the following tests:\n>\n> * 9816.5\n>\n>     Traceback (most recent call last):\n>     File \"/home/bkeene/git/git-p4\", line 4227, in \n>         main()\n>     File \"/home/bkeene/git/git-p4\", line 4221, in main\n>         if not cmd.run(args):\n>     File \"/home/bkeene/git/git-p4\", line 2381, in run\n>         ok = self.applyCommit(commit)\n>     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n>         p4_write_pipe(['submit', '-i'], submitTemplate)\n>     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n>         return write_pipe(real_cmd, stdin)\n>     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n>         die('Command failed: %s' % str(c))\n>     File \"/home/bkeene/git/git-p4\", line 158, in die\n>         raise Exception(msg)\n>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n>\n> * 9816.6\n>\n>     Traceback (most recent call last):\n>     File \"/home/bkeene/git/git-p4\", line 4227, in \n>         main()\n>     File \"/home/bkeene/git/git-p4\", line 4221, in main\n>         if not cmd.run(args):\n>     File \"/home/bkeene/git/git-p4\", line 2381, in run\n>         ok = self.applyCommit(commit)\n>     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n>         p4_write_pipe(['submit', '-i'], submitTemplate)\n>     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n>         return write_pipe(real_cmd, stdin)\n>     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n>         die('Command failed: %s' % str(c))\n>     File \"/home/bkeene/git/git-p4\", line 158, in die\n>         raise Exception(msg)\n>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n>\n> * 9816.7\n>\n>  Traceback (most recent call last):\n>    File \"/home/bkeene/git/git-p4\", line 4227, in \n>      main()\n>    File \"/home/bkeene/git/git-p4\", line 4221, in main\n>      if not cmd.run(args):\n>    File \"/home/bkeene/git/git-p4\", line 2381, in run\n>      ok = self.applyCommit(commit)\n>    File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n>      p4_write_pipe(['submit', '-i'], submitTemplate)\n>    File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n>      return write_pipe(real_cmd, stdin)\n>    File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n>      die('Command failed: %s' % str(c))\n>    File \"/home/bkeene/git/git-p4\", line 158, in die\n>      raise Exception(msg)\n>  Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n>\n> * 9816.9\n>\n>     Traceback (most recent call last):\n>     File \"/home/bkeene/git/git-p4\", line 4227, in \n>         main()\n>     File \"/home/bkeene/git/git-p4\", line 4221, in main\n>         if not cmd.run(args):\n>     File \"/home/bkeene/git/git-p4\", line 2381, in run\n>         ok = self.applyCommit(commit)\n>     File \"/home/bkeene/git/git-p4\", line 2106, in applyCommit\n>         p4_write_pipe(['submit', '-i'], submitTemplate)\n>     File \"/home/bkeene/git/git-p4\", line 207, in p4_write_pipe\n>         return write_pipe(real_cmd, stdin)\n>     File \"/home/bkeene/git/git-p4\", line 201, in write_pipe\n>         die('Command failed: %s' % str(c))\n>     File \"/home/bkeene/git/git-p4\", line 158, in die\n>         raise Exception(msg)\n>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']\n>\n> * 9810.16\n>\n>     Traceback (most recent call last):\n>     File \"/home/bkeene/git/git-p4\", line 4227, in \n>         main()\n>     File \"/home/bkeene/git/git-p4\", line 4221, in main\n>         if not cmd.run(args):\n>     File \"/home/bkeene/git/git-p4\", line 2436, in run\n>         rebase.rebase()\n>     File \"/home/bkeene/git/git-p4\", line 3913, in rebase\n>         system(\"git rebase %s\" % upstream)\n>     File \"/home/bkeene/git/git-p4\", line 305, in system\n>         raise CalledProcessError(retcode, cmd)\n>     subprocess.CalledProcessError: Command 'git rebase \n> remotes/p4/master' returned non-zero exit status 1\n>\n> The last test was a breaking test that stopped the test make.\n>\n>\n>> If you're OK with it, I would prefer that we work from my version as \n>> a base and\n>> add some of your quality-of-life enhancements on top. I can do the \n>> merges myself\n>> if you are pressed for time.\n>>\n>> Thanks,\n>> Yang\n>\n> I am not a Python developer and my code is further behind than yours, so\n> it makes complete sense to use yours as the base.\n\n\nSo, I just attempted to run a base case on windows: git p4 clone //depot \nand I'm getting an error:\n\nDepot paths must start with \"//\": /depot\n\n"},{"id":"387828","messageId":"nycvar.QRO.7.76.6.1912092043470.31080@tvgsbejvaqbjf.bet","threadId":"52399","inReplyTo":"ec301179-f9dc-4148-8634-2abc9263af5f@gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-09T19:48:45Z","receivedAt":"2019-12-09T19:49:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ben,\n\nOn Mon, 9 Dec 2019, Ben Keene wrote:\n\n> So, I just attempted to run a base case on windows: git p4 clone //depot and\n> I'm getting an error:\n>\n> Depot paths must start with \"//\": /depot\n\nYou started this in a Bash, right?\n\nThe Git Bash has the very specific problem that many of Git's shell\nscripts assume that forward slashes are directory separators, not\nbackslashes, and that absolute paths start with a single forward slash. In\nother words, they expect Unix paths.\n\nBut we're on Windows! So the MSYS2 runtime (which is the POSIX emulation\nlayer derived from Cygwin which allows us to build and run Bash on\nWindows) \"translates\" between the paths. For example, if you pass `/depot`\nas a parameter to a Git command, the MSYS2 runtime notices that `git.exe`\nis not an MSYS2 program (i.e. it does not understand pseudo-Unix paths),\nand translates the path to `C:/Program Files/Git/depot`.\n\nHowever, your call has _two_ slashes, right? That is unfortunately MSYS2's\ntrick to say \"oh BTW keep the slash, this is not a Unix path\".\n\nTo avoid this, just set `MSYS_NO_PATHCONV`, like so:\n\n\tMSYS_NO_PATHCONV=1 git p4 clone //depot\n\nThis behavior is documented in our release notes, by the way:\nhttps://github.com/git-for-windows/build-extra/blob/master/ReleaseNotes.md#known-issues\n\nCiao,\nJohannes\n"},{"id":"387830","messageId":"CABvFv3LAPPib-Lz+2MQvyZdq2qrmFTxN-Ya9ACnGg32d3tO9Rg@mail.gmail.com","threadId":"52399","inReplyTo":"afa761cf-9c0e-cdcc-9c32-be88c5507042@gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-09T20:21:37Z","receivedAt":"2019-12-09T20:19:59Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Mon, Dec 9, 2019 at 7:03 AM Ben Keene wrote:\n> I downloaded your code and it looks like it works for Python 2.7. I'm\n> seeing errors with the following tests:\n>\n> * 9816.5\n> ...\n\nI'm not sure why those would fail for your local environment but not in CI.\n\nI've just pushed an updated PR to GitHub which is now passing all\ntests on python-3.5. Give that a go.\n\nIf tests are still failing for you, it'd be good to get the verbose\noutput from the specific test scripts. They don't tell us much without\nit.\n\ne.g.:\n ~/git.git/t $ : ./t9816-git-p4-locked.sh --verbose\n\nThanks,\nYang\n"},{"id":"387878","messageId":"20191210103014.GF6527@szeder.dev","threadId":"52399","inReplyTo":"20191207003333.3228-2-yang.zhao@skyboxlabs.com","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-10T10:30:14Z","receivedAt":"2019-12-10T10:30:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Dec 06, 2019 at 04:33:19PM -0800, Yang Zhao wrote:\n> git-p4.py includes support for python-3, but this was not previously\n> validated in CI. Lets actually do that.\n> \n> There is no tangible benefit to repeating python-3 tests for all\n> environments, so only limit it to linux-gcc for now.\n\nIn the subject line and the commit message body you speak about CI in\ngeneral, without sinling out a particular CI system ...\n\n> azure-pipelines.yml | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n\n... but the patch only modifies 'azure-pipelines.yml', and not\n'.travis.yml'.\n\n> diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> index 37ed7e06c6..d5f9413248 100644\n> --- a/azure-pipelines.yml\n> +++ b/azure-pipelines.yml\n> @@ -331,7 +331,18 @@ jobs:\n> displayName: linux-gcc\n> condition: succeeded()\n> pool: Hosted Ubuntu 1604\n> + strategy:\n> + matrix:\n> + python27:\n> + python.version: '2.7'\n> + python37:\n> + python.version: '3.7'\n> steps:\n> + - task: UsePythonVersion@0\n> + inputs:\n> + versionSpec: '$(python.version)'\n> + - bash: |\n> + echo \"##vso[task.setvariable variable=python_path]$(which python)\"\n\nI don't speak 'azure-pipelines.yml', so question: will this build Git\nand run the whole test suite twice, once with Python 2.7 and once with\n3.7? I'm asking because 'git-p4' is the one and only Python script we\nhave, with no plans for more, so running the whole test suite with a\ndifferent Python version for a second time instead of running only the\n'git-p4'-specific tests (t98*) seems to be quite wasteful.\n\nFurthermore, this is the first patch of the series, with all the\nPython3 fixes in subsequent commits, so the Azure Pipelines build with\nPython 3.7 would fail with only this patch, wouldn't it? I think this\npatch should be the last in the series, after all the Python 2 vs 3\nissues are sorted out.\n\n> - bash: |\n> test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n> \n> -- \n> 2.21.0.windows.1\n> \n"},{"id":"387890","messageId":"caa4b235-8ec8-0b6f-49e5-3c95e3a5f5e3@gmail.com","threadId":"52399","inReplyTo":"nycvar.QRO.7.76.6.1912092043470.31080@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Ben Keene","fromEmail":"seraphire@gmail.com","sentAt":"2019-12-10T14:20:05Z","receivedAt":"2019-12-10T14:20:09Z","isPatch":true,"sender":{"key":"seraphire@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22523774?v=4"},"body":"On 12/9/2019 2:48 PM, Johannes Schindelin wrote:\n> Hi Ben,\n>\n> On Mon, 9 Dec 2019, Ben Keene wrote:\n>\n>> So, I just attempted to run a base case on windows: git p4 clone //depot and\n>> I'm getting an error:\n>>\n>> Depot paths must start with \"//\": /depot\n> You started this in a Bash, right?\nNo, I started it from a windows command cmd.exe prompt.  (I almost never \nuse the bash prompt)\n>\n> The Git Bash has the very specific problem that many of Git's shell\n> scripts assume that forward slashes are directory separators, not\n> backslashes, and that absolute paths start with a single forward slash. In\n> other words, they expect Unix paths.\n>\n> But we're on Windows! So the MSYS2 runtime (which is the POSIX emulation\n> layer derived from Cygwin which allows us to build and run Bash on\n> Windows) \"translates\" between the paths. For example, if you pass `/depot`\n> as a parameter to a Git command, the MSYS2 runtime notices that `git.exe`\n> is not an MSYS2 program (i.e. it does not understand pseudo-Unix paths),\n> and translates the path to `C:/Program Files/Git/depot`.\nThat is good to know!\n>\n> However, your call has _two_ slashes, right? That is unfortunately MSYS2's\n> trick to say \"oh BTW keep the slash, this is not a Unix path\".\n>\n> To avoid this, just set `MSYS_NO_PATHCONV`, like so:\n\nWhen I first installed git, I didn't read the release notes. (Shame on \nme!) and I installed\npython for windows and added an alias for git-p4.py against the windows \nversion of\npython, so when I run git, it's not performing that conversion.\n\n> \tMSYS_NO_PATHCONV=1 git p4 clone //depot\n>\n> This behavior is documented in our release notes, by the way:\n> https://github.com/git-for-windows/build-extra/blob/master/ReleaseNotes.md#known-issues\n>\n> Ciao,\n> Johannes\n\n\nI'm starting to run out of time at work, so I'll be slow to try and \nrepro this.\n\nThanks for the info!\n\n- Ben\n\n"},{"id":"387902","messageId":"CABvFv3Lud80UzFXa6BRMGLwRV6gsJpNcs-mrgOiNHoJL0d+koA@mail.gmail.com","threadId":"52399","inReplyTo":"20191210103014.GF6527@szeder.dev","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-10T19:11:09Z","receivedAt":"2019-12-10T19:09:31Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Tue, Dec 10, 2019 at 2:30 AM SZEDER Gábor wrote:\n> On Fri, Dec 06, 2019 at 04:33:19PM -0800, Yang Zhao wrote:\n> > diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> > index 37ed7e06c6..d5f9413248 100644\n> > --- a/azure-pipelines.yml\n> > +++ b/azure-pipelines.yml\n> > @@ -331,7 +331,18 @@ jobs:\n> > displayName: linux-gcc\n> > condition: succeeded()\n> > pool: Hosted Ubuntu 1604\n> > + strategy:\n> > + matrix:\n> > + python27:\n> > + python.version: '2.7'\n> > + python37:\n> > + python.version: '3.7'\n> > steps:\n> > + - task: UsePythonVersion@0\n> > + inputs:\n> > + versionSpec: '$(python.version)'\n> > + - bash: |\n> > + echo \"##vso[task.setvariable variable=python_path]$(which python)\"\n>\n> I don't speak 'azure-pipelines.yml', so question: will this build Git\n> and run the whole test suite twice, once with Python 2.7 and once with\n> 3.7? I'm asking because 'git-p4' is the one and only Python script we\n> have, with no plans for more, so running the whole test suite with a\n> different Python version for a second time instead of running only the\n> 'git-p4'-specific tests (t98*) seems to be quite wasteful.\n\nThe CI scripts as it is currently does not separate compiling and testing for\nnon-Windows builds. I don't see a good way to only run a specific set of tests\ngiven a particular environment without re-architecturing the CI pipeline.\n\nFurthermore, there's a step in the build that hard-codes the\nenvironment's python\npath into the installed version of the script. This complicates being\nable to even create\na `git-p4` that runs under different python environments in Azure\nPipelines due to how\n`UsePythonVersion@0` pulls python into version-specific directories.\nI haven't dug into\nwhy this hardcoding is done in the first place.\n\nSo, the question is if it's worth doing this work now when the desire\nseems to be dropping\npython-2.7 completely in the (near?) future.\n\n-- \nYang\n"},{"id":"388013","messageId":"20191212141322.GK6527@szeder.dev","threadId":"52399","inReplyTo":"CABvFv3Lud80UzFXa6BRMGLwRV6gsJpNcs-mrgOiNHoJL0d+koA@mail.gmail.com","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-12T14:13:22Z","receivedAt":"2019-12-12T14:13:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Dec 10, 2019 at 11:11:09AM -0800, Yang Zhao wrote:\n> On Tue, Dec 10, 2019 at 2:30 AM SZEDER Gábor wrote:\n> > On Fri, Dec 06, 2019 at 04:33:19PM -0800, Yang Zhao wrote:\n> > > diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> > > index 37ed7e06c6..d5f9413248 100644\n> > > --- a/azure-pipelines.yml\n> > > +++ b/azure-pipelines.yml\n> > > @@ -331,7 +331,18 @@ jobs:\n> > > displayName: linux-gcc\n> > > condition: succeeded()\n> > > pool: Hosted Ubuntu 1604\n> > > + strategy:\n> > > + matrix:\n> > > + python27:\n> > > + python.version: '2.7'\n> > > + python37:\n> > > + python.version: '3.7'\n> > > steps:\n> > > + - task: UsePythonVersion@0\n> > > + inputs:\n> > > + versionSpec: '$(python.version)'\n> > > + - bash: |\n> > > + echo \"##vso[task.setvariable variable=python_path]$(which python)\"\n> >\n> > I don't speak 'azure-pipelines.yml', so question: will this build Git\n> > and run the whole test suite twice, once with Python 2.7 and once with\n> > 3.7? I'm asking because 'git-p4' is the one and only Python script we\n> > have, with no plans for more, so running the whole test suite with a\n> > different Python version for a second time instead of running only the\n> > 'git-p4'-specific tests (t98*) seems to be quite wasteful.\n> \n> The CI scripts as it is currently does not separate compiling and testing for\n> non-Windows builds. I don't see a good way to only run a specific set of tests\n> given a particular environment without re-architecturing the CI pipeline.\n\nBuilding git and running the test suite is encapsulated in the\n'ci/run-build-and-tests.sh' script, while installing dependencies is\nencapsulated in 'ci/install-dependencies.sh', just in case Azure\nPipelines Linux images don't contain both Python 2 and 3 (Travis CI\nimages contain 2.7 and 3.5) So I don't think it's necessary to touch\n'azure-pipelines.yml' or '.travis.yml' at all.\n\n> Furthermore, there's a step in the build that hard-codes the\n> environment's python\n> path into the installed version of the script. This complicates being\n> able to even create\n> a `git-p4` that runs under different python environments in Azure\n> Pipelines due to how\n> `UsePythonVersion@0` pulls python into version-specific directories.\n\nThe PYTHON_PATH that we build 'git p4' with can be a symbolink link,\nand then choosing which Python version to use is only a matter of\npointing that symbolic link to the python binary of the desired\nversion.\n\nIn fact our default PYTHON_PATH is '/usr/bin/python', which is a\nsymbolic link pointing to 'python2.7' on Ubuntu 16.04, including the\nTravis CI's images that we use.\n\n> I haven't dug into\n> why this hardcoding is done in the first place.\n> \n> So, the question is if it's worth doing this work now when the desire\n> seems to be dropping\n> python-2.7 completely in the (near?) future.\n> \n> -- \n> Yang\n"},{"id":"388033","messageId":"CABvFv3J8JjXGeAXSWDmK5zDav8qYNQ6Ce-8dPGAmuySGj8xvNg@mail.gmail.com","threadId":"52399","inReplyTo":"20191212141322.GK6527@szeder.dev","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-12T17:04:24Z","receivedAt":"2019-12-12T17:04:43Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Thu, Dec 12, 2019 at 6:13 AM SZEDER Gábor wrote:\n>\n> > The CI scripts as it is currently does not separate compiling and testing for\n> > non-Windows builds. I don't see a good way to only run a specific set of tests\n> > given a particular environment without re-architecturing the CI pipeline.\n>\n> Building git and running the test suite is encapsulated in the\n> 'ci/run-build-and-tests.sh' script, while installing dependencies is\n> encapsulated in 'ci/install-dependencies.sh', just in case Azure\n> Pipelines Linux images don't contain both Python 2 and 3 (Travis CI\n> images contain 2.7 and 3.5) So I don't think it's necessary to touch\n> 'azure-pipelines.yml' or '.travis.yml' at all.\n\nYes, and this is implemented as a single step as far as the CI\npipeline is concerned. It does not produce a build artifact that can\nthen be loaded into multiple environments for running tests.\n\nUnless there's a very good reason to _not_ use Azure Pipeline's\nbuilt-in Python version selection support, I believe it's more\ndesirable in the long-run to leverage the feature rather than maintain\nsome custom solution.\n\n-- \nYang\n"},{"id":"388034","messageId":"20191212171516.GL6527@szeder.dev","threadId":"52399","inReplyTo":"CABvFv3J8JjXGeAXSWDmK5zDav8qYNQ6Ce-8dPGAmuySGj8xvNg@mail.gmail.com","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-12T17:15:16Z","receivedAt":"2019-12-12T17:15:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Dec 12, 2019 at 09:04:24AM -0800, Yang Zhao wrote:\n> On Thu, Dec 12, 2019 at 6:13 AM SZEDER Gábor wrote:\n> >\n> > > The CI scripts as it is currently does not separate compiling and testing for\n> > > non-Windows builds. I don't see a good way to only run a specific set of tests\n> > > given a particular environment without re-architecturing the CI pipeline.\n> >\n> > Building git and running the test suite is encapsulated in the\n> > 'ci/run-build-and-tests.sh' script, while installing dependencies is\n> > encapsulated in 'ci/install-dependencies.sh', just in case Azure\n> > Pipelines Linux images don't contain both Python 2 and 3 (Travis CI\n> > images contain 2.7 and 3.5) So I don't think it's necessary to touch\n> > 'azure-pipelines.yml' or '.travis.yml' at all.\n> \n> Yes, and this is implemented as a single step as far as the CI\n> pipeline is concerned. It does not produce a build artifact that can\n> then be loaded into multiple environments for running tests.\n\nI don't understand what artifact should be loaded into what\nenvironments...\n\n> Unless there's a very good reason to _not_ use Azure Pipeline's\n> built-in Python version selection support, I believe it's more\n> desirable in the long-run to leverage the feature rather than maintain\n> some custom solution.\n\nAzure Pipelines's built-in Python version selection support only works\non Azure Pipelines, therefore it's more desirable to have a general\nsolution.\n\n"},{"id":"388037","messageId":"CABvFv3Ky4K4dFFCJogpY_8Z7Qk3HiUQUMqSch+B+fKDXjjfUeA@mail.gmail.com","threadId":"52399","inReplyTo":"20191212171516.GL6527@szeder.dev","subject":"Re: [PATCH 01/13] ci: also run linux-gcc pipeline with python-3.7 environment","fromName":"Yang Zhao","fromEmail":"yang.zhao@skyboxlabs.com","sentAt":"2019-12-12T19:02:48Z","receivedAt":"2019-12-12T19:01:09Z","isPatch":true,"sender":{"key":"yang.zhao@skyboxlabs.com","avatar":"https://avatars.githubusercontent.com/u/45857825?v=4"},"body":"On Thu, Dec 12, 2019 at 9:15 AM SZEDER Gábor wrote:\n>\n> On Thu, Dec 12, 2019 at 09:04:24AM -0800, Yang Zhao wrote:\n> > Unless there's a very good reason to _not_ use Azure Pipeline's\n> > built-in Python version selection support, I believe it's more\n> > desirable in the long-run to leverage the feature rather than maintain\n> > some custom solution.\n>\n> Azure Pipelines's built-in Python version selection support only works\n> on Azure Pipelines, therefore it's more desirable to have a general\n> solution.\n\nThat's fair. However, if we actually want to have something unified\nthat works for Linux and macOS (t90** isn't run on Windows afaict)\nthen I won't have the bandwidth for it in the near term. I'd be more\ninclined to drop the CI changes from the series if we don't want a\nstop-gap in the meantime.\n"},{"id":"388119","messageId":"27d379c4-bc84-8219-ac0d-0b84fbdc0ff0@gmail.com","threadId":"52399","inReplyTo":"CABvFv3LAPPib-Lz+2MQvyZdq2qrmFTxN-Ya9ACnGg32d3tO9Rg@mail.gmail.com","subject":"Re: [PATCH 00/13] git-p4: python3 compatibility","fromName":"Ben Keene","fromEmail":"seraphire@gmail.com","sentAt":"2019-12-13T17:10:22Z","receivedAt":"2019-12-13T20:39:38Z","isPatch":true,"sender":{"key":"seraphire@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22523774?v=4"},"body":"Here's a patch I have on my tree that I would offer -\nit removes references to basestring and should be a drop in patch.\n\n\n From 1cc3c0f8570adb1ef2bacc0009aac979a3263d70 Mon Sep 17 00:00:00 2001\nFrom: Ben Keene \nDate: Tue, 3 Dec 2019 16:36:26 -0500\nSubject: [PATCH] git-p4: change the expansion test from basestring to list\n\nPython 3 handles strings differently than Python 2.7. Since Python 2\nis reaching it's end of life, a series of changes are being submitted to\nenable python 3.5 and following support. The current code fails basic\ntests under python 3.5.\n\nSome codepaths can represent a command line the program\ninternally prepares to execute either as a single string\n(i.e. each token properly quoted, concatenated with $IFS) or\nas a list of argv[] elements, and there are 9 places where\nwe say \"if X is isinstance(_, basestring), then do this\nthing to handle X as a command line in a single string; if\nnot, X is a command line in a list form\".\n\nThis does not work well with Python 3, as there is no\nbasestring (everything is Unicode now), and even with Python\n2, it was not an ideal way to tell the two cases apart,\nbecause an internally formed command line could have been in\na single Unicode string.\n\nFlip the check to say \"if X is not a list, then handle X as\na command line in a single string; otherwise treat it as a\ncommand line in a list form\".\n\nThis will get rid of references to 'basestring', to migrate\nthe code ready for Python 3.\n\nThanks-to: Junio C Hamano \nSigned-off-by: Ben Keene \n---\n  git-p4.py | 18 +++++++++---------\n  1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 25d8012e23..d322ae20ef 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -98,7 +98,7 @@ def p4_build_cmd(cmd):\n          # Provide a way to not pass this option by setting \ngit-p4.retries to 0\n          real_cmd += [\"-r\", str(retries)]\n\n-    if isinstance(cmd,basestring):\n+    if not isinstance(cmd, list):\n          real_cmd = ' '.join(real_cmd) + ' ' + cmd\n      else:\n          real_cmd += cmd\n@@ -192,7 +192,7 @@ def write_pipe(c, stdin):\n      if verbose:\n          sys.stderr.write('Writing pipe: %s\\n' % str(c))\n\n-    expand = isinstance(c,basestring)\n+    expand = not isinstance(c, list)\n      p = subprocess.Popen(c, stdin=subprocess.PIPE, shell=expand)\n      pipe = p.stdin\n      val = pipe.write(stdin)\n@@ -214,7 +214,7 @@ def read_pipe_full(c):\n      if verbose:\n          sys.stderr.write('Reading pipe: %s\\n' % str(c))\n\n-    expand = isinstance(c,basestring)\n+    expand = not isinstance(c, list)\n      p = subprocess.Popen(c, stdout=subprocess.PIPE, \nstderr=subprocess.PIPE, shell=expand)\n      (out, err) = p.communicate()\n      return (p.returncode, out, decode_text_stream(err))\n@@ -254,7 +254,7 @@ def read_pipe_lines(c):\n      if verbose:\n          sys.stderr.write('Reading pipe: %s\\n' % str(c))\n\n-    expand = isinstance(c, basestring)\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@@ -297,7 +297,7 @@ def p4_has_move_command():\n      return True\n\n  def system(cmd, ignore_error=False):\n-    expand = isinstance(cmd,basestring)\n+    expand = not isinstance(cmd, list)\n      if verbose:\n          sys.stderr.write(\"executing %s\\n\" % str(cmd))\n      retcode = subprocess.call(cmd, shell=expand)\n@@ -309,7 +309,7 @@ def system(cmd, ignore_error=False):\n  def p4_system(cmd):\n      \"\"\"Specifically invoke p4 as the system command. \"\"\"\n      real_cmd = p4_build_cmd(cmd)\n-    expand = isinstance(real_cmd, basestring)\n+    expand = not isinstance(real_cmd, list)\n      retcode = subprocess.call(real_cmd, shell=expand)\n      if retcode:\n          raise CalledProcessError(retcode, real_cmd)\n@@ -547,7 +547,7 @@ def getP4OpenedType(file):\n  # Return the set of all p4 labels\n  def getP4Labels(depotPaths):\n      labels = set()\n-    if isinstance(depotPaths,basestring):\n+    if not isinstance(depotPaths, list):\n          depotPaths = [depotPaths]\n\n      for l in p4CmdList([\"labels\"] + [\"%s...\" % p for p in depotPaths]):\n@@ -633,7 +633,7 @@ def isModeExecChanged(src_mode, dst_mode):\n  def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,\n          errors_as_exceptions=False):\n\n-    if isinstance(cmd,basestring):\n+    if not isinstance(cmd, list):\n          cmd = \"-G \" + cmd\n          expand = True\n      else:\n@@ -650,7 +650,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', \ncb=None, skip_info=False,\n      stdin_file = None\n      if stdin is not None:\n          stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', \nmode=stdin_mode)\n-        if isinstance(stdin,basestring):\n+        if not isinstance(stdin, list):\n              stdin_file.write(stdin)\n          else:\n              for i in stdin:\n-- \n2.24.1.windows.2\n\n\n"}]}