RE: [PATCH] git-p4: Fix git-p4.mapUser on Windows

On 21 Jan 2017, at 13:33, Lars Schneider [off-list ref]

quoted

On 21 Jan 2017, at 13:02, George Vanburgh [off-list ref]

wrote:

quoted

From: George Vanburgh <redacted>

When running git-p4 on Windows, with multiple git-p4.mapUser entries
in git config - no user mappings are applied to the generated

repository.

quoted

Reproduction Steps:

1. Add multiple git-p4.mapUser entries to git config on a Windows
  machine
2. Attempt to clone a p4 repository

None of the user mappings will be applied.

This issue is caused by the fact that gitConfigList, uses
split(os.linesep) to convert the output of git config --get-all into a
list.

On Windows, os.linesep is equal to '\r\n' - however git.exe returns
configuration with a line seperator of '\n'. This leads to the list
returned by gitConfigList containing only one element - which contains
the full output of git config --get-all in string form. This causes
problems for the code introduced to getUserMapFromPerforceServer in
10d08a1.

This issue should be caught by the test introduced in 10d08a1, and
would require running on Windows to reproduce. When running inside
MinGW/Cygwin, however, os.linesep correctly returns '\n', and
everything works as expected.

This surprises me. I would expect `\r\n` in a MinGW env...
Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests

on

Windows...

It appears I was mistaken - the successful tests I ran were actually under 
the Ubuntu subsystem for Windows, which (obviously) passed. 

Just did a quick experiment:

Git Bash (MinGW):
georg@TEMPEST MINGW64 ~
$ python -c "import os
print(repr(os.linesep))"
'\r\n'

Powershell:
PS C:\Users\georg> python -c "import os

quoted

print(repr(os.linesep))"

'\r\n'

Ubuntu subsystem for Windows:
george@TEMPEST:~$ python -c "import os
print(repr(os.linesep))"
'\n'

So this issue applies to git-p4 running under both PowerShell and MinGW.

quoted

The simplest fix for this issue would be to convert the line split
logic inside gitConfigList to use splitlines(), which splits on any
standard line delimiter. However, this function was only introduced in
Python 2.7, and would mean a bump in the minimum required version of
Python required to run git-p4. The alternative fix, implemented here,
is to use '\n' as a delimiter, which git.exe appears to output
consistently on Windows anyway.

Well, that also means if we ever use splitlines() then your fix below

would

brake the code, right?

Python 2.7 was released 7 years ago in 2010. 

Now I feel old...

Therefore, I would vote to
bump the minimum version. But that's just my opinion :-)

I feel like splitlines is the better/safer fix - but figured bumping the
minimum 
Python version was probably part of a wider discussion. If it's something
people 
are comfortable with - I'd be happy to rework the fix to use splitlines.
Luke - do you have any thoughts on this?

quoted

Signed-off-by: George Vanburgh <redacted>
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index f427bf6..c134a58 100755
--- a/git-p4.py
+++ b/git-p4.py

@@ -656,7 +656,7 @@ def gitConfigInt(key):

def gitConfigList(key):
    if not _gitConfig.has_key(key):
        s = read_pipe(["git", "config", "--get-all", key],

ignore_error=True)

quoted

-        _gitConfig[key] = s.strip().split(os.linesep)
+        _gitConfig[key] = s.strip().split("\n")

I can't easily reproduce this as I don't have a running git-p4 setup on
Windows.
However, your explanation and your fix make sense to me. If we don't want
to bump the version then this looks good to me.

Cheers,
Lars