[PATCH v6] git-p4: add config git-p4.pathEncoding

[PATCH v6] git-p4: add config git-p4.pathEncoding

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

Diff to v5:
* use "test_config" (Thanks Remi! I am still learning all the tools...)
* removed whitespaces (Thanks Luke! I added this to my "generate patch" script. Won't happen again :-)
* added ACK from Luke (I interpreted "Looks good to me" that way. I hope this is OK.)

Thanks,
Lars

Lars Schneider (1):
  git-p4: add config git-p4.pathEncoding

 Documentation/git-p4.txt        |  7 +++++
 git-p4.py                       | 11 ++++++++
 t/t9822-git-p4-path-encoding.sh | 60 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100755 t/t9822-git-p4-path-encoding.sh

--
1.9.5 (Apple Git-50.3)

[PATCH v6] git-p4: add config git-p4.pathEncoding

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

Perforce keeps the encoding of a path as given by the originating OS.
Git expects paths encoded as UTF-8. Add a config to tell git-p4 what
encoding Perforce had used for the paths. This encoding is used to
transcode the paths to UTF-8. As an example, Perforce on Windows often
uses “cp1252” to encode path names.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 Documentation/git-p4.txt        |  7 +++++
 git-p4.py                       | 11 ++++++++
 t/t9822-git-p4-path-encoding.sh | 60 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100755 t/t9822-git-p4-path-encoding.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..12a57d4 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,13 @@ git-p4.useClientSpec::
 	option '--use-client-spec'.  See the "CLIENT SPEC" section above.
 	This variable is a boolean, not the name of a p4 client.
 
+git-p4.pathEncoding::
+	Perforce keeps the encoding of a path as given by the originating OS.
+	Git expects paths encoded as UTF-8. Use this config to tell git-p4
+	what encoding Perforce had used for the paths. This encoding is used
+	to transcode the paths to UTF-8. As an example, Perforce on Windows
+	often uses “cp1252” to encode path names.
+
 Submit variables
 ~~~~~~~~~~~~~~~~
 git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 073f87b..b1ad86d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2213,6 +2213,17 @@ class P4Sync(Command, P4UserMap):
             text = regexp.sub(r'$\1$', text)
             contents = [ text ]
 
+        if gitConfig("git-p4.pathEncoding"):
+            relPath = relPath.decode(gitConfig("git-p4.pathEncoding")).encode('utf8', 'replace')
+        elif self.verbose:
+            try:
+                relPath.decode('ascii')
+            except:
+                print (
+                    "Path with Non-ASCII characters detected and no path encoding defined. "
+                    "Please check the encoding: %s" % relPath
+                )
+
         self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
 
         # total length...
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
new file mode 100755
index 0000000..e507ad7
--- /dev/null
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='Clone repositories with non ASCII paths'
+
+. ./lib-git-p4.sh
+
+UTF8_ESCAPED="a-\303\244_o-\303\266_u-\303\274.txt"
+ISO8859_ESCAPED="a-\344_o-\366_u-\374.txt"
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'Create a repo containing iso8859-1 encoded paths' '
+	(
+		cd "$cli" &&
+		ISO8859="$(printf "$ISO8859_ESCAPED")" &&
+		echo content123 >"$ISO8859" &&
+		p4 add "$ISO8859" &&
+		p4 submit -d "test commit"
+	)
+'
+
+test_expect_success 'Clone repo containing iso8859-1 encoded paths without git-p4.pathEncoding' '
+	git p4 clone --destination="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		UTF8="$(printf "$UTF8_ESCAPED")" &&
+		echo $UTF8 >expect &&
+		git -c core.quotepath=false ls-files >actual &&
+		test_must_fail test_cmp expect actual
+	)
+'
+
+test_expect_success 'Clone repo containing iso8859-1 encoded paths with git-p4.pathEncoding' '
+
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git init . &&
+		test_config git-p4.pathEncoding iso8859-1 &&
+		git p4 clone --use-client-spec --destination="$git" //depot &&
+		UTF8="$(printf "$UTF8_ESCAPED")" &&
+		echo $UTF8 >expect &&
+		git -c core.quotepath=false ls-files >actual &&
+		test_cmp expect actual &&
+		cat >expect <<-\EOF &&
+		content123
+		EOF
+		cat $UTF8 >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.9.5 (Apple Git-50.3)

