Re: [PATCH v1] git-p4: fix git-p4.pathEncoding for removed files

[Junio C Hamano <gitster@pobox.com>]

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> In a9e38359e3 we taught git-p4 a way to re-encode path names from what
> was used in Perforce to UTF-8. This path re-encoding worked properly for
> "added" paths. "Removed" paths were not re-encoded and therefore
> different from the "added" paths. Consequently, these files were not
> removed in a git-p4 cloned Git repository because the path names did not
> match.
>
> Fix this by moving the re-encoding to a place that affects "added" and
> "removed" paths. Add a test to demonstrate the issue.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---

Thanks.

The above description makes me wonder what happens to "modified"
paths, but presumably they are handled in a separate codepath?  Or
does this also cover not just "removed" but also paths with any
change?

Luke, does this look good?

> Notes:
>     Base Commit: d1271bddd4 (v2.11.0)
>     Diff on Web: https://github.com/git/git/compare/d1271bddd4...larsxschneider:05a82caa69
>     Checkout:    git fetch https://github.com/larsxschneider/git git-p4/fix-path-encoding-v1 && git checkout 05a82caa69
>
>  git-p4.py                       | 19 +++++++++----------
>  t/t9822-git-p4-path-encoding.sh | 16 ++++++++++++++++
>  2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index fd5ca52462..8f311cb4e8 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2366,6 +2366,15 @@ class P4Sync(Command, P4UserMap):
>                      break
>  
>          path = wildcard_decode(path)
> +        try:
> +            path.decode('ascii')
> +        except:
> +            encoding = 'utf8'
> +            if gitConfig('git-p4.pathEncoding'):
> +                encoding = gitConfig('git-p4.pathEncoding')
> +            path = path.decode(encoding, 'replace').encode('utf8', 'replace')
> +            if self.verbose:
> +                print 'Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, path)
>          return path
>  
>      def splitFilesIntoBranches(self, commit):
> @@ -2495,16 +2504,6 @@ class P4Sync(Command, P4UserMap):
>              text = regexp.sub(r'$\1$', text)
>              contents = [ text ]
>  
> -        try:
> -            relPath.decode('ascii')
> -        except:
> -            encoding = 'utf8'
> -            if gitConfig('git-p4.pathEncoding'):
> -                encoding = gitConfig('git-p4.pathEncoding')
> -            relPath = relPath.decode(encoding, 'replace').encode('utf8', 'replace')
> -            if self.verbose:
> -                print 'Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, relPath)
> -
>          if self.largeFileSystem:
>              (git_mode, contents) = self.largeFileSystem.processContent(git_mode, relPath, contents)
>  
> diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> index 7b83e696a9..c78477c19b 100755
> --- a/t/t9822-git-p4-path-encoding.sh
> +++ b/t/t9822-git-p4-path-encoding.sh
> @@ -51,6 +51,22 @@ test_expect_success 'Clone repo containing iso8859-1 encoded paths with git-p4.p
>  	)
>  '
>  
> +test_expect_success 'Delete iso8859-1 encoded paths and clone' '
> +	(
> +		cd "$cli" &&
> +		ISO8859="$(printf "$ISO8859_ESCAPED")" &&
> +		p4 delete "$ISO8859" &&
> +		p4 submit -d "remove file"
> +	) &&
> +	git p4 clone --destination="$git" //depot@all &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		git -c core.quotepath=false ls-files >actual &&
> +		test_must_be_empty actual
> +	)
> +'
> +
>  test_expect_success 'kill p4d' '
>  	kill_p4d
>  '