{"thread":{"id":"29399","subject":"[PATCHv4 0/5] git-p4: small fixes to branches and labels","startedAt":"2012-01-19T09:52:24Z","lastAt":"2012-01-19T15:19:34Z","messageCount":7,"participants":["Luke Diamand","Pete Wyckoff"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"182774","messageId":"1326966749-9077-1-git-send-email-luke@diamand.org","threadId":"29399","inReplyTo":null,"subject":"[PATCHv4 0/5] git-p4: small fixes to branches and labels","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-01-19T09:52:24Z","receivedAt":"2012-01-19T09:52:24Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This is the fourth version of some small fixes to git-p4 branch and\nlabel handling, incorporating a fix from Pete Wyckoff and an\nadditional failing test.\n\nThis change does not fix the other problems with git-p4 labels:\n\n- two p4 labels on the same changelist will fall over\n- labels must match exactly the list of files imported\n- you can't import a label without a p4 commit\n\n\nLuke Diamand (5):\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 git-p4: label import fails with multiple labels at the same\n changelist\n\n contrib/fast-import/git-p4 | 79 ++++++++++++++-----------\n t/t9803-git-p4-shell-metachars.sh | 48 ++++++++++++++++\n t/t9804-git-p4-label.sh | 113 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 205 insertions(+), 35 deletions(-)\n create mode 100755 t/t9804-git-p4-label.sh\n\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"182775","messageId":"1326966749-9077-2-git-send-email-luke@diamand.org","threadId":"29399","inReplyTo":"1326966749-9077-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 1/5] git-p4: handle p4 branches and labels containing shell chars","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-01-19T09:52:25Z","receivedAt":"2012-01-19T09:52:25Z","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/t9803-git-p4-shell-metachars.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 5aacea8..163e95a 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -799,7 +799,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@@ -1769,7 +1769,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...\" % p for p in self.depotPaths])\n if len(l) > 0 and not self.silent:\n print \"Finding files belonging to labels in %s\" % `self.depotPaths`\n \n@@ -1811,7 +1811,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@@ -1949,7 +1949,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/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh\nindex db04375..db67020 100755\n--- a/t/t9803-git-p4-shell-metachars.sh\n+++ b/t/t9803-git-p4-shell-metachars.sh\n@@ -57,6 +57,54 @@ test_expect_success 'deleting with shell metachars' '\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":"182776","messageId":"1326966749-9077-3-git-send-email-luke@diamand.org","threadId":"29399","inReplyTo":"1326966749-9077-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 2/5] git-p4: cope with labels with empty descriptions","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-01-19T09:52:26Z","receivedAt":"2012-01-19T09:52:26Z","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 163e95a..5b59bc8 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1752,9 +1752,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 | 63 ++++++++++++++++++++++++-------------------\n 1 files changed, 35 insertions(+), 28 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 5b59bc8..58551b9 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -563,6 +563,26 @@ class Command:\n class P4UserMap:\n def __init__(self):\n self.userMapFromPerforceServer = False\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 def getUserCacheFilename(self):\n home = os.environ.get(\"HOME\", os.environ.get(\"USERPROFILE\"))\n@@ -700,7 +720,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@@ -808,25 +827,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@@ -1675,6 +1675,12 @@ class P4Sync(Command, P4UserMap):\n if self.stream_file.has_key('depotFile'):\n self.streamOneP4File(self.stream_file, self.stream_contents)\n \n+ def make_email(self, userid):\n+ if userid in self.users:\n+ return self.users[userid]\n+ else:\n+ return \"%s \" % userid\n+\n def commit(self, details, files, branch, branchPrefixes, parent = \"\"):\n epoch = details[\"time\"]\n author = details[\"user\"]\n@@ -1698,10 +1704,7 @@ class P4Sync(Command, P4UserMap):\n committer = \"\"\n if author not in self.users:\n self.getUserMapFromPerforceServer()\n- if author in self.users:\n- committer = \"%s %s %s\" % (self.users[author], epoch, self.tz)\n- else:\n- committer = \"%s %s %s\" % (author, epoch, self.tz)\n+ committer = \"%s %s %s\" % (self.make_email(author), epoch, self.tz)\n \n self.gitStream.write(\"committer %s\\n\" % committer)\n \n@@ -1746,11 +1749,15 @@ class P4Sync(Command, P4UserMap):\n self.gitStream.write(\"from %s\\n\" % branch)\n \n owner = labelDetails[\"Owner\"]\n- tagger = \"\"\n- if author in self.users:\n- tagger = \"%s %s %s\" % (self.users[owner], epoch, self.tz)\n+\n+ # Try to use the owner of the p4 label, or failing that,\n+ # the current p4 user id.\n+ if owner:\n+ email = self.make_email(owner)\n else:\n- tagger = \"%s %s %s\" % (owner, epoch, self.tz)\n+ email = self.make_email(self.p4UserId())\n+ tagger = \"%s %s %s\" % (email, epoch, self.tz)\n+\n self.gitStream.write(\"tagger %s\\n\" % tagger)\n \n description = labelDetails[\"Description\"]\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"182778","messageId":"1326966749-9077-5-git-send-email-luke@diamand.org","threadId":"29399","inReplyTo":"1326966749-9077-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 4/5] git-p4: add test for p4 labels","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-01-19T09:52:28Z","receivedAt":"2012-01-19T09:52:28Z","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\nSigned-off-by: Luke Diamand \n---\n t/t9804-git-p4-label.sh | 72 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 72 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..5fa2bcf\n--- /dev/null\n+++ b/t/t9804-git-p4-label.sh\n@@ -0,0 +1,72 @@\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\t\"$GITP4\" clone --dest=\"$git\" --detect-labels //depot@all &&\n+\t\tcd \"$git\" &&\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_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.8.rc1.209.geac91.dirty\n"},{"id":"182779","messageId":"1326966749-9077-6-git-send-email-luke@diamand.org","threadId":"29399","inReplyTo":"1326966749-9077-1-git-send-email-luke@diamand.org","subject":"[PATCHv4 5/5] git-p4: label import fails with multiple labels at the same changelist","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-01-19T09:52:29Z","receivedAt":"2012-01-19T09:52:29Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"git-p4 has an array of changelists with one label per changelist.\nBut you can have multiple labels on a single changelist and so this\ncode fails.\n\nAdd a test case demonstrating the problem.\n\nSigned-off-by: Luke Diamand \n---\n t/t9804-git-p4-label.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh\nindex 5fa2bcf..80d01ea 100755\n--- a/t/t9804-git-p4-label.sh\n+++ b/t/t9804-git-p4-label.sh\n@@ -65,6 +65,47 @@ test_expect_success 'basic p4 labels' '\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; this kind of tag\n+# cannot be imported (at least not easily).\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/... &&\n+\t\tp4 tag -l tag_f1_2 main/... &&\n+\n+\t\tp4 labels ... &&\n+\n+\t\t\"$GITP4\" clone --dest=\"$git\" --detect-labels //depot@all &&\n+\t\tcd \"$git\" &&\n+\n+\t\tgit tag | grep tag_f1 &&\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\tls &&\n+\t\ttest -f f1 &&\n+\n+\t\tgit checkout tag_tag_f1_2 &&\n+\t\tls &&\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":"182790","messageId":"20120119151934.GA3698@padd.com","threadId":"29399","inReplyTo":"1326966749-9077-1-git-send-email-luke@diamand.org","subject":"Re: [PATCHv4 0/5] git-p4: small fixes to branches and labels","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-19T15:19:34Z","receivedAt":"2012-01-19T15:19:34Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Thu, 19 Jan 2012 09:52 +0000:\n> This is the fourth version of some small fixes to git-p4 branch and\n> label handling, incorporating a fix from Pete Wyckoff and an\n> additional failing test.\n> \n> This change does not fix the other problems with git-p4 labels:\n> \n> - two p4 labels on the same changelist will fall over\n> - labels must match exactly the list of files imported\n> - you can't import a label without a p4 commit\n\nThis all looks great to me. Thanks for adding that failing test,\nand fixing the pre-existing bug in invoking p4 labels.\n\n\t\t-- Pete\n"}]}