Re: [PATCH v6] git-p4: add config git-p4.pathEncoding

[Junio C Hamano]

larsxschneider@gmail.com writes:

> +test_expect_success 'Clone repo containing iso8859-1 encoded paths without git-p4.pathEncoding' '
> +	git p4 clone --destination="$git" //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		UTF8="$(printf "$UTF8_ESCAPED")" &&
> +		echo $UTF8 >expect &&
> +		git -c core.quotepath=false ls-files >actual &&
> +		test_must_fail test_cmp expect actual

I am not sure what this test wants to do.  It is not inconceivable
that future versions of "git p4 clone" becomes more intelligent to
detect the need for git-p4.pathEncoding and set it, so the only
effect to insist the comparison fails is to block future advance in
that direction.

Besides, "test_must_fail test_cmp" looks like a strange thing to
say.  "! test_cmp expect actual" perhaps.

Even better, expect that "expect" and "actual" becomes the same, but
mark the test itself to expect failure, to say "it ought to work
this way in the ideal world, but we know the system currently does
not pass this test".

I'm tempted to suggest squashing the following in.  Thoughts?


 t/t9822-git-p4-path-encoding.sh | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index e507ad7..2d652d89 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -21,15 +21,15 @@ test_expect_success 'Create a repo containing iso8859-1 encoded paths' '
 	)
 '
 
-test_expect_success 'Clone repo containing iso8859-1 encoded paths without git-p4.pathEncoding' '
+test_expect_failure 'Clone auto-detects depot with iso8859-1 paths' '
 	git p4 clone --destination="$git" //depot &&
 	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
 		UTF8="$(printf "$UTF8_ESCAPED")" &&
-		echo $UTF8 >expect &&
+		echo "$UTF8" >expect &&
 		git -c core.quotepath=false ls-files >actual &&
-		test_must_fail test_cmp expect actual
+		test_cmp expect actual
 	)
 '
 
@@ -39,16 +39,15 @@ test_expect_success 'Clone repo containing iso8859-1 encoded paths with git-p4.p
 	(
 		cd "$git" &&
 		git init . &&
-		test_config git-p4.pathEncoding iso8859-1 &&
+		git config git-p4.pathEncoding iso8859-1 &&
 		git p4 clone --use-client-spec --destination="$git" //depot &&
 		UTF8="$(printf "$UTF8_ESCAPED")" &&
-		echo $UTF8 >expect &&
+		echo "$UTF8" >expect &&
 		git -c core.quotepath=false ls-files >actual &&
 		test_cmp expect actual &&
-		cat >expect <<-\EOF &&
-		content123
-		EOF
-		cat $UTF8 >actual &&
+
+		echo content123 >expect &&
+		cat "$UTF8" >actual &&
 		test_cmp expect actual
 	)
 '

Re: [PATCH v6] git-p4: add config git-p4.pathEncoding

[Lars Schneider]

On 03 Sep 2015, at 19:03, Junio C Hamano <gitster@pobox.com> wrote:

