Re: [PATCH] git-p4: Fetch the proper revision of utf16 files

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 3, 2015, 21:46 UTC
Message-ID: <CAPig+cT59B-ccvbfyPvVt_1dTO7jFPn7YQdhu81WSz_1WFM2GA@mail.gmail.com>
In-Reply-To: <1428095627-8772-2-git-send-email-git@dbingham.com>

On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham <daniel@dbingham.com> wrote:

Show 8 quoted lines

> git-p4 always fetches the latest revision of UTF16
> files from P4 rather than the revision at the commit being sync'd.
>
> The print command should, instead, specify the revision number from the
> commit in question using the file#revision syntax.
>
> The file#revision syntax is preferable over file@changelist for
> consistency with how streamP4Files formats the fileArgs list.

As a non-Perforce reader trying to understand this patch, there are a couple issues which are unclear or inadequately explained. Perhaps you could provide a bit more detail or cite relevant sources.

First, does "UTF16 file" refer to the content or the filename?

Second, I may be entirely missing it, but the commit message doesn't seem to explain why this impacts only "UTF16 files", and why this solution is the appropriate fix.

If the answer to the first question is that the filename is UTF-16, then would an alternate fix be to convert the value of file['depotFile'] to have the same encoding as the "print -q -o - ..." command-line? (Again, please excuse my Perforce-ignorance if I'm completely off the mark.)

Show 21 quoted lines

> Signed-off-by: Daniel Bingham <git@dbingham.com>
> ---
>  git-p4.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index ff132b2..156f3a4 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2101,7 +2101,8 @@ class P4Sync(Command, P4UserMap):
>              # them back too.  This is not needed to the cygwin windows version,
>              # just the native "NT" type.
>              #
> -            text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
> +            ufile = "%s#%s" % (file['depotFile'], file['rev'])
> +            text = p4_read_pipe(['print', '-q', '-o', '-', ufile])
>              if p4_version_string().find("/NT") >= 0:
>                  text = text.replace("\r\n", "\n")
>              contents = [ text ]
> --
> 2.3.5