{"thread":{"id":"29053","subject":"[PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","startedAt":"2011-11-30T09:03:33Z","lastAt":"2011-12-04T16:07:39Z","messageCount":17,"participants":["Luke Diamand","Vitor Antunes","Pete Wyckoff"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180146","messageId":"1322643817-13051-1-git-send-email-luke@diamand.org","threadId":"29053","inReplyTo":null,"subject":"[PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-11-30T09:03:33Z","receivedAt":"2011-11-30T09:03:33Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This is a small set of patches to git-p4 to fix a couple of issues with\nbranches and labels.\n\nThis is the second version of this patch series; the first one vanished\nwithout trace :-)\n\nFirstly, I've added the fixes needed so that branches and labels can\ncontain shell metacharacters (missed from the previous series). Added\na test case for this.\n\nIn adding the test case for labels I also found and fixed a few other\nsmall bugs in the label handling:\n\n - labels missing a description or \"EOT\" in their text cause problems;\n - labels without an owner cause problems.\n\nI also noticed, but did not fix, that you can't have more than one label\nper commit (the others are silently dropped) and the documentation for\nbranch import could be improved. I've added a (failing) test case for\nthe multiple label problem.\n\n\nLuke Diamand (4):\n git-p4: handle p4 branches and labels containing shell chars\n git-p4: cope with labels with empty descriptions\n git-p4: importing labels should cope with missing owner\n git-p4: add test for p4 labels\n\n contrib/fast-import/git-p4 | 61 +++++++++++++-----------\n t/t9801-git-p4-branch.sh | 48 ++++++++++++++++++\n t/t9804-git-p4-label.sh | 114 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 195 insertions(+), 28 deletions(-)\n create mode 100755 t/t9804-git-p4-label.sh\n\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"180147","messageId":"1322643817-13051-2-git-send-email-luke@diamand.org","threadId":"29053","inReplyTo":"1322643817-13051-1-git-send-email-luke@diamand.org","subject":"[PATCHv2 1/4] git-p4: handle p4 branches and labels containing shell chars","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-11-30T09:03:34Z","receivedAt":"2011-11-30T09:03:34Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Don't use shell expansion when detecting branches, as it will\nfail if the branch name contains a shell metachar. Similarly\nfor labels.\n\nAdd additional test for branches with shell metachars.\n\nSigned-off-by: Luke Diamand \n---\n contrib/fast-import/git-p4 | 8 +++---\n t/t9801-git-p4-branch.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex b975d67..bcac6ec 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -793,7 +793,7 @@ class P4Submit(Command, P4UserMap):\n def canChangeChangelists(self):\n # check to see if we have p4 admin or super-user permissions, either of\n # which are required to modify changelists.\n- results = p4CmdList(\"protects %s\" % self.depotPath)\n+ results = p4CmdList([\"protects\", self.depotPath])\n for r in results:\n if r.has_key('perm'):\n if r['perm'] == 'admin':\n@@ -1528,7 +1528,7 @@ class P4Sync(Command, P4UserMap):\n def getLabels(self):\n self.labels = {}\n \n- l = p4CmdList(\"labels %s...\" % ' '.join (self.depotPaths))\n+ l = p4CmdList([\"labels\", \"%s...\" % ' '.join (self.depotPaths)])\n if len(l) > 0 and not self.silent:\n print \"Finding files belonging to labels in %s\" % `self.depotPaths`\n \n@@ -1570,7 +1570,7 @@ class P4Sync(Command, P4UserMap):\n command = \"branches\"\n \n for info in p4CmdList(command):\n- details = p4Cmd(\"branch -o %s\" % info[\"branch\"])\n+ details = p4Cmd([\"branch\", \"-o\", info[\"branch\"]])\n viewIdx = 0\n while details.has_key(\"View%s\" % viewIdx):\n paths = details[\"View%s\" % viewIdx].split(\" \")\n@@ -1708,7 +1708,7 @@ class P4Sync(Command, P4UserMap):\n sourceRef = self.gitRefForBranch(sourceBranch)\n #print \"source \" + sourceBranch\n \n- branchParentChange = int(p4Cmd(\"changes -m 1 %s...@1,%s\" % (sourceDepotPath, firstChange))[\"change\"])\n+ branchParentChange = int(p4Cmd([\"changes\", \"-m\", \"1\", \"%s...@1,%s\" % (sourceDepotPath, firstChange)])[\"change\"])\n #print \"branch parent: %s\" % branchParentChange\n gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange)\n if len(gitParent) > 0:\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex a25f18d..bd08dff 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -226,6 +226,54 @@ test_expect_success 'git-p4 clone simple branches' '\n \t)\n '\n \n+# Create a branch with a shell metachar in its name\n+#\n+# 1. //depot/main\n+# 2. //depot/branch$3\n+\n+test_expect_success 'branch with shell char' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\n+\t\tmkdir -p main &&\n+\n+\t\techo f1 >main/f1 &&\n+\t\tp4 add main/f1 &&\n+\t\tp4 submit -d \"main/f1\" &&\n+\n+\t\tp4 integrate //depot/main/... //depot/branch\\$3/... &&\n+\t\tp4 submit -d \"integrate main to branch\\$3\" &&\n+\n+\t\techo f1 >branch\\$3/shell_char_branch_file &&\n+\t\tp4 add branch\\$3/shell_char_branch_file &&\n+\t\tp4 submit -d \"branch\\$3/shell_char_branch_file\" &&\n+\n+\t\tp4 branch -i <<-EOF &&\n+\t\tBranch: branch\\$3\n+\t\tView: //depot/main/... //depot/branch\\$3/...\n+\t\tEOF\n+\n+\t\tp4 edit main/f1 &&\n+\t\techo \"a change\" >> main/f1 &&\n+\t\tp4 submit -d \"a change\" main/f1 &&\n+\n+\t\tp4 integrate -b branch\\$3 &&\n+\t\tp4 resolve -am branch\\$3/... &&\n+\t\tp4 submit -d \"integrate main to branch\\$3\" &&\n+\n+\t\tcd \"$git\" &&\n+\n+\t\tgit config git-p4.branchList main:branch\\$3 &&\n+\t\t\"$GITP4\" clone --dest=. --detect-branches //depot@all &&\n+\t\tgit log --all --graph --decorate --stat &&\n+\t\tgit reset --hard p4/depot/branch\\$3 &&\n+\t\ttest -f shell_char_branch_file &&\n+\t\ttest -f f1\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"180148","messageId":"1322643817-13051-3-git-send-email-luke@diamand.org","threadId":"29053","inReplyTo":"1322643817-13051-1-git-send-email-luke@diamand.org","subject":"[PATCHv2 2/4] git-p4: cope with labels with empty descriptions","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-11-30T09:03:35Z","receivedAt":"2011-11-30T09:03:35Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Use an explicit length for the data in a label, rather\nthan EOT, so that labels with empty descriptions are\npassed through correctly.\n\nSigned-off-by: Luke Diamand \n---\n contrib/fast-import/git-p4 | 8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex bcac6ec..02f0f54 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1511,9 +1511,11 @@ class P4Sync(Command, P4UserMap):\n else:\n tagger = \"%s %s %s\" % (owner, epoch, self.tz)\n self.gitStream.write(\"tagger %s\\n\" % tagger)\n- self.gitStream.write(\"data <\n---\n contrib/fast-import/git-p4 | 45 +++++++++++++++++++++++--------------------\n 1 files changed, 24 insertions(+), 21 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 02f0f54..d97f927 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -553,6 +553,27 @@ class Command:\n def __init__(self):\n self.usage = \"usage: %prog [options]\"\n self.needsGit = True\n+ self.myP4UserId = None\n+\n+ def p4UserId(self):\n+ if self.myP4UserId:\n+ return self.myP4UserId\n+\n+ results = p4CmdList(\"user -o\")\n+ for r in results:\n+ if r.has_key('User'):\n+ self.myP4UserId = r['User']\n+ return r['User']\n+ die(\"Could not find your p4 user id\")\n+\n+ def p4UserIsMe(self, p4User):\n+ # return True if the given p4 user is actually me\n+ me = self.p4UserId()\n+ if not p4User or p4User != me:\n+ return False\n+ else:\n+ return True\n+\n \n class P4UserMap:\n def __init__(self):\n@@ -694,7 +715,6 @@ class P4Submit(Command, P4UserMap):\n self.verbose = False\n self.preserveUser = gitConfig(\"git-p4.preserveUser\").lower() == \"true\"\n self.isWindows = (platform.system() == \"Windows\")\n- self.myP4UserId = None\n \n def check(self):\n if len(p4CmdList(\"opened ...\")) > 0:\n@@ -802,25 +822,6 @@ class P4Submit(Command, P4UserMap):\n return 1\n return 0\n \n- def p4UserId(self):\n- if self.myP4UserId:\n- return self.myP4UserId\n-\n- results = p4CmdList(\"user -o\")\n- for r in results:\n- if r.has_key('User'):\n- self.myP4UserId = r['User']\n- return r['User']\n- die(\"Could not find your p4 user id\")\n-\n- def p4UserIsMe(self, p4User):\n- # return True if the given p4 user is actually me\n- me = self.p4UserId()\n- if not p4User or p4User != me:\n- return False\n- else:\n- return True\n-\n def prepareSubmitTemplate(self):\n # remove lines in the Files section that show changes to files outside the depot path we're committing into\n template = \"\"\n@@ -1506,7 +1507,9 @@ class P4Sync(Command, P4UserMap):\n \n owner = labelDetails[\"Owner\"]\n tagger = \"\"\n- if author in self.users:\n+ if not owner:\n+ tagger = \"%s %s %s\" % (self.p4UserId(), epoch, self.tz)\n+ elif author in self.users:\n tagger = \"%s %s %s\" % (self.users[owner], epoch, self.tz)\n else:\n tagger = \"%s %s %s\" % (owner, epoch, self.tz)\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"180149","messageId":"1322643817-13051-5-git-send-email-luke@diamand.org","threadId":"29053","inReplyTo":"1322643817-13051-1-git-send-email-luke@diamand.org","subject":"[PATCHv2 4/4] git-p4: add test for p4 labels","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-11-30T09:03:37Z","receivedAt":"2011-11-30T09:03:37Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Add basic test of p4 label import. Checks label import and\nimport with shell metachars; labels with different length\ndescriptions.\n\nAdd failure test case for multiple labels.\n\nSigned-off-by: Luke Diamand \n---\n t/t9804-git-p4-label.sh | 114 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 114 insertions(+), 0 deletions(-)\n create mode 100755 t/t9804-git-p4-label.sh\n\ndiff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh\nnew file mode 100755\nindex 0000000..a46763f\n--- /dev/null\n+++ b/t/t9804-git-p4-label.sh\n@@ -0,0 +1,114 @@\n+test_description='git-p4 p4 label tests'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+# Basic p4 label tests.\n+#\n+# Note: can't have more than one label per commit - others\n+# are silently discarded.\n+#\n+test_expect_success 'basic p4 labels' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tmkdir -p main &&\n+\n+\t\techo f1 >main/f1 &&\n+\t\tp4 add main/f1 &&\n+\t\tp4 submit -d \"main/f1\" &&\n+\n+\t\techo f2 >main/f2 &&\n+\t\tp4 add main/f2 &&\n+\t\tp4 submit -d \"main/f2\" &&\n+\n+\t\techo f3 >main/file_with_\\$metachar &&\n+\t\tp4 add main/file_with_\\$metachar &&\n+\t\tp4 submit -d \"file with metachar\" &&\n+\n+\t\tp4 tag -l tag_f1_only main/f1 &&\n+\t\tp4 tag -l tag_with\\$_shell_char main/... &&\n+\n+\t\techo f4 >main/f4 &&\n+\t\tp4 add main/f4 &&\n+\t\tp4 submit -d \"main/f4\" &&\n+\n+\t\tp4 label -i <<-EOF &&\n+\t\tLabel: long_label\n+\t\tDescription:\n+\t\t A Label first line\n+\t\t A Label second line\n+\t\tView:\t//depot/...\n+\t\tEOF\n+\n+\t\tp4 tag -l long_label ... &&\n+\n+\t\tp4 labels ... &&\n+\n+\t\tcd \"$git\" &&\n+\t\tpwd &&\n+\t\t\"$GITP4\" clone --dest=. --detect-labels //depot@all &&\n+\n+\t\tgit tag &&\n+\t\tgit tag >taglist &&\n+\t\ttest_line_count = 3 taglist &&\n+\n+\t\tcd main &&\n+\t\tgit checkout tag_tag_f1_only &&\n+\t\t! test -f f2 &&\n+\t\tgit checkout tag_tag_with\\$_shell_char &&\n+\t\ttest -f f1 && test -f f2 && test -f file_with_\\$metachar &&\n+\n+\t\tgit show tag_long_label | grep -q \"A Label second line\"\n+\t)\n+'\n+\n+# Test some label corner cases:\n+#\n+# - two tags on the same file; both should be available\n+# - a tag that is only on one file; the other file should\n+# not be checked out on the tag.\n+\n+test_expect_failure 'two labels on the same changelist' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tmkdir -p main &&\n+\n+\t\tp4 edit main/f1 main/f2 &&\n+\t\techo \"hello world\" >main/f1 &&\n+\t\techo \"not in the tag\" >main/f2 &&\n+\t\tp4 submit -d \"main/f[12]: testing two labels\" &&\n+\n+\t\tp4 tag -l tag_f1_1 main/f1 &&\n+\t\tp4 tag -l tag_f1_2 main/f1 &&\n+\n+\t\tp4 labels ... &&\n+\n+\t\tcd \"$git\" &&\n+\t\tpwd &&\n+\t\t\"$GITP4\" clone --dest=. --detect-labels //depot@all &&\n+\n+\t\tgit tag | grep -q tag_f1_1 &&\n+\t\tgit tag | grep -q tag_f1_2 &&\n+\n+\t\tcd main &&\n+\n+\t\tgit checkout tag_tag_f1_1 &&\n+\t\ttest -f f1 &&\n+\n+\t\tgit checkout tag_tag_f1_1 &&\n+\t\ttest -f f1 &&\n+\n+\t\t! test -f f2\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"180155","messageId":"loom.20111130T155409-599@post.gmane.org","threadId":"29053","inReplyTo":"1322643817-13051-1-git-send-email-luke@diamand.org","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-11-30T14:55:53Z","receivedAt":"2011-11-30T14:55:53Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Luke Diamand diamand.org> writes:\n\n> In adding the test case for labels I also found and fixed a few other\n> small bugs in the label handling:\n> \n> - labels missing a description or \"EOT\" in their text cause problems;\n> - labels without an owner cause problems.\n> \n> I also noticed, but did not fix, that you can't have more than one label\n> per commit (the others are silently dropped) and the documentation for\n> branch import could be improved. I've added a (failing) test case for\n> the multiple label problem.\n\nHi Luke,\n\nSeeing that you have some experience using labels, could I kindly ask you to\ninclude some description of it in git-p4.txt?\n\nThanks,\nVitor\n"},{"id":"180166","messageId":"4ED6809A.9020703@diamand.org","threadId":"29053","inReplyTo":"loom.20111130T155409-599@post.gmane.org","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-11-30T19:14:34Z","receivedAt":"2011-11-30T19:14:34Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 30/11/11 14:55, Vitor Antunes wrote:\n> Luke Diamand diamand.org> writes:\n>\n>> In adding the test case for labels I also found and fixed a few other\n>> small bugs in the label handling:\n>>\n>> - labels missing a description or \"EOT\" in their text cause problems;\n>> - labels without an owner cause problems.\n>>\n>> I also noticed, but did not fix, that you can't have more than one label\n>> per commit (the others are silently dropped) and the documentation for\n>> branch import could be improved. I've added a (failing) test case for\n>> the multiple label problem.\n>\n> Hi Luke,\n>\n> Seeing that you have some experience using labels, could I kindly ask you to\n> include some description of it in git-p4.txt?\n\nOK, if you can help me understand what's going on...\n\nThe label-detection bug that I've described, on further investigation, \nlooks to be a fundamental limitation.\n\nWith perforce, I can check out the head revision, and then tag just a \nsingle file. If I then check out on that tag, I get just that one file.\n\nI think I can't do that with git; certainly fast-import can't do it.\n\nSo the code in git-p4 that is checking the file vs label counts (git-p4 \naround line 1496) is actually trying to say \"this label can't be \nimported into git\".\n\nIf my understanding is correct, I can then fix my test and update the \ndocs and the code to explain this.\n\nAs an aside, git-p4.txt currently has quite good information on the \nconfig variables, but nothing on the command line variables. Possibly \nthat should be fixed.\n\nCheers!\nLuke\n"},{"id":"180169","messageId":"CAOpHH-XQdvdPtj=bCpyFVOLOCPf6+YnEUvk2pLh4sVp0XfvLTw@mail.gmail.com","threadId":"29053","inReplyTo":"4ED6809A.9020703@diamand.org","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-11-30T19:44:22Z","receivedAt":"2011-11-30T19:44:22Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Wed, Nov 30, 2011 at 7:14 PM, Luke Diamand wrote:\n> OK, if you can help me understand what's going on...\n>\n> The label-detection bug that I've described, on further investigation, looks\n> to be a fundamental limitation.\n>\n> With perforce, I can check out the head revision, and then tag just a single\n> file. If I then check out on that tag, I get just that one file.\n>\n> I think I can't do that with git; certainly fast-import can't do it.\n>\n> So the code in git-p4 that is checking the file vs label counts (git-p4\n> around line 1496) is actually trying to say \"this label can't be imported\n> into git\".\n>\n> If my understanding is correct, I can then fix my test and update the docs\n> and the code to explain this.\n>\n> As an aside, git-p4.txt currently has quite good information on the config\n> variables, but nothing on the command line variables. Possibly that should\n> be fixed.\n\nHey Luke,\n\nI did not have any success in the few times I tried to import labels.\nSince you were updating some of that code I was hoping you could\nextend git-p4.txt such that I would be able to understand it. This to\nsay: I can't help you much with my current expertise level on how\nlabels are implemented. Will need to look into that later.\n\n-- \nVitor Antunes\n"},{"id":"180172","messageId":"20111130225813.GA11544@arf.padd.com","threadId":"29053","inReplyTo":"4ED6809A.9020703@diamand.org","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-11-30T22:58:13Z","receivedAt":"2011-11-30T22:58:13Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Wed, 30 Nov 2011 19:14 +0000:\n> On 30/11/11 14:55, Vitor Antunes wrote:\n> >Luke Diamand diamand.org> writes:\n> >\n> >>In adding the test case for labels I also found and fixed a few other\n> >>small bugs in the label handling:\n> >>\n> >> - labels missing a description or \"EOT\" in their text cause problems;\n> >> - labels without an owner cause problems.\n> >>\n> >>I also noticed, but did not fix, that you can't have more than one label\n> >>per commit (the others are silently dropped) and the documentation for\n> >>branch import could be improved. I've added a (failing) test case for\n> >>the multiple label problem.\n\nI was hanging onto your v1, and made a comment on the v1's 3/4\nthat perhaps you missed. Also acked the entire thing. I can\nresend if my mailer ate it.\n\nDon't expect Junio to pick it up until after 1.7.8 goes out.\n\n> >Hi Luke,\n> >\n> >Seeing that you have some experience using labels, could I kindly ask you to\n> >include some description of it in git-p4.txt?\n> \n> OK, if you can help me understand what's going on...\n> \n> The label-detection bug that I've described, on further\n> investigation, looks to be a fundamental limitation.\n> \n> With perforce, I can check out the head revision, and then tag just\n> a single file. If I then check out on that tag, I get just that one\n> file.\n> \n> I think I can't do that with git; certainly fast-import can't do it.\n\nThis is another fundamental disconnect between p4 and git.\nReading\n\nhttp://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html\n\nit is clear that labels are supposed to be used exactly where\ntags cannot: to specify a collection of files as they existed\nat _different_ points in the commit history.\n\nThus I think supporting labels is kind of pointless. But in the\nrestricted use case that perforce docs tell us not to do, namely\nusing labels to identify change numbers, git can reflect that\nwith tags.\n\n> So the code in git-p4 that is checking the file vs label counts\n> (git-p4 around line 1496) is actually trying to say \"this label\n> can't be imported into git\".\n> \n> If my understanding is correct, I can then fix my test and update\n> the docs and the code to explain this.\n\nYeah. It's just a big restriction on how labels get imported.\nA better error message and some docs would be useful.\n\n> As an aside, git-p4.txt currently has quite good information on the\n> config variables, but nothing on the command line variables.\n> Possibly that should be fixed.\n\nRecently, I wrote an asciidoc-style document for git-p4, and\ntried to find all the options on all the commands. There's a lot\nmore than I ever knew about. :) I'll take another pass\nthrough it then send it out for review. Maybe we can get rid\nof the old git-p4.txt then and work on improving a more\nstructured document.\n\n\t\t-- Pete\n"},{"id":"180173","messageId":"20111130230007.GA11598@arf.padd.com","threadId":"29053","inReplyTo":"20111130225813.GA11544@arf.padd.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-11-30T23:00:07Z","receivedAt":"2011-11-30T23:00:07Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"P.S. Since you're respinning anyway to change the label code,\ncan you stick the 'branch with shell char' test from t9801 in\nwith t9803? It feels more appropriate there than with the branch\ntests. And avoids collision with some Vitor code that will get\nadded eventually.\n\n\t\t-- Pete\n"},{"id":"180181","messageId":"CAOpHH-U3PF0v7EPqnO0FNxNKh+uF1GH=cnA_MA09SaQTxo0cDA@mail.gmail.com","threadId":"29053","inReplyTo":"20111130225813.GA11544@arf.padd.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-12-01T00:33:15Z","receivedAt":"2011-12-01T00:33:15Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff wrote:\n> This is another fundamental disconnect between p4 and git.\n> Reading\n>\n> http://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html\n>\n> it is clear that labels are supposed to be used exactly where\n> tags cannot:  to specify a collection of files as they existed\n> at _different_ points in the commit history.\n\nCheck the \"Use Tag Fixup Branches\" section in fast-import manual, it\nmight help on this. The basic concept is to create a special branch\nthat puts all files in the same state the P4 label would put them and\nthen tag it in git.\n\nTried to use this for my branch stuff, but with no success.\n\n> Thus I think supporting labels is kind of pointless.  But in the\n> restricted use case that perforce docs tell us not to do, namely\n> using labels to identify change numbers, git can reflect that\n> with tags.\n\nI still use labels as simple tags. Telling that we should use\nchangelists instead of labels is the same as saying that we should use\nIP addresses instead of host names. It works, but I doubt you will\never remember it unless you write it down somewhere.\n\n-- \nVitor Antunes\n"},{"id":"180182","messageId":"CAOpHH-U6NxRSioRZg9_+f146vVR+S1hWsVbRmHz+vsqtz+vXiA@mail.gmail.com","threadId":"29053","inReplyTo":"20111130230007.GA11598@arf.padd.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-12-01T00:37:05Z","receivedAt":"2011-12-01T00:37:05Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff wrote:\n> And avoids collision with some Vitor code that will get\n> added eventually.\n\nI'm starting to doubt I will ever be able to overcome the fast-import\nlimitation on not allowing branch delesetion. Sure, the code I wrote was\ngarbage! But they seem to be very relunctant on the concept of deleting\nbranches on the fly.\nDid you ever take a look at the patch I sent? Maybe you could help me\nshape it up a bit.\n\n-- \nVitor Antunes\n"},{"id":"180189","messageId":"20111201040259.GA22189@arf.padd.com","threadId":"29053","inReplyTo":"CAOpHH-U3PF0v7EPqnO0FNxNKh+uF1GH=cnA_MA09SaQTxo0cDA@mail.gmail.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-12-01T04:02:59Z","receivedAt":"2011-12-01T04:02:59Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:33 +0000:\n> On Wed, Nov 30, 2011 at 10:58 PM, Pete Wyckoff wrote:\n> > This is another fundamental disconnect between p4 and git.\n> > Reading\n> >\n> > http://www.perforce.com/perforce/doc.current/manuals/p4guide/07_labels.html\n> >\n> > it is clear that labels are supposed to be used exactly where\n> > tags cannot:  to specify a collection of files as they existed\n> > at _different_ points in the commit history.\n> \n> Check the \"Use Tag Fixup Branches\" section in fast-import manual, it\n> might help on this. The basic concept is to create a special branch\n> that puts all files in the same state the P4 label would put them and\n> then tag it in git.\n> \n> Tried to use this for my branch stuff, but with no success.\n\nInteresting, thanks. One could certainly construct any arbitrary\ntree to represent the tag. But it would be a truly evil merge of\npossibly many commits.\n\n> > Thus I think supporting labels is kind of pointless.  But in the\n> > restricted use case that perforce docs tell us not to do, namely\n> > using labels to identify change numbers, git can reflect that\n> > with tags.\n> \n> I still use labels as simple tags. Telling that we should use\n> changelists instead of labels is the same as saying that we should use\n> IP addresses instead of host names. It works, but I doubt you will\n> ever remember it unless you write it down somewhere.\n\nI see your point. P4 labels are the only way that they support\ntagging, apparently. I'm okay with leaving label support in\ngit-p4. And it will be nice if Luke makes it behave a bit\nbetter. But doing heroics to emulate cross-commit tags feels\nlike a lot of work, and the wrong direction.\n\n\t\t-- Pete\n"},{"id":"180195","messageId":"4ED73B67.6080706@diamand.org","threadId":"29053","inReplyTo":"20111130230007.GA11598@arf.padd.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-12-01T08:31:35Z","receivedAt":"2011-12-01T08:31:35Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 30/11/11 23:00, Pete Wyckoff wrote:\n> P.S. Since you're respinning anyway to change the label code,\n> can you stick the 'branch with shell char' test from t9801 in\n> with t9803? It feels more appropriate there than with the branch\n> tests. And avoids collision with some Vitor code that will get\n> added eventually.\n>\n\nOK!\n"},{"id":"180223","messageId":"CAOpHH-XL5OGpnihEgqnXqUUFsMxXn2wSdLadegnC1epg44vs8A@mail.gmail.com","threadId":"29053","inReplyTo":"CAOpHH-UMdLpCPx1+D2dtQJs+=t1+0U2srKfTwBi-TEF4F7EDyw@mail.gmail.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-12-01T21:59:29Z","receivedAt":"2011-12-01T21:59:29Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Dec 1, 2011 4:03 AM, \"Pete Wyckoff\" wrote:\n> I see your point.  P4 labels are the only way that they support\n> tagging, apparently.  I'm okay with leaving label support in\n> git-p4.  And it will be nice if Luke makes it behave a bit\n> better.  But doing heroics to emulate cross-commit tags feels\n> like a lot of work, and the wrong direction.\n\nAgreed. Lets keep it simple.\n\n-- \nVitor Antunes\n"},{"id":"180241","messageId":"4ED89115.6040309@diamand.org","threadId":"29053","inReplyTo":"CAOpHH-XL5OGpnihEgqnXqUUFsMxXn2wSdLadegnC1epg44vs8A@mail.gmail.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-12-02T08:49:25Z","receivedAt":"2011-12-02T08:49:25Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 01/12/11 21:59, Vitor Antunes wrote:\n> On Dec 1, 2011 4:03 AM, \"Pete Wyckoff\" wrote:\n>> I see your point. P4 labels are the only way that they support\n>> tagging, apparently. I'm okay with leaving label support in\n>> git-p4. And it will be nice if Luke makes it behave a bit\n>> better. But doing heroics to emulate cross-commit tags feels\n>> like a lot of work, and the wrong direction.\n>\n> Agreed. Lets keep it simple.\n>\n\nI think I'm going to have to go away and do a bit more work on this. The \nexisting label code is still quite buggy (or my understanding is \nbroken). Either way I'd rather get it to the point where it actually \nworks and passes all its tests.\n\nThe two issues I'm seeing are:\n\n- two p4 labels covering the same set of files; only one of them gets \nimported.\n\n- if you have a p4 label on a subset of files then it gets dropped \n(which is fine) but so do most of the other labels (as far as I can tell).\n\nI think if this could be made to work it would actually be really useful \nthough.\n\n(Pete - I've found your previous email; not sure why I didn't see it \nbefore. I'll roll that change in with what I'm doing).\n\n\nRegards!\nLuke\n"},{"id":"180295","messageId":"20111204160739.GA13549@arf.padd.com","threadId":"29053","inReplyTo":"CAOpHH-U6NxRSioRZg9_+f146vVR+S1hWsVbRmHz+vsqtz+vXiA@mail.gmail.com","subject":"Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-12-04T16:07:39Z","receivedAt":"2011-12-04T16:07:39Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:37 +0000:\n> On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff wrote:\n> > And avoids collision with some Vitor code that will get\n> > added eventually.\n> \n> I'm starting to doubt I will ever be able to overcome the fast-import\n> limitation on not allowing branch delesetion. Sure, the code I wrote was\n> garbage! But they seem to be very relunctant on the concept of deleting\n> branches on the fly.\n> Did you ever take a look at the patch I sent? Maybe you could help me\n> shape it up a bit.\n\nI don't think we necessarily need branch deletion inside the\nfast-import. Can you go back and look at my mail from August and\nsee if that approach is doable? Just make a single commit on a\nthrowaway branch with no parent, checkpoint, then do diff-tree\nfor each potential parent until you find a match. Do the commit\nfor real where it goes. As git-p4 exits, we'll delete the branch\nref of the test commit.\n\nIf this works, we can see if fast-import can be taught to\ngenerate a tree object without a commit to save the need for a\ntemporary branch.\n\n\t\t-- Pete\n"}]}