{"thread":{"id":"44929","subject":"[PATCH] git-p4: Fix git-p4.mapUser on Windows","startedAt":"2017-01-21T12:02:59Z","lastAt":"2017-01-30T17:07:33Z","messageCount":9,"participants":["George Vanburgh","Lars Schneider","Luke Diamand","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"309885","messageId":"01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com","threadId":"44929","inReplyTo":null,"subject":"[PATCH] git-p4: Fix git-p4.mapUser on Windows","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":"2017-01-21T12:02:36Z","receivedAt":"2017-01-21T12:02:59Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"From: George Vanburgh <gvanburgh@bloomberg.net>\n\nWhen running git-p4 on Windows, with multiple git-p4.mapUser entries in\ngit config - no user mappings are applied to the generated repository.\n\nReproduction Steps:\n\n1. Add multiple git-p4.mapUser entries to git config on a Windows\n   machine\n2. Attempt to clone a p4 repository\n\nNone of the user mappings will be applied.\n\nThis issue is caused by the fact that gitConfigList,\nuses split(os.linesep) to convert the output of\ngit config --get-all into a list.\n\nOn Windows, os.linesep is equal to '\\r\\n' - however git.exe\nreturns configuration with a line seperator of '\\n'. This leads to\nthe list returned by gitConfigList containing only one element - which\ncontains the full output of git config --get-all in string form. This\ncauses problems for the code introduced to getUserMapFromPerforceServer\nin 10d08a1.\n\nThis issue should be caught by the test introduced in 10d08a1, and\nwould require running on Windows to reproduce. When running inside\nMinGW/Cygwin, however, os.linesep correctly returns '\\n', and everything\nworks as expected.\n\nThe simplest fix for this issue would be to convert the line split logic\ninside gitConfigList to use splitlines(), which splits on any standard\nline delimiter. However, this function was only introduced in Python\n2.7, and would mean a bump in the minimum required version of Python\nrequired to run git-p4. The alternative fix, implemented here, is to use\n'\\n' as a delimiter, which git.exe appears to output consistently on\nWindows anyway.\n\nSigned-off-by: George Vanburgh <gvanburgh@bloomberg.net>\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 f427bf6..c134a58 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -656,7 +656,7 @@ def gitConfigInt(key):\n def gitConfigList(key):\n     if not _gitConfig.has_key(key):\n         s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n-        _gitConfig[key] = s.strip().split(os.linesep)\n+        _gitConfig[key] = s.strip().split(\"\\n\")\n         if _gitConfig[key] == ['']:\n             _gitConfig[key] = []\n     return _gitConfig[key]\n\n--\nhttps://github.com/git/git/pull/319\n"},{"id":"309890","messageId":"A7425283-9C32-4AE8-A442-11B7CFEAB4E8@gmail.com","threadId":"44929","inReplyTo":"01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] git-p4: Fix git-p4.mapUser on Windows","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-01-21T13:32:43Z","receivedAt":"2017-01-21T13:32:51Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me> wrote:\n> \n> From: George Vanburgh <gvanburgh@bloomberg.net>\n> \n> When running git-p4 on Windows, with multiple git-p4.mapUser entries in\n> git config - no user mappings are applied to the generated repository.\n> \n> Reproduction Steps:\n> \n> 1. Add multiple git-p4.mapUser entries to git config on a Windows\n>   machine\n> 2. Attempt to clone a p4 repository\n> \n> None of the user mappings will be applied.\n> \n> This issue is caused by the fact that gitConfigList,\n> uses split(os.linesep) to convert the output of\n> git config --get-all into a list.\n> \n> On Windows, os.linesep is equal to '\\r\\n' - however git.exe\n> returns configuration with a line seperator of '\\n'. This leads to\n> the list returned by gitConfigList containing only one element - which\n> contains the full output of git config --get-all in string form. This\n> causes problems for the code introduced to getUserMapFromPerforceServer\n> in 10d08a1.\n> \n> This issue should be caught by the test introduced in 10d08a1, and\n> would require running on Windows to reproduce. When running inside\n> MinGW/Cygwin, however, os.linesep correctly returns '\\n', and everything\n> works as expected.\n\nThis surprises me. I would expect `\\r\\n` in a MinGW env...\nNevertheless, I wouldn't have caught that as I don't run the git-p4 tests\non Windows...\n\n\n> The simplest fix for this issue would be to convert the line split logic\n> inside gitConfigList to use splitlines(), which splits on any standard\n> line delimiter. However, this function was only introduced in Python\n> 2.7, and would mean a bump in the minimum required version of Python\n> required to run git-p4. The alternative fix, implemented here, is to use\n> '\\n' as a delimiter, which git.exe appears to output consistently on\n> Windows anyway.\n\nWell, that also means if we ever use splitlines() then your fix below\nwould brake the code, right?\n\nPython 2.7 was released 7 years ago in 2010. Therefore, I would vote to\nbump the minimum version. But that's just my opinion :-)\n\n\n> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n> ---\n> git-p4.py | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index f427bf6..c134a58 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -656,7 +656,7 @@ def gitConfigInt(key):\n> def gitConfigList(key):\n>     if not _gitConfig.has_key(key):\n>         s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n> -        _gitConfig[key] = s.strip().split(os.linesep)\n> +        _gitConfig[key] = s.strip().split(\"\\n\")\n\nI can't easily reproduce this as I don't have a running git-p4 setup on Windows.\nHowever, your explanation and your fix make sense to me. If we don't want to bump\nthe version then this looks good to me.\n\nCheers,\nLars"},{"id":"309898","messageId":"000f01d273fa$05358ee0$0fa0aca0$@vanburgh.me","threadId":"44929","inReplyTo":"A7425283-9C32-4AE8-A442-11B7CFEAB4E8@gmail.com","subject":"RE: [PATCH] git-p4: Fix git-p4.mapUser on Windows","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":null,"receivedAt":"2017-01-21T15:21:28Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>\n> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>\n> wrote:\n> >\n> > From: George Vanburgh <gvanburgh@bloomberg.net>\n> >\n> > When running git-p4 on Windows, with multiple git-p4.mapUser entries\n> > in git config - no user mappings are applied to the generated\nrepository.\n> >\n> > Reproduction Steps:\n> >\n> > 1. Add multiple git-p4.mapUser entries to git config on a Windows\n> >   machine\n> > 2. Attempt to clone a p4 repository\n> >\n> > None of the user mappings will be applied.\n> >\n> > This issue is caused by the fact that gitConfigList, uses\n> > split(os.linesep) to convert the output of git config --get-all into a\n> > list.\n> >\n> > On Windows, os.linesep is equal to '\\r\\n' - however git.exe returns\n> > configuration with a line seperator of '\\n'. This leads to the list\n> > returned by gitConfigList containing only one element - which contains\n> > the full output of git config --get-all in string form. This causes\n> > problems for the code introduced to getUserMapFromPerforceServer in\n> > 10d08a1.\n> >\n> > This issue should be caught by the test introduced in 10d08a1, and\n> > would require running on Windows to reproduce. When running inside\n> > MinGW/Cygwin, however, os.linesep correctly returns '\\n', and\n> > everything works as expected.\n> \n> This surprises me. I would expect `\\r\\n` in a MinGW env...\n> Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests\non\n> Windows...\n\nIt appears I was mistaken - the successful tests I ran were actually under \nthe Ubuntu subsystem for Windows, which (obviously) passed. \n\nJust did a quick experiment:\n\nGit Bash (MinGW):\ngeorg@TEMPEST MINGW64 ~\n$ python -c \"import os\nprint(repr(os.linesep))\"\n'\\r\\n'\n\nPowershell:\nPS C:\\Users\\georg> python -c \"import os\n>> print(repr(os.linesep))\"\n'\\r\\n'\n\nUbuntu subsystem for Windows:\ngeorge@TEMPEST:~$ python -c \"import os\nprint(repr(os.linesep))\"\n'\\n'\n\nSo this issue applies to git-p4 running under both PowerShell and MinGW.\n\n> \n> \n> > The simplest fix for this issue would be to convert the line split\n> > logic inside gitConfigList to use splitlines(), which splits on any\n> > standard line delimiter. However, this function was only introduced in\n> > Python 2.7, and would mean a bump in the minimum required version of\n> > Python required to run git-p4. The alternative fix, implemented here,\n> > is to use '\\n' as a delimiter, which git.exe appears to output\n> > consistently on Windows anyway.\n> \n> Well, that also means if we ever use splitlines() then your fix below\nwould\n> brake the code, right?\n> \n> Python 2.7 was released 7 years ago in 2010. \n\nNow I feel old...\n\n> Therefore, I would vote to\n> bump the minimum version. But that's just my opinion :-)\n\nI feel like splitlines is the better/safer fix - but figured bumping the\nminimum \nPython version was probably part of a wider discussion. If it's something\npeople \nare comfortable with - I'd be happy to rework the fix to use splitlines.\nLuke - do you have any thoughts on this?\n\n> \n> \n> > Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n> > ---\n> > git-p4.py | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/git-p4.py b/git-p4.py\n> > index f427bf6..c134a58 100755\n> > --- a/git-p4.py\n> > +++ b/git-p4.py\n> > @@ -656,7 +656,7 @@ def gitConfigInt(key):\n> > def gitConfigList(key):\n> >     if not _gitConfig.has_key(key):\n> >         s = read_pipe([\"git\", \"config\", \"--get-all\", key],\nignore_error=True)\n> > -        _gitConfig[key] = s.strip().split(os.linesep)\n> > +        _gitConfig[key] = s.strip().split(\"\\n\")\n> \n> I can't easily reproduce this as I don't have a running git-p4 setup on\n> Windows.\n> However, your explanation and your fix make sense to me. If we don't want\n> to bump the version then this looks good to me.\n> \n> Cheers,\n> Lars\n\n"},{"id":"309927","messageId":"CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com","threadId":"44929","inReplyTo":"000f01d273fa$05358ee0$0fa0aca0$@vanburgh.me","subject":"Re: [PATCH] git-p4: Fix git-p4.mapUser on Windows","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-22T13:04:54Z","receivedAt":"2017-01-22T13:05:01Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"I'm confused....see below.\n\nOn 21 January 2017 at 15:21, George Vanburgh <george@vanburgh.me> wrote:\n>> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>\n>> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>\n>> wrote:\n>> >\n>> > From: George Vanburgh <gvanburgh@bloomberg.net>\n>> >\n>> > When running git-p4 on Windows, with multiple git-p4.mapUser entries\n>> > in git config - no user mappings are applied to the generated\n> repository.\n>> >\n>> > Reproduction Steps:\n>> >\n>> > 1. Add multiple git-p4.mapUser entries to git config on a Windows\n>> >   machine\n>> > 2. Attempt to clone a p4 repository\n>> >\n>> > None of the user mappings will be applied.\n>> >\n>> > This issue is caused by the fact that gitConfigList, uses\n>> > split(os.linesep) to convert the output of git config --get-all into a\n>> > list.\n>> >\n>> > On Windows, os.linesep is equal to '\\r\\n' - however git.exe returns\n>> > configuration with a line seperator of '\\n'. This leads to the list\n>> > returned by gitConfigList containing only one element - which contains\n>> > the full output of git config --get-all in string form. This causes\n>> > problems for the code introduced to getUserMapFromPerforceServer in\n>> > 10d08a1.\n>> >\n>> > This issue should be caught by the test introduced in 10d08a1, and\n>> > would require running on Windows to reproduce. When running inside\n>> > MinGW/Cygwin, however, os.linesep correctly returns '\\n', and\n>> > everything works as expected.\n>>\n>> This surprises me. I would expect `\\r\\n` in a MinGW env...\n>> Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests\n> on\n>> Windows...\n>\n> It appears I was mistaken - the successful tests I ran were actually under\n> the Ubuntu subsystem for Windows, which (obviously) passed.\n>\n> Just did a quick experiment:\n>\n> Git Bash (MinGW):\n> georg@TEMPEST MINGW64 ~\n> $ python -c \"import os\n> print(repr(os.linesep))\"\n> '\\r\\n'\n>\n> Powershell:\n> PS C:\\Users\\georg> python -c \"import os\n>>> print(repr(os.linesep))\"\n> '\\r\\n'\n>\n> Ubuntu subsystem for Windows:\n> george@TEMPEST:~$ python -c \"import os\n> print(repr(os.linesep))\"\n> '\\n'\n>\n> So this issue applies to git-p4 running under both PowerShell and MinGW.\n>\n>>\n>>\n>> > The simplest fix for this issue would be to convert the line split\n>> > logic inside gitConfigList to use splitlines(), which splits on any\n>> > standard line delimiter. However, this function was only introduced in\n>> > Python 2.7, and would mean a bump in the minimum required version of\n>> > Python required to run git-p4. The alternative fix, implemented here,\n>> > is to use '\\n' as a delimiter, which git.exe appears to output\n>> > consistently on Windows anyway.\n\nHave you tried a 2.6 Python with splitlines? I just tried this code\nand it seems fine.\n\n> val = s.strip().splitlines()\n> print(\"splitlines:\", val)\n\nTried with 2.7 and 2.6, and bot print out an array of strings returned\nfrom read_pipe().\n\nAnd 'grep -r splitlines' on the Python2.6 source has lots of hits.\n\n>>\n>> Well, that also means if we ever use splitlines() then your fix below\n> would\n>> brake the code, right?\n>>\n>> Python 2.7 was released 7 years ago in 2010.\n>\n> Now I feel old...\n\n:-)\n\n>\n>> Therefore, I would vote to\n>> bump the minimum version. But that's just my opinion :-)\n>\n> I feel like splitlines is the better/safer fix - but figured bumping the\n> minimum\n> Python version was probably part of a wider discussion. If it's something\n> people\n> are comfortable with - I'd be happy to rework the fix to use splitlines.\n> Luke - do you have any thoughts on this?\n\nI agree that we have to stop supporting 2.6 at some point (and start\nsupporting 3.x, but that's another world of hurt). But does 2.6 really\nnot have splitlines?\n\nIf you send a patch with splitlines I can try it out (although I guess\nit could be broken under Windows).\n\nLuke\n"},{"id":"309928","messageId":"001901d274b9$eaad3f80$c007be80$@vanburgh.me","threadId":"44929","inReplyTo":"CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com","subject":"RE: [PATCH] git-p4: Fix git-p4.mapUser on Windows","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":null,"receivedAt":"2017-01-22T14:15:42Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"> On 22 Jan 2017, at 13:05, Luke Diamand <luke@diamand.org>\n> \n> I'm confused....see below.\n\nThat now makes two of us! \nI think I've figured out where I messed up, see below.\n\n> \n> On 21 January 2017 at 15:21, George Vanburgh <george@vanburgh.me>\n> wrote:\n> >> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>\n> >> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>\n> >> wrote:\n> >> >\n> >> > From: George Vanburgh <gvanburgh@bloomberg.net>\n> >> >\n> >> > When running git-p4 on Windows, with multiple git-p4.mapUser\n> >> > entries in git config - no user mappings are applied to the\n> >> > generated\n> > repository.\n> >> >\n> >> > Reproduction Steps:\n> >> >\n> >> > 1. Add multiple git-p4.mapUser entries to git config on a Windows\n> >> >   machine\n> >> > 2. Attempt to clone a p4 repository\n> >> >\n> >> > None of the user mappings will be applied.\n> >> >\n> >> > This issue is caused by the fact that gitConfigList, uses\n> >> > split(os.linesep) to convert the output of git config --get-all\n> >> > into a list.\n> >> >\n> >> > On Windows, os.linesep is equal to '\\r\\n' - however git.exe returns\n> >> > configuration with a line seperator of '\\n'. This leads to the list\n> >> > returned by gitConfigList containing only one element - which\n> >> > contains the full output of git config --get-all in string form.\n> >> > This causes problems for the code introduced to\n> >> > getUserMapFromPerforceServer in 10d08a1.\n> >> >\n> >> > This issue should be caught by the test introduced in 10d08a1, and\n> >> > would require running on Windows to reproduce. When running inside\n> >> > MinGW/Cygwin, however, os.linesep correctly returns '\\n', and\n> >> > everything works as expected.\n> >>\n> >> This surprises me. I would expect `\\r\\n` in a MinGW env...\n> >> Nevertheless, I wouldn't have caught that as I don't run the git-p4\n> >> tests\n> > on\n> >> Windows...\n> >\n> > It appears I was mistaken - the successful tests I ran were actually\n> > under the Ubuntu subsystem for Windows, which (obviously) passed.\n> >\n> > Just did a quick experiment:\n> >\n> > Git Bash (MinGW):\n> > georg@TEMPEST MINGW64 ~\n> > $ python -c \"import os\n> > print(repr(os.linesep))\"\n> > '\\r\\n'\n> >\n> > Powershell:\n> > PS C:\\Users\\georg> python -c \"import os\n> >>> print(repr(os.linesep))\"\n> > '\\r\\n'\n> >\n> > Ubuntu subsystem for Windows:\n> > george@TEMPEST:~$ python -c \"import os print(repr(os.linesep))\"\n> > '\\n'\n> >\n> > So this issue applies to git-p4 running under both PowerShell and MinGW.\n> >\n> >>\n> >>\n> >> > The simplest fix for this issue would be to convert the line split\n> >> > logic inside gitConfigList to use splitlines(), which splits on any\n> >> > standard line delimiter. However, this function was only introduced\n> >> > in Python 2.7, and would mean a bump in the minimum required\n> >> > version of Python required to run git-p4. The alternative fix,\n> >> > implemented here, is to use '\\n' as a delimiter, which git.exe\n> >> > appears to output consistently on Windows anyway.\n> \n> Have you tried a 2.6 Python with splitlines? I just tried this code and it seems\n> fine.\n> \n> > val = s.strip().splitlines()\n> > print(\"splitlines:\", val)\n> \n> Tried with 2.7 and 2.6, and bot print out an array of strings returned from\n> read_pipe().\n> \n> And 'grep -r splitlines' on the Python2.6 source has lots of hits.\n\nAh - it appears I was looking in the wrong part of the Python \ndocumentation - which is what lead me to think this. \nFrom the Python 2.4 documentation:\n\nhttps://docs.python.org/release/2.4/lib/string-methods.html\n\nSplitlines is a built-in method, not part of the string class.\n\n> george@TEMPEST:~# /home/george/.pyenv/versions/2.4.1/bin/python -c 'import string\n> print(\"test1\\ntest2\\r\\ntest3\".splitlines())'\n> ['test1', 'test2', 'test3']\n\n> \n> >>\n> >> Well, that also means if we ever use splitlines() then your fix below\n> > would\n> >> brake the code, right?\n> >>\n> >> Python 2.7 was released 7 years ago in 2010.\n> >\n> > Now I feel old...\n> \n> :-)\n> \n> >\n> >> Therefore, I would vote to\n> >> bump the minimum version. But that's just my opinion :-)\n> >\n> > I feel like splitlines is the better/safer fix - but figured bumping\n> > the minimum Python version was probably part of a wider discussion. If\n> > it's something people are comfortable with - I'd be happy to rework\n> > the fix to use splitlines.\n> > Luke - do you have any thoughts on this?\n> \n> I agree that we have to stop supporting 2.6 at some point (and start\n> supporting 3.x, but that's another world of hurt). But does 2.6 really not have\n> splitlines?\n\nThe minimum supported version is currently Python 2.4, no?\n\n> \n> If you send a patch with splitlines I can try it out (although I guess it could be\n> broken under Windows).\n\nGiven that there doesn't _actually_ seem to be an issue with using splitlines in\n2.4 (sorry for the confusion!) - I'll test out a patch using splitlines on \nWindows, and resubmit =]\n\n> \n> Luke\n\n"},{"id":"310143","messageId":"01020159d4ea6861-fce45d61-8be5-49b0-ab4e-d161b9429795-000000@eu-west-1.amazonses.com","threadId":"44929","inReplyTo":"01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] git-p4: Fix git-p4.mapUser on Windows","fromName":"George Vanburgh","fromEmail":"george@vanburgh.me","sentAt":"2017-01-25T09:17:29Z","receivedAt":"2017-01-25T09:17:37Z","isPatch":true,"sender":{"key":"george@vanburgh.me","avatar":"https://gravatar.com/avatar/575b39450d443981d8b1d1ca4b380c5af51b54adb6dbe3f737cb495e15b2b748?d=mp&s=160"},"body":"From: George Vanburgh <gvanburgh@bloomberg.net>\n\nWhen running git-p4 on Windows, with multiple git-p4.mapUser entries in\ngit config - no user mappings are applied to the generated repository.\n\nReproduction Steps:\n\n1. Add multiple git-p4.mapUser entries to git config on a Windows\n   machine\n2. Attempt to clone a p4 repository\n\nNone of the user mappings will be applied.\n\nThis issue is actually caused by gitConfigList, using\nsplit(os.linesep) to convert the output of git config --get-all into\na list. On Windows, os.linesep is equal to '\\r\\n' - however git.exe\nreturns configuration with a line seperator of '\\n'. This leads to\nthe list returned by gitConfigList containing only one element - which\ncontains the full output of git config --get-all in string form, which\ncauses problems for the code introduced to getUserMapFromPerforceServer\nin 10d08a1.\n\nThis issue should be caught by the test introduced in 10d08a1, however\nwould require running on Windows to reproduce.\n\nUsing splitlines solves this issue, by splitting config on all\ntypical delimiters ('\\n', '\\r\\n' etc.)\n\nSigned-off-by: George Vanburgh <gvanburgh@bloomberg.net>\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 f427bf6..b66f68b 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -656,7 +656,7 @@ def gitConfigInt(key):\n def gitConfigList(key):\n     if not _gitConfig.has_key(key):\n         s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n-        _gitConfig[key] = s.strip().split(os.linesep)\n+        _gitConfig[key] = s.strip().splitlines()\n         if _gitConfig[key] == ['']:\n             _gitConfig[key] = []\n     return _gitConfig[key]\n\n--\nhttps://github.com/git/git/pull/319\n"},{"id":"310414","messageId":"xmqqa8ac1k7o.fsf@gitster.mtv.corp.google.com","threadId":"44929","inReplyTo":"01020159d4ea6861-fce45d61-8be5-49b0-ab4e-d161b9429795-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-27T17:33:31Z","receivedAt":"2017-01-27T17:35:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"George Vanburgh <george@vanburgh.me> writes:\n\n> From: George Vanburgh <gvanburgh@bloomberg.net>\n>\n> When running git-p4 on Windows, with multiple git-p4.mapUser entries in\n> git config - no user mappings are applied to the generated repository.\n> ...\n> Using splitlines solves this issue, by splitting config on all\n> typical delimiters ('\\n', '\\r\\n' etc.)\n\nLuke, Lars, this version seems to be in line with the conclusion of\nyour earlier reviews, e.g.\n\n<CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>\n\nEven though it looks OK to my eyes, I'll wait for Acks or further\nrefinement suggestions from either of you two before acting on this\npatch.\n\nThanks.\n\n> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n> ---\n>  git-p4.py | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index f427bf6..b66f68b 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -656,7 +656,7 @@ def gitConfigInt(key):\n>  def gitConfigList(key):\n>      if not _gitConfig.has_key(key):\n>          s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n> -        _gitConfig[key] = s.strip().split(os.linesep)\n> +        _gitConfig[key] = s.strip().splitlines()\n>          if _gitConfig[key] == ['']:\n>              _gitConfig[key] = []\n>      return _gitConfig[key]\n>\n> --\n> https://github.com/git/git/pull/319\n"},{"id":"310509","messageId":"CAE5ih7-qug9n-Df2gA27iTjSQo67tAnPhTJWQhyvR_PP9h3rcg@mail.gmail.com","threadId":"44929","inReplyTo":"xmqqa8ac1k7o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2017-01-29T17:10:04Z","receivedAt":"2017-01-29T17:19:06Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 27 January 2017 at 17:33, Junio C Hamano <gitster@pobox.com> wrote:\n> George Vanburgh <george@vanburgh.me> writes:\n>\n>> From: George Vanburgh <gvanburgh@bloomberg.net>\n>>\n>> When running git-p4 on Windows, with multiple git-p4.mapUser entries in\n>> git config - no user mappings are applied to the generated repository.\n>> ...\n>> Using splitlines solves this issue, by splitting config on all\n>> typical delimiters ('\\n', '\\r\\n' etc.)\n>\n> Luke, Lars, this version seems to be in line with the conclusion of\n> your earlier reviews, e.g.\n>\n> <CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>\n>\n> Even though it looks OK to my eyes, I'll wait for Acks or further\n> refinement suggestions from either of you two before acting on this\n> patch.\n\nIt looks good to me. The tests all pass, and the change looks correct.\n\nAck.\n\nLuke\n\n\n>\n> Thanks.\n>\n>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>\n>> ---\n>>  git-p4.py | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/git-p4.py b/git-p4.py\n>> index f427bf6..b66f68b 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -656,7 +656,7 @@ def gitConfigInt(key):\n>>  def gitConfigList(key):\n>>      if not _gitConfig.has_key(key):\n>>          s = read_pipe([\"git\", \"config\", \"--get-all\", key], ignore_error=True)\n>> -        _gitConfig[key] = s.strip().split(os.linesep)\n>> +        _gitConfig[key] = s.strip().splitlines()\n>>          if _gitConfig[key] == ['']:\n>>              _gitConfig[key] = []\n>>      return _gitConfig[key]\n>>\n>> --\n>> https://github.com/git/git/pull/319\n"},{"id":"310537","messageId":"xmqqinowwk6q.fsf@gitster.mtv.corp.google.com","threadId":"44929","inReplyTo":"CAE5ih7-qug9n-Df2gA27iTjSQo67tAnPhTJWQhyvR_PP9h3rcg@mail.gmail.com","subject":"Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-30T17:07:25Z","receivedAt":"2017-01-30T17:07:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand <luke@diamand.org> writes:\n\n> On 27 January 2017 at 17:33, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Luke, Lars, this version seems to be in line with the conclusion of\n>> your earlier reviews, e.g.\n>>\n>> <CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>\n>>\n>> Even though it looks OK to my eyes, I'll wait for Acks or further\n>> refinement suggestions from either of you two before acting on this\n>> patch.\n>\n> It looks good to me. The tests all pass, and the change looks correct.\n\nThanks, queued.\n"}]}