Re: git-p4: problem with commit 97a21ca50ef8

From: Pete Wyckoff <pw@padd.com>
Date: Nov 1, 2011, 02:08 UTC
Message-ID: <20111101020841.GA8116@arf.padd.com>
In-Reply-To: <CAOk9v+-==GwDQaZ=4BW1QfEF7+5SfhNF409Xom0bHdT_qKaiFA@mail.gmail.com>

michaelwookey@gmail.com wrote on Tue, 01 Nov 2011 10:11 +1100:

Show 31 quoted lines

> [ 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