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

2017-01-30 Thread Junio C Hamano
Luke Diamand  writes:

> On 27 January 2017 at 17:33, Junio C Hamano  wrote:
>>
>> Luke, Lars, this version seems to be in line with the conclusion of
>> your earlier reviews, e.g.
>>
>> 
>>
>> Even though it looks OK to my eyes, I'll wait for Acks or further
>> refinement suggestions from either of you two before acting on this
>> patch.
>
> It looks good to me. The tests all pass, and the change looks correct.

Thanks, queued.


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

2017-01-29 Thread Luke Diamand
On 27 January 2017 at 17:33, Junio C Hamano  wrote:
> George Vanburgh  writes:
>
>> From: George Vanburgh 
>>
>> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
>> git config - no user mappings are applied to the generated repository.
>> ...
>> Using splitlines solves this issue, by splitting config on all
>> typical delimiters ('\n', '\r\n' etc.)
>
> Luke, Lars, this version seems to be in line with the conclusion of
> your earlier reviews, e.g.
>
> 
>
> Even though it looks OK to my eyes, I'll wait for Acks or further
> refinement suggestions from either of you two before acting on this
> patch.

It looks good to me. The tests all pass, and the change looks correct.

Ack.

Luke


>
> Thanks.
>
>> Signed-off-by: George Vanburgh 
>> ---
>>  git-p4.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index f427bf6..b66f68b 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)
>> -_gitConfig[key] = s.strip().split(os.linesep)
>> +_gitConfig[key] = s.strip().splitlines()
>>  if _gitConfig[key] == ['']:
>>  _gitConfig[key] = []
>>  return _gitConfig[key]
>>
>> --
>> https://github.com/git/git/pull/319


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

2017-01-27 Thread Junio C Hamano
George Vanburgh  writes:

> From: George Vanburgh 
>
> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
> git config - no user mappings are applied to the generated repository.
> ...
> Using splitlines solves this issue, by splitting config on all
> typical delimiters ('\n', '\r\n' etc.)

Luke, Lars, this version seems to be in line with the conclusion of
your earlier reviews, e.g.



Even though it looks OK to my eyes, I'll wait for Acks or further
refinement suggestions from either of you two before acting on this
patch.

Thanks.

> Signed-off-by: George Vanburgh 
> ---
>  git-p4.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f427bf6..b66f68b 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)
> -_gitConfig[key] = s.strip().split(os.linesep)
> +_gitConfig[key] = s.strip().splitlines()
>  if _gitConfig[key] == ['']:
>  _gitConfig[key] = []
>  return _gitConfig[key]
>
> --
> https://github.com/git/git/pull/319


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

2017-01-25 Thread George Vanburgh
From: George Vanburgh 

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

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 actually caused by gitConfigList, using
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, which
causes problems for the code introduced to getUserMapFromPerforceServer
in 10d08a1.

This issue should be caught by the test introduced in 10d08a1, however
would require running on Windows to reproduce.

Using splitlines solves this issue, by splitting config on all
typical delimiters ('\n', '\r\n' etc.)

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

diff --git a/git-p4.py b/git-p4.py
index f427bf6..b66f68b 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)
-_gitConfig[key] = s.strip().split(os.linesep)
+_gitConfig[key] = s.strip().splitlines()
 if _gitConfig[key] == ['']:
 _gitConfig[key] = []
 return _gitConfig[key]

--
https://github.com/git/git/pull/319