Re: git-p4: problem with commit 97a21ca50ef8

[Pete Wyckoff <pw@xxxxxxxx>]

michaelwookey@xxxxxxxxx wrote on Tue, 01 Nov 2011 10:11 +1100:
> [ please CC me as I am not subscribed to the list ]
> 
> Hi,
> 
> Commit 97a21ca50ef893a171a50c863fe21a924935fd2a "git-p4: stop ignoring
> apple filetype" isn't correct. Without knowing too much about how
> git-p4 works, it appears that the "apple" filetype includes the
> resource fork, and the "p4 print" that is used to obtain the content
> from the perforce server doesn't take this into account, or maybe some
> post processing of the file needs to be done to include the data, but
> not the resource fork, before inclusion into the git repo.
> 
> With the above commit, a binary blob that literally contains the
> resource fork and data is included within the git repo. Of course,
> without the above commit, the intended file was never included in the
> git repo at all. Perhaps the resource fork issue was a known problem
> by the original git-p4 author.
> 
> A sample file that that demonstrates what the above commit produces is
> here (use curl/wget):
> 
>   http://dl.dropbox.com/u/1006983/sample_image_fail.png
> 
> This is literally a binary blob with about 110 KiB of resource fork
> plus the PNG data. The same image, minus about 110 KiB of resource
> fork is here:
> 
>   http://dl.dropbox.com/u/1006983/sample_image_correct.png
> 
> I'm happy to test patches as we have a perforce repository with files
> of the "apple" filetype.

Thanks so much for taking the time to find this and to narrow it
down.

I found icnsutils that shows the fail.png file has a bunch of
icons glued onto the front of the correct image file.

We can certainly revert this commit, but first I'd like to
understand what the right behavior should be.

I managed to include an apple filetype in a repo from a linux box
by hacking:

    $ cp /tmp/sample_image_fail.png fail.png
    $ p4 add -t apple fail.png 
    $ p4 submit -dfail-apple
    Submitting change 2.
    Locking 1 files ...
    add //depot/fail.png#1
    Unable to read AppleDouble Header.
    open for read: /home/pw/src/perforce/cli/%fail.png: No such
    file or directory
    Submit aborted -- fix problems then use 'p4 submit -c 2'.
    Some file(s) could not be transferred from client.

Hrm.  Fake it by copying your example apple file to what it asks
for:

    $ cp fail.png %fail.png
    $ p4 submit -c 2
    Submitting change 2.
    add //depot/fail.png#1
    Change 2 submitted.

(But later p4 sync -f destroy both files.)

Voila:

    $ p4 fstat //depot/fail.png
    ... depotFile //depot/fail.png
    ... clientFile /home/pw/src/perforce/cli/fail.png
    ... isMapped 
    ... headAction add
    ... headType apple
    ... headTime 1320111844
    ... headRev 1
    ... headChange 2
    ... headModTime 1320111842
    ... haveRev 1

And git-p4 checks it out intact:

    $ git p4 clone //depot
    [..]
    $ sha1sum depot/fail.png /tmp/sample_image_fail.png 
    93d175ad906147f4d75296bd2adb6d706f798c64  depot/fail.png
    93d175ad906147f4d75296bd2adb6d706f798c64  /tmp/sample_image_fail.png

Which is what I thought an apple-filetype user would want.
Reverting the patch causes _no_ file to be created.  Is
this better?  Maybe the single-blob file, since it no longer
appears in AppleDouble format, is just as useless as no file?

The other option is to use "p4 print" without the -G, which
seems to retrieve only the data fork, and leave that in the repo.
Of course, if you change it, and submit it, it makes a mess.

Would it be good if git-p4 understood how to identify and create
AppleDouble files on Mac?  If yes, eventually, we can revert the
commit and explain how this feature doesn't quite work yet.
Even if no, it seems like we should revert and complain that
this apple support is broken.

		-- Pete
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html