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

I discovered what appears to be a bug with the handling of utf16 type files 
when cloning from P4.  It appears that it always fetches the content of the 
latest version rather than getting the revision specified in the changelist 
being processed.  This patch is an attempt to resolve that by formatting
the print command using the revision number as is done in streamP4Files().

Daniel Bingham (1):
  git-p4: Fetch the proper revision of utf16 files

 git-p4.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.3.5

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.

Signed-off-by: Daniel Bingham <redacted>
---
 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

On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham [off-list ref] wrote:

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.)

*note* I tried to send this to the list earlier but forgot to set the
mode to plain-text so it got rejected.  Resending.  Apologies for any
duplicates.

I think the context of the patch doesn't make this clear, but if you
look at git-p4.py in this spot you'll see that this is in a block that
begins:

if type_base == "utf16":

Where "type_base" is extracted, by the script, from the filetype
provided by p4 (i.e. metadata provided by p4). In the particular
scenario I encountered, P4 said that the file was of type xutf16 from
which the split_p4_type() method extracts "utf16" as the type_base.

The essence of the issue here has nothing to do with UTF16, its just
coincidental that it happens for files of that type.  The problem is
that the script wasn't requesting the proper revision from P4 and thus
would always get the *latest* revision.  This corrects that by having
the script request the appropriate revision from P4.

Daniel

On Fri, Apr 3, 2015 at 2:46 PM, Eric Sunshine [off-list ref] wrote:

On Fri, Apr 3, 2015 at 5:13 PM, Daniel Bingham [off-list ref] wrote:

quoted

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.)

quoted

Signed-off-by: Daniel Bingham <redacted>
---
 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