> larsxschneider@gmail.com writes:
> 
>> +test_expect_success 'Clone repo containing iso8859-1 encoded paths without git-p4.pathEncoding' '
>> +	git p4 clone --destination="$git" //depot &&
>> +	test_when_finished cleanup_git &&
>> +	(
>> +		cd "$git" &&
>> +		UTF8="$(printf "$UTF8_ESCAPED")" &&
>> +		echo $UTF8 >expect &&
>> +		git -c core.quotepath=false ls-files >actual &&
>> +		test_must_fail test_cmp expect actual
> 
> I am not sure what this test wants to do.  It is not inconceivable
> that future versions of "git p4 clone" becomes more intelligent to
> detect the need for git-p4.pathEncoding and set it, so the only
> effect to insist the comparison fails is to block future advance in
> that direction.
> 
> Besides, "test_must_fail test_cmp" looks like a strange thing to
> say.  "! test_cmp expect actual" perhaps.
> 
> Even better, expect that "expect" and "actual" becomes the same, but
> mark the test itself to expect failure, to say "it ought to work
> this way in the ideal world, but we know the system currently does
> not pass this test".
> 
> I'm tempted to suggest squashing the following in.  Thoughts?
OK. The diff looks good to me. For some reason I can’t apply the patch though. git patch gives me "fatal: corrupt patch at line 10”. Any idea? (I might do something stupid because I am not used to patches…)

Thanks,
Lars

> 
> 
> t/t9822-git-p4-path-encoding.sh | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> index e507ad7..2d652d89 100755
> --- a/t/t9822-git-p4-path-encoding.sh
> +++ b/t/t9822-git-p4-path-encoding.sh
> @@ -21,15 +21,15 @@ test_expect_success 'Create a repo containing iso8859-1 encoded paths' '
> 	)
> '
> 
> -test_expect_success 'Clone repo containing iso8859-1 encoded paths without git-p4.pathEncoding' '
> +test_expect_failure 'Clone auto-detects depot with iso8859-1 paths' '
> 	git p4 clone --destination="$git" //depot &&
> 	test_when_finished cleanup_git &&
> 	(
> 		cd "$git" &&
> 		UTF8="$(printf "$UTF8_ESCAPED")" &&
> -		echo $UTF8 >expect &&
> +		echo "$UTF8" >expect &&
> 		git -c core.quotepath=false ls-files >actual &&
> -		test_must_fail test_cmp expect actual
> +		test_cmp expect actual
> 	)
> '
> 
> @@ -39,16 +39,15 @@ test_expect_success 'Clone repo containing iso8859-1 encoded paths with git-p4.p
> 	(
> 		cd "$git" &&
> 		git init . &&
> -		test_config git-p4.pathEncoding iso8859-1 &&
> +		git config git-p4.pathEncoding iso8859-1 &&
> 		git p4 clone --use-client-spec --destination="$git" //depot &&
> 		UTF8="$(printf "$UTF8_ESCAPED")" &&
> -		echo $UTF8 >expect &&
> +		echo "$UTF8" >expect &&
> 		git -c core.quotepath=false ls-files >actual &&
> 		test_cmp expect actual &&
> -		cat >expect <<-\EOF &&
> -		content123
> -		EOF
> -		cat $UTF8 >actual &&
> +
> +		echo content123 >expect &&
> +		cat "$UTF8" >actual &&
> 		test_cmp expect actual
> 	)
> '

Re: [PATCH v6] git-p4: add config git-p4.pathEncoding

[Junio C Hamano]

Lars Schneider <larsxschneider@gmail.com> writes:

> On 03 Sep 2015, at 19:03, Junio C Hamano <gitster@pobox.com> wrote:
>
>> I'm tempted to suggest squashing the following in.  Thoughts?
>
> OK. The diff looks good to me. For some reason I can’t apply the
> patch though. git patch gives me "fatal: corrupt patch at line
> 10”. Any idea? (I might do something stupid because I am not used to
> patches…)

Sorry, but no idea.  I just saved the message I see on the list to a
file and did "git am" myself and didn't find anything.

Line 9 of the patch is a removal of your overlong test_expect_success
line, so if that were somehow line-wrapped by something between the
list and your "git apply" (e.g. with your MUA), it is very probable
that "git apply" would see there is something funny on line 10, but
line 5 (a hunk header "@@ -21,15 ...") is also rather long, so...