{"thread":{"id":"39751","subject":"[git-p4] import with labels fails when commit is not transferred","startedAt":"2015-06-30T08:45:23Z","lastAt":"2015-08-04T07:19:24Z","messageCount":3,"participants":["Holl, Marcus","Luke Diamand"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"265203","messageId":"A24B2157A13A04409644C1456B5AE27F1CE4B663@DEWDFEMB19A.global.corp.sap","threadId":"39751","inReplyTo":null,"subject":"[git-p4] import with labels fails when commit is not transferred","fromName":"Holl, Marcus","fromEmail":"marcus.holl@sap.com","sentAt":"2015-06-30T08:45:23Z","receivedAt":"2015-06-30T08:45:23Z","isPatch":false,"sender":{"key":"marcus.holl@sap.com","avatar":null},"body":"Hi,\n\nI have an issue with the git p4 tooling regarding import of labels.\n\nMy git version is 2.4.5\n\nI try to transform a perforce repository. My command line is:\ngit p4 clone --verbose --detect-branches --import-local --import-labels --destination //depot@all\n\n\nThe relevant parts in the gitconfig is:\n[git-p4]\n branchUser = \n \n \nFor that user there is a branch mapping defined with a lot of entries like:\n//depot/trunk/... //depot/branches/ipro-status-8-2--branch/...\n//depot/trunk/... //depot/branches/9-0-preview/...\n//depot/trunk/... //depot/branches/release-8-0-0-branch/...\n//depot/trunk/... //depot/branches/release-8-1-0-branch/...\n//depot/trunk/... //depot/branches/release-8-2-0-branch/...\n//depot/trunk/... //depot/branches/release-8-3-0-branch/...\n//depot/trunk/... //depot/branches/release-8-4-branch/...\n//depot/trunk/... //depot/branches/release-8-5-branch/...\n...\n\n\nThe import fails with the log output that can be found at the bottom of this mail.\n\ngit log -all -grep \"\\[git-p4:.*change\\ =\\ 69035\\]\" reports nothing. The commit is not contained in the git repository.\n\np4 describe for changelist 69035 returns a reasonable result. This change contains one file located at a path in the perforce folder structure that comes without corresponding entry in the perforce branch mapping. \n\nAccording to the given branch mapping it looks reasonable to me that the change is omitted in the git repository. But in my opinion the import should not fail in such a case.\n\nA reasonable behavior would be to blacklist the label (add it to git-p4.ignoredP4Labels) and to continue with the next label.\n\nAttached is a proposal for a fix that needs to be carefully reviews since I'm not that experienced with python.\n\nOther proposals for resolving this issue are highly appreciated.\n\nThanks a lot and best regards,\nMarcus Holl\n\n\nLog output:\n\nReading pipe: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\\\[git-p4:.*change = 69035\\\\]']\nfatal: ambiguous argument ':/\\[git-p4:.*change = 69035\\]': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\nied with change: 69078, original Date: 2010-04-22T09:07:24.000000Z\\n', 'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976', 'Access': '2015/06/26 14:50:15', 'Owner': 'svn_p4_converter', 'Options': 'unlocked noautoreload'}\np4 label release-8-1-0-976 mapped to git commit 82a11809928b86a7bde03cf486428de52ab3380f\nwriting tag release-9-0-0-179 for commit fb8370cd04806686c567ad720d065436f2334b4a\nlabelDetails= {'code': 'stat', 'Description': 'Created or modified with change: 96984, original Date: 2011-12-22T16:01:25.681427Z\\n', 'Update': '2013/11/02 15:15:50', 'Label': 'release-9-0-0-179', 'Access': '2015/06/26 14:50:16', 'Owner': 'build', 'Options': 'unlocked noautoreload'}\np4 label release-9-0-0-179 mapped to git commit fb8370cd04806686c567ad720d065436f2334b4a\nTraceback (most recent call last):\n File \"/usr/lib/git/git-p4\", line 3297, in \n main()\n File \"/usr/lib/git/git-p4\", line 3291, in main\n if not cmd.run(args):\n File \"/usr/lib/git/git-p4\", line 3165, in run\n if not P4Sync.run(self, depotPaths):\n File \"/usr/lib/git/git-p4\", line 3045, in run\n self.importP4Labels(self.gitStream, missingP4Labels)\n File \"/usr/lib/git/git-p4\", line 2421, in importP4Labels\n \"--reverse\", \":/\\[git-p4:.*change = %d\\]\" % changelist])\n File \"/usr/lib/git/git-p4\", line 138, in read_pipe\n die('Command failed: %s' % str(c))\n File \"/usr/lib/git/git-p4\", line 106, in die\n raise Exception(msg)\nException: Command failed: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\\\[git-p4:.*change = 69035\\\\]']\n\n\nFrom 4714fb23885278963db09aa3b7a426db1ebd5be4 Mon Sep 17 00:00:00 2001\nFrom: Marcus Holl \nDate: Tue, 30 Jun 2015 10:10:12 +0200\nSubject: [PATCH] [git-p4] Do not fail on not found commit when importing\n labels\n\nWhen a commit is not transferred from perforce to git since it is located\nin the perforce folder layout in a place that does not match the branches\naccording to a given branch mapping the import fails.\nWith this change the corresponding label is added to git-p4.ignoredP4Labels\nand it is continued with the next label.\n---\n git-p4.py | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 549022e..f3c8c6f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2417,8 +2417,12 @@ class P4Sync(Command, P4UserMap):\n if change.has_key('change'):\n # find the corresponding git commit; take the oldest commit\n changelist = int(change['change'])\n- gitCommit = read_pipe([\"git\", \"rev-list\", \"--max-count=1\",\n- \"--reverse\", \":/\\[git-p4:.*change = %d\\]\" % changelist])\n+ gitCommit = \"\"\n+ try:\n+ gitCommit = read_pipe([\"git\", \"rev-list\", \"--max-count=1\",\n+ \"--reverse\", \":/\\[git-p4:.*change = %d\\]\" % changelist])\n+ except:\n+ pass\n if len(gitCommit) == 0:\n print \"could not find git commit for changelist %d\" % changelist\n else:\n-- \n1.9.3 (Apple Git-50)\n\n"},{"id":"265479","messageId":"5597529D.9080100@diamand.org","threadId":"39751","inReplyTo":"A24B2157A13A04409644C1456B5AE27F1CE4B663@DEWDFEMB19A.global.corp.sap","subject":"Re: [git-p4] import with labels fails when commit is not transferred","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-07-04T03:27:25Z","receivedAt":"2015-07-04T03:27:25Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Sorry for not replying earlier, and thanks for taking the time to \ninvestigate this!\n\nIt's a pretty subtle corner case: I think a test case would be useful. \nI'm going to try to put something together, unless you beat me to it!\n\n(I think t9811-git-p4-label-import.sh is the one that needs extending).\n\nThanks!\nLuke\n\nOn 30/06/15 09:45, Holl, Marcus wrote:\n> Hi,\n>\n> I have an issue with the git p4 tooling regarding import of labels.\n>\n> My git version is 2.4.5\n>\n> I try to transform a perforce repository. My command line is:\n> git p4 clone --verbose --detect-branches --import-local --import-labels --destination //depot@all\n>\n>\n> The relevant parts in the gitconfig is:\n> [git-p4]\n> branchUser = \n>\n>\n> For that user there is a branch mapping defined with a lot of entries like:\n> //depot/trunk/... //depot/branches/ipro-status-8-2--branch/...\n> //depot/trunk/... //depot/branches/9-0-preview/...\n> //depot/trunk/... //depot/branches/release-8-0-0-branch/...\n> //depot/trunk/... //depot/branches/release-8-1-0-branch/...\n> //depot/trunk/... //depot/branches/release-8-2-0-branch/...\n> //depot/trunk/... //depot/branches/release-8-3-0-branch/...\n> //depot/trunk/... //depot/branches/release-8-4-branch/...\n> //depot/trunk/... //depot/branches/release-8-5-branch/...\n> ...\n>\n>\n> The import fails with the log output that can be found at the bottom of this mail.\n>\n> git log -all -grep \"\\[git-p4:.*change\\ =\\ 69035\\]\" reports nothing. The commit is not contained in the git repository.\n>\n> p4 describe for changelist 69035 returns a reasonable result. This change contains one file located at a path in the perforce folder structure that comes without corresponding entry in the perforce branch mapping.\n>\n> According to the given branch mapping it looks reasonable to me that the change is omitted in the git repository. But in my opinion the import should not fail in such a case.\n>\n> A reasonable behavior would be to blacklist the label (add it to git-p4.ignoredP4Labels) and to continue with the next label.\n>\n> Attached is a proposal for a fix that needs to be carefully reviews since I'm not that experienced with python.\n>\n> Other proposals for resolving this issue are highly appreciated.\n>\n> Thanks a lot and best regards,\n> Marcus Holl\n>\n>\n> Log output:\n>\n> Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\\\[git-p4:.*change = 69035\\\\]']\n> fatal: ambiguous argument ':/\\[git-p4:.*change = 69035\\]': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git [...] -- [...]'\n> ied with change: 69078, original Date: 2010-04-22T09:07:24.000000Z\\n', 'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976', 'Access': '2015/06/26 14:50:15', 'Owner': 'svn_p4_converter', 'Options': 'unlocked noautoreload'}\n> p4 label release-8-1-0-976 mapped to git commit 82a11809928b86a7bde03cf486428de52ab3380f\n> writing tag release-9-0-0-179 for commit fb8370cd04806686c567ad720d065436f2334b4a\n> labelDetails= {'code': 'stat', 'Description': 'Created or modified with change: 96984, original Date: 2011-12-22T16:01:25.681427Z\\n', 'Update': '2013/11/02 15:15:50', 'Label': 'release-9-0-0-179', 'Access': '2015/06/26 14:50:16', 'Owner': 'build', 'Options': 'unlocked noautoreload'}\n> p4 label release-9-0-0-179 mapped to git commit fb8370cd04806686c567ad720d065436f2334b4a\n> Traceback (most recent call last):\n> File \"/usr/lib/git/git-p4\", line 3297, in \n> main()\n> File \"/usr/lib/git/git-p4\", line 3291, in main\n> if not cmd.run(args):\n> File \"/usr/lib/git/git-p4\", line 3165, in run\n> if not P4Sync.run(self, depotPaths):\n> File \"/usr/lib/git/git-p4\", line 3045, in run\n> self.importP4Labels(self.gitStream, missingP4Labels)\n> File \"/usr/lib/git/git-p4\", line 2421, in importP4Labels\n> \"--reverse\", \":/\\[git-p4:.*change = %d\\]\" % changelist])\n> File \"/usr/lib/git/git-p4\", line 138, in read_pipe\n> die('Command failed: %s' % str(c))\n> File \"/usr/lib/git/git-p4\", line 106, in die\n> raise Exception(msg)\n> Exception: Command failed: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\\\[git-p4:.*change = 69035\\\\]']\n>\n"},{"id":"267383","messageId":"55C0677C.6090202@diamand.org","threadId":"39751","inReplyTo":"5597529D.9080100@diamand.org","subject":"Re: [git-p4] import with labels fails when commit is not transferred","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-08-04T07:19:24Z","receivedAt":"2015-08-04T07:19:24Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 04/07/15 04:27, Luke Diamand wrote:\n> Sorry for not replying earlier, and thanks for taking the time to\n> investigate this!\n>\n> It's a pretty subtle corner case: I think a test case would be useful.\n> I'm going to try to put something together, unless you beat me to it!\n>\n> (I think t9811-git-p4-label-import.sh is the one that needs extending).\n\nI've been looking into this a bit more, and I think that you don't need \na try/except - it's enough just to add \"ignore_error=True\" to the call \nto read_lines.\n\nIt also looks like this is a general problem of importing a label where \nthe revision doesn't exist in git. For example, if you create a p4 \nlabel, then clone at the revision after that, you will get the problem.\n\nI've got a test case and a modified git-p4.py, but I haven't yet got to \nthe point where I'm convinced it works. I think the fix is pretty much \nwhat you've shown (replacing try-except with ignore_error) but I'd like \nmy test case to be a bit more convincing.\n\nThanks!\nLuke\n\n>\n> On 30/06/15 09:45, Holl, Marcus wrote:\n>> Hi,\n>>\n>> I have an issue with the git p4 tooling regarding import of labels.\n>>\n>> My git version is 2.4.5\n>>\n>> I try to transform a perforce repository. My command line is:\n>> git p4 clone --verbose --detect-branches --import-local\n>> --import-labels --destination //depot@all\n>>\n>>\n>> The relevant parts in the gitconfig is:\n>> [git-p4]\n>> branchUser = \n>>\n>>\n>> For that user there is a branch mapping defined with a lot of entries\n>> like:\n>> //depot/trunk/... //depot/branches/ipro-status-8-2--branch/...\n>> //depot/trunk/... //depot/branches/9-0-preview/...\n>> //depot/trunk/... //depot/branches/release-8-0-0-branch/...\n>> //depot/trunk/... //depot/branches/release-8-1-0-branch/...\n>> //depot/trunk/... //depot/branches/release-8-2-0-branch/...\n>> //depot/trunk/... //depot/branches/release-8-3-0-branch/...\n>> //depot/trunk/... //depot/branches/release-8-4-branch/...\n>> //depot/trunk/... //depot/branches/release-8-5-branch/...\n>> ...\n>>\n>>\n>> The import fails with the log output that can be found at the bottom\n>> of this mail.\n>>\n>> git log -all -grep \"\\[git-p4:.*change\\ =\\ 69035\\]\" reports nothing.\n>> The commit is not contained in the git repository.\n>>\n>> p4 describe for changelist 69035 returns a reasonable result. This\n>> change contains one file located at a path in the perforce folder\n>> structure that comes without corresponding entry in the perforce\n>> branch mapping.\n>>\n>> According to the given branch mapping it looks reasonable to me that\n>> the change is omitted in the git repository. But in my opinion the\n>> import should not fail in such a case.\n>>\n>> A reasonable behavior would be to blacklist the label (add it to\n>> git-p4.ignoredP4Labels) and to continue with the next label.\n>>\n>> Attached is a proposal for a fix that needs to be carefully reviews\n>> since I'm not that experienced with python.\n>>\n>> Other proposals for resolving this issue are highly appreciated.\n>>\n>> Thanks a lot and best regards,\n>> Marcus Holl\n>>\n>>\n>> Log output:\n>>\n>> Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse',\n>> ':/\\\\[git-p4:.*change = 69035\\\\]']\n>> fatal: ambiguous argument ':/\\[git-p4:.*change = 69035\\]': unknown\n>> revision or path not in the working tree.\n>> Use '--' to separate paths from revisions, like this:\n>> 'git [...] -- [...]'\n>> ied with change: 69078, original Date: 2010-04-22T09:07:24.000000Z\\n',\n>> 'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976',\n>> 'Access': '2015/06/26 14:50:15', 'Owner': 'svn_p4_converter',\n>> 'Options': 'unlocked noautoreload'}\n>> p4 label release-8-1-0-976 mapped to git commit\n>> 82a11809928b86a7bde03cf486428de52ab3380f\n>> writing tag release-9-0-0-179 for commit\n>> fb8370cd04806686c567ad720d065436f2334b4a\n>> labelDetails= {'code': 'stat', 'Description': 'Created or modified\n>> with change: 96984, original Date: 2011-12-22T16:01:25.681427Z\\n',\n>> 'Update': '2013/11/02 15:15:50', 'Label': 'release-9-0-0-179',\n>> 'Access': '2015/06/26 14:50:16', 'Owner': 'build', 'Options':\n>> 'unlocked noautoreload'}\n>> p4 label release-9-0-0-179 mapped to git commit\n>> fb8370cd04806686c567ad720d065436f2334b4a\n>> Traceback (most recent call last):\n>> File \"/usr/lib/git/git-p4\", line 3297, in \n>> main()\n>> File \"/usr/lib/git/git-p4\", line 3291, in main\n>> if not cmd.run(args):\n>> File \"/usr/lib/git/git-p4\", line 3165, in run\n>> if not P4Sync.run(self, depotPaths):\n>> File \"/usr/lib/git/git-p4\", line 3045, in run\n>> self.importP4Labels(self.gitStream, missingP4Labels)\n>> File \"/usr/lib/git/git-p4\", line 2421, in importP4Labels\n>> \"--reverse\", \":/\\[git-p4:.*change = %d\\]\" % changelist])\n>> File \"/usr/lib/git/git-p4\", line 138, in read_pipe\n>> die('Command failed: %s' % str(c))\n>> File \"/usr/lib/git/git-p4\", line 106, in die\n>> raise Exception(msg)\n>> Exception: Command failed: ['git', 'rev-list', '--max-count=1',\n>> '--reverse', ':/\\\\[git-p4:.*change = 69035\\\\]']\n>>\n>\n"}]}