{"thread":{"id":"28813","subject":"git-p4: problem with commit 97a21ca50ef8","startedAt":"2011-10-31T23:11:02Z","lastAt":"2011-11-07T04:33:40Z","messageCount":10,"participants":["Michael Wookey","Pete Wyckoff","Vitor Antunes","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"178601","messageId":"CAOk9v+-==GwDQaZ=4BW1QfEF7+5SfhNF409Xom0bHdT_qKaiFA@mail.gmail.com","threadId":"28813","inReplyTo":null,"subject":"git-p4: problem with commit 97a21ca50ef8","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2011-10-31T23:11:02Z","receivedAt":"2011-10-31T23:11:02Z","isPatch":false,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"[ please CC me as I am not subscribed to the list ]\n\nHi,\n\nCommit 97a21ca50ef893a171a50c863fe21a924935fd2a \"git-p4: stop ignoring\napple filetype\" isn't correct. Without knowing too much about how\ngit-p4 works, it appears that the \"apple\" filetype includes the\nresource fork, and the \"p4 print\" that is used to obtain the content\nfrom the perforce server doesn't take this into account, or maybe some\npost processing of the file needs to be done to include the data, but\nnot the resource fork, before inclusion into the git repo.\n\nWith the above commit, a binary blob that literally contains the\nresource fork and data is included within the git repo. Of course,\nwithout the above commit, the intended file was never included in the\ngit repo at all. Perhaps the resource fork issue was a known problem\nby the original git-p4 author.\n\nA sample file that that demonstrates what the above commit produces is\nhere (use curl/wget):\n\n  http://dl.dropbox.com/u/1006983/sample_image_fail.png\n\nThis is literally a binary blob with about 110 KiB of resource fork\nplus the PNG data. The same image, minus about 110 KiB of resource\nfork is here:\n\n  http://dl.dropbox.com/u/1006983/sample_image_correct.png\n\nI'm happy to test patches as we have a perforce repository with files\nof the \"apple\" filetype.\n\nThanks\n"},{"id":"178607","messageId":"20111101020841.GA8116@arf.padd.com","threadId":"28813","inReplyTo":"CAOk9v+-==GwDQaZ=4BW1QfEF7+5SfhNF409Xom0bHdT_qKaiFA@mail.gmail.com","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-11-01T02:08:41Z","receivedAt":"2011-11-01T02:08:41Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"michaelwookey@gmail.com wrote on Tue, 01 Nov 2011 10:11 +1100:\n> [ please CC me as I am not subscribed to the list ]\n> \n> Hi,\n> \n> Commit 97a21ca50ef893a171a50c863fe21a924935fd2a \"git-p4: stop ignoring\n> apple filetype\" isn't correct. Without knowing too much about how\n> git-p4 works, it appears that the \"apple\" filetype includes the\n> resource fork, and the \"p4 print\" that is used to obtain the content\n> from the perforce server doesn't take this into account, or maybe some\n> post processing of the file needs to be done to include the data, but\n> not the resource fork, before inclusion into the git repo.\n> \n> With the above commit, a binary blob that literally contains the\n> resource fork and data is included within the git repo. Of course,\n> without the above commit, the intended file was never included in the\n> git repo at all. Perhaps the resource fork issue was a known problem\n> by the original git-p4 author.\n> \n> A sample file that that demonstrates what the above commit produces is\n> here (use curl/wget):\n> \n>   http://dl.dropbox.com/u/1006983/sample_image_fail.png\n> \n> This is literally a binary blob with about 110 KiB of resource fork\n> plus the PNG data. The same image, minus about 110 KiB of resource\n> fork is here:\n> \n>   http://dl.dropbox.com/u/1006983/sample_image_correct.png\n> \n> I'm happy to test patches as we have a perforce repository with files\n> of the \"apple\" filetype.\n\nThanks so much for taking the time to find this and to narrow it\ndown.\n\nI found icnsutils that shows the fail.png file has a bunch of\nicons glued onto the front of the correct image file.\n\nWe can certainly revert this commit, but first I'd like to\nunderstand what the right behavior should be.\n\nI managed to include an apple filetype in a repo from a linux box\nby hacking:\n\n    $ cp /tmp/sample_image_fail.png fail.png\n    $ p4 add -t apple fail.png \n    $ p4 submit -dfail-apple\n    Submitting change 2.\n    Locking 1 files ...\n    add //depot/fail.png#1\n    Unable to read AppleDouble Header.\n    open for read: /home/pw/src/perforce/cli/%fail.png: No such\n    file or directory\n    Submit aborted -- fix problems then use 'p4 submit -c 2'.\n    Some file(s) could not be transferred from client.\n\nHrm.  Fake it by copying your example apple file to what it asks\nfor:\n\n    $ cp fail.png %fail.png\n    $ p4 submit -c 2\n    Submitting change 2.\n    add //depot/fail.png#1\n    Change 2 submitted.\n\n(But later p4 sync -f destroy both files.)\n\nVoila:\n\n    $ p4 fstat //depot/fail.png\n    ... depotFile //depot/fail.png\n    ... clientFile /home/pw/src/perforce/cli/fail.png\n    ... isMapped \n    ... headAction add\n    ... headType apple\n    ... headTime 1320111844\n    ... headRev 1\n    ... headChange 2\n    ... headModTime 1320111842\n    ... haveRev 1\n\nAnd git-p4 checks it out intact:\n\n    $ git p4 clone //depot\n    [..]\n    $ sha1sum depot/fail.png /tmp/sample_image_fail.png \n    93d175ad906147f4d75296bd2adb6d706f798c64  depot/fail.png\n    93d175ad906147f4d75296bd2adb6d706f798c64  /tmp/sample_image_fail.png\n\nWhich is what I thought an apple-filetype user would want.\nReverting the patch causes _no_ file to be created.  Is\nthis better?  Maybe the single-blob file, since it no longer\nappears in AppleDouble format, is just as useless as no file?\n\nThe other option is to use \"p4 print\" without the -G, which\nseems to retrieve only the data fork, and leave that in the repo.\nOf course, if you change it, and submit it, it makes a mess.\n\nWould it be good if git-p4 understood how to identify and create\nAppleDouble files on Mac?  If yes, eventually, we can revert the\ncommit and explain how this feature doesn't quite work yet.\nEven if no, it seems like we should revert and complain that\nthis apple support is broken.\n\n\t\t-- Pete\n"},{"id":"178612","messageId":"CAOk9v+_xXRGAGWg2L5u=r9qBS=H+ZmdF=TwumSyq7WKf-15okw@mail.gmail.com","threadId":"28813","inReplyTo":"20111101020841.GA8116@arf.padd.com","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2011-11-01T04:50:31Z","receivedAt":"2011-11-01T04:50:31Z","isPatch":false,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"> Would it be good if git-p4 understood how to identify and create\n> AppleDouble files on Mac?  If yes, eventually, we can revert the\n> commit and explain how this feature doesn't quite work yet.\n> Even if no, it seems like we should revert and complain that\n> this apple support is broken.\n\nI've used git-p4 for many years, and have always had to work around\nthe limitation of the \"apple\" filetype and the resulting lack of files\nadded to the git repo.\n\nOf course, I'd love to have git-p4 work seamlessly for this scenario.\nEven Perforce have a KB article on the limitation of the \"apple\"\nfiletype with git-p4:\n\n  http://kb.perforce.com/article/1417/git-p4\n\nAt least with 97a21ca50ef8 reverted, there is a warning that files\nwill be missing. The current behaviour results in a git repo with\nunusable files without any warning whatsoever. I think having unusable\nfiles, and without warnings, is worse as there is no indication that\nthere is a problem with files in the working tree.\n"},{"id":"178689","messageId":"loom.20111102T153631-769@post.gmane.org","threadId":"28813","inReplyTo":"CAOk9v+_xXRGAGWg2L5u=r9qBS=H+ZmdF=TwumSyq7WKf-15okw@mail.gmail.com","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-11-02T14:43:06Z","receivedAt":"2011-11-02T14:43:06Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Michael Wookey <michaelwookey <at> gmail.com> writes:\n> Of course, I'd love to have git-p4 work seamlessly for this scenario.\n> Even Perforce have a KB article on the limitation of the \"apple\"\n> filetype with git-p4:\n> \n>   http://kb.perforce.com/article/1417/git-p4\n> \n\"\"\"\nStep 2: Download Git-p4\n\nRecommended version is ermshiperete’s branch, which is available from:\n\nhttps://github.com/ermshiperete/git-p4\n\nNote: Omit the “git-p4.py25” file, which is an older version that is no longer\nneeded.\nAvoid Kernel.org’s Version of Git-p4\n\nGit’s main source at http://git-scm.com/download and\nhttp://www.kernel.org/pub/software/scm/git/ contains an older version of Git-p4\nwith limitations that ermshiperete’s branch avoids.\n\"\"\"\n\nI can almost guess _who_ wrote this KB ;)\n\nBut this is really frustrating. Why can't people just cooperate to make sure the\nversion in the main branch is the latest?\n\n\nVitor\n"},{"id":"178733","messageId":"CAOk9v+_xaS_Y1m17TROOSjgiscT+QEJWbpZbAZFmh8_tAviF6Q@mail.gmail.com","threadId":"28813","inReplyTo":"loom.20111102T153631-769@post.gmane.org","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2011-11-02T22:42:31Z","receivedAt":"2011-11-02T22:42:31Z","isPatch":false,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"On 3 November 2011 01:43, Vitor Antunes <vitor.hda@gmail.com> wrote:\n> Michael Wookey <michaelwookey <at> gmail.com> writes:\n>> Of course, I'd love to have git-p4 work seamlessly for this scenario.\n>> Even Perforce have a KB article on the limitation of the \"apple\"\n>> filetype with git-p4:\n>>\n>>   http://kb.perforce.com/article/1417/git-p4\n>>\n> \"\"\"\n> Step 2: Download Git-p4\n>\n> Recommended version is ermshiperete’s branch, which is available from:\n>\n> https://github.com/ermshiperete/git-p4\n>\n> Note: Omit the “git-p4.py25” file, which is an older version that is no longer\n> needed.\n> Avoid Kernel.org’s Version of Git-p4\n>\n> Git’s main source at http://git-scm.com/download and\n> http://www.kernel.org/pub/software/scm/git/ contains an older version of Git-p4\n> with limitations that ermshiperete’s branch avoids.\n> \"\"\"\n>\n> I can almost guess _who_ wrote this KB ;)\n>\n> But this is really frustrating. Why can't people just cooperate to make sure the\n> version in the main branch is the latest?\n\nI tried your suggested version of git-p4 (at rev 630fb678c46c) and\nunfortunately, the perforce repository fails to import. Firstly, there\nwas a problem with importing UTF-16 encoded files, secondly the\n\"apple\" filetype files are still skipped.\n"},{"id":"178765","messageId":"CAOpHH-W1JO9PLsyp2hQxfr6eyKRr+=pMkaDikV5NcFwF98Miow@mail.gmail.com","threadId":"28813","inReplyTo":"CAOk9v+_xaS_Y1m17TROOSjgiscT+QEJWbpZbAZFmh8_tAviF6Q@mail.gmail.com","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-11-03T11:04:06Z","receivedAt":"2011-11-03T11:04:06Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Hi Michael,\n\nOn Wed, Nov 2, 2011 at 10:42 PM, Michael Wookey <michaelwookey@gmail.com> wrote:\n> I tried your suggested version of git-p4 (at rev 630fb678c46c) and\n> unfortunately, the perforce repository fails to import. Firstly, there\n> was a problem with importing UTF-16 encoded files, secondly the\n> \"apple\" filetype files are still skipped.\n\nI had no intention of directing you to try that version. Sorry for\nmisleading you on this.\n\nI just found it interesting that P4's KB contains an article that\ndirects users to another version which isn't this one.\n\n-- \nVitor Antunes\n"},{"id":"178848","messageId":"20111104183957.GB18517@padd.com","threadId":"28813","inReplyTo":"CAOpHH-W1JO9PLsyp2hQxfr6eyKRr+=pMkaDikV5NcFwF98Miow@mail.gmail.com","subject":"Re: git-p4: problem with commit 97a21ca50ef8","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-11-04T18:39:57Z","receivedAt":"2011-11-04T18:39:57Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Thu, 03 Nov 2011 11:04 +0000:\n> Hi Michael,\n> \n> On Wed, Nov 2, 2011 at 10:42 PM, Michael Wookey <michaelwookey@gmail.com> wrote:\n> > I tried your suggested version of git-p4 (at rev 630fb678c46c) and\n> > unfortunately, the perforce repository fails to import. Firstly, there\n> > was a problem with importing UTF-16 encoded files, secondly the\n> > \"apple\" filetype files are still skipped.\n> \n> I had no intention of directing you to try that version. Sorry for\n> misleading you on this.\n> \n> I just found it interesting that P4's KB contains an article that\n> directs users to another version which isn't this one.\n\nWe're making contact offline with perforce folk and other git-p4\nfolk.  I'll update with the results.\n\nI've not run the kb version, but for git's git-p4, Utf-16 was\nfixed only recently (55aa571).  I'm going to revert the apple\nfiletype issue (97a21ca) that Michael found soon, hopefully\nbefore v1.7.8 goes out.\n\n\t\t-- Pete\n"},{"id":"178921","messageId":"20111105173607.GA12532@arf.padd.com","threadId":"28813","inReplyTo":"20111104183957.GB18517@padd.com","subject":"[PATCH] git-p4: ignore apple filetype","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-11-05T17:36:07Z","receivedAt":"2011-11-05T17:36:07Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Revert 97a21ca (git-p4: stop ignoring apple filetype, 2011-10-16)\nand add a test case.\n\nReported-by: Michael Wookey <michaelwookey@gmail.com>\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n\nThis is mostly a revert, but the test moves down a bit to be near\na similar clause for utf16.  Adding a big comment and test case\nhopefully keeps this code in place in the future.\n\nMichael: if you're willing to test this, I'd appreciate it.  In\nfact, running all the git-p4 unit tests on Mac would be great\nif you have a p4d:\n\n    mac$ ( cd t ; make t98* )\n\n contrib/fast-import/git-p4 |   13 +++++++++++++\n t/t9802-git-p4-filetype.sh |   31 +++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex f885d70..b975d67 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1318,6 +1318,19 @@ class P4Sync(Command, P4UserMap):\n             text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])\n             contents = [ text ]\n \n+        if type_base == \"apple\":\n+            # Apple filetype files will be streamed as a concatenation of\n+            # its appledouble header and the contents.  This is useless\n+            # on both macs and non-macs.  If using \"print -q -o xx\", it\n+            # will create \"xx\" with the data, and \"%xx\" with the header.\n+            # This is also not very useful.\n+            #\n+            # Ideally, someday, this script can learn how to generate\n+            # appledouble files directly and import those to git, but\n+            # non-mac machines can never find a use for apple filetype.\n+            print \"\\nIgnoring apple filetype file %s\" % file['depotFile']\n+            return\n+\n         # Perhaps windows wants unicode, utf16 newlines translated too;\n         # but this is not doing it.\n         if self.isWindows and type_base == \"text\":\ndiff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh\nindex 3b358ef..992bb8c 100755\n--- a/t/t9802-git-p4-filetype.sh\n+++ b/t/t9802-git-p4-filetype.sh\n@@ -101,6 +101,37 @@ test_expect_success 'keyword file test' '\n \t)\n '\n \n+build_gendouble() {\n+\tcat >gendouble.py <<-\\EOF\n+\timport sys\n+\timport struct\n+\timport array\n+\n+\ts = array.array(\"c\", '\\0' * 26)\n+\tstruct.pack_into(\">L\", s,  0, 0x00051607)  # AppleDouble\n+\tstruct.pack_into(\">L\", s,  4, 0x00020000)  # version 2\n+\ts.tofile(sys.stdout)\n+\tEOF\n+}\n+\n+test_expect_success 'ignore apple' '\n+\ttest_when_finished rm -f gendouble.py &&\n+\tbuild_gendouble &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest-genrandom apple 1024 >double.png &&\n+\t\t\"$PYTHON_PATH\" \"$TRASH_DIRECTORY/gendouble.py\" >%double.png &&\n+\t\tp4 add -t apple double.png &&\n+\t\tp4 submit -d appledouble\n+\t) &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --dest=\"$git\" //depot@all &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest ! -f double.png\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.7.345.g88d3c\n"},{"id":"178989","messageId":"CAOk9v+9xbq0zBF=96GXeK4L-Z9PrGB_NO5h06u63PweRgFFB2g@mail.gmail.com","threadId":"28813","inReplyTo":"20111105173607.GA12532@arf.padd.com","subject":"Re: [PATCH] git-p4: ignore apple filetype","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2011-11-07T02:21:33Z","receivedAt":"2011-11-07T02:21:33Z","isPatch":true,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"> This is mostly a revert, but the test moves down a bit to be near\n> a similar clause for utf16.  Adding a big comment and test case\n> hopefully keeps this code in place in the future.\n>\n> Michael: if you're willing to test this, I'd appreciate it.  In\n> fact, running all the git-p4 unit tests on Mac would be great\n> if you have a p4d:\n>\n>    mac$ ( cd t ; make t98* )\n\nI tested this and the warnings about the \"apple\" filetype do indeed\nappear. I have also run the test suite and all git-p4 tests pass on\nMac OS X (10.7.2).\n\nThanks again.\n"},{"id":"178997","messageId":"7vobwo4km3.fsf@alter.siamese.dyndns.org","threadId":"28813","inReplyTo":"CAOk9v+9xbq0zBF=96GXeK4L-Z9PrGB_NO5h06u63PweRgFFB2g@mail.gmail.com","subject":"Re: [PATCH] git-p4: ignore apple filetype","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-07T04:33:40Z","receivedAt":"2011-11-07T04:33:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, both. Will include the patch in 1.7.8-rc1.\n"}]}