{"thread":{"id":"29976","subject":"[RFC/PATCHv1] git-p4: label import: first draft","startedAt":"2012-03-17T10:44:40Z","lastAt":"2012-03-17T10:44:41Z","messageCount":2,"participants":["Luke Diamand"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"187142","messageId":"1331981081-25598-1-git-send-email-luke@diamand.org","threadId":"29976","inReplyTo":null,"subject":"[RFC/PATCHv1] git-p4: label import: first draft","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-03-17T10:44:40Z","receivedAt":"2012-03-17T10:44:40Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"From: Luke Diamand <luked@diamand.org>\n\nThe existing label import code (--detect-labels) looks at each\ncommit being imported, and then checks for labels at that commit.\n\nThis doesn't work in the real world though because labels applied\nin changelists that have already been imported are ignored. e.g.:\n\n% p4 submit -d 'a change'\n% git-p4 rebase\n% p4 tag -l TAG ...\n% git-p4 sync --detect-labels\n\nTAG will never be detected.\n\nAdditionally the existing label detection has some annoying shortcomings\n\n* It gets the list of files at each label every time git-p4 is run,\n  which on a large repo with many labels is a problem. The new\n  scheme only pays this price the first time a label is seen.\n\n* It only imports a label if the number of files in the label matches\n  the number of files in the repo at the highest revision found. This\n  is to cope with the way that git tags and p4 labels cannot be\n  directly mapped to each other. In practice this is not a useful rule,\n  for example if the p4 tag was applied across a wider area of the tree\n  than just the part being imported by git-p4, then the tag is ignored.\n  The new code just checks that there are no diffs between the label\n  and the p4 changelist to be used.\n\nThe new code gets the lists of git tags and p4 labels, and then creates\ngit tags for all the missing p4 labels. Any labels that could not be\nimported are added to an ignore-list so that they are not repeatedly\nqueried. A regular expression can be specified to limit the p4 label\npatterns that will be imported.\n\nLimitations:\n\n* The code currently doesn't know anything about branch-based setups, and\n  has 'p4/master' hardcoded as the branch to search for git revisions. I\n  haven't yet thought about the best way to handle this, which is why this\n  is only an RFC at this stage. I would welcome suggestions on the best way\n  to deal with this.\n\n* The code uses \"git log --grep\" to get from a p4 changelist to a git\n  commit. This means it doesn't work on older versions of git, e.g.\n  git 1.7.0. Arguably it should use gitCommitByP4Change().\n\n* The unit tests could be expanded.\n\nThanks,\nLuke\n\nLuke Diamand (1):\n  git-p4: improved import of labels from p4\n\n Documentation/git-p4.txt       |   15 +++-\n contrib/fast-import/git-p4     |  152 +++++++++++++++++++++++++++++++---------\n t/t9811-git-p4-label-import.sh |  116 ++++++++++++++++++++++++++++++\n 3 files changed, 248 insertions(+), 35 deletions(-)\n create mode 100755 t/t9811-git-p4-label-import.sh\n\n-- \n1.7.10.rc0.158.gd9e55\n"},{"id":"187143","messageId":"1331981081-25598-2-git-send-email-luke@diamand.org","threadId":"29976","inReplyTo":"1331981081-25598-1-git-send-email-luke@diamand.org","subject":"[RFC/PATCHv1] git-p4: improved import of labels from p4","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2012-03-17T10:44:41Z","receivedAt":"2012-03-17T10:44:41Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"The existing label import code looks at each commit\nbeing imported, and then checks for labels at that\ncommit. This doesn't work in the real world though\nbecause it will drop labels applied on changelists\nthat have already been imported, a common pattern.\n\nThis change adds a new --import-labels option. With\nthis option, at the end of the sync, git-p4 gets lists\nof labels in p4 and git, and then creates a git tag\nfor each missing p4 label.\n\nThis means that tags created on older changelists are\nstill imported.\n\nTags that could not be imported are added to an ignore\nlist.\n---\n Documentation/git-p4.txt       |   15 +++-\n contrib/fast-import/git-p4     |  152 +++++++++++++++++++++++++++++++---------\n t/t9811-git-p4-label-import.sh |  116 ++++++++++++++++++++++++++++++\n 3 files changed, 248 insertions(+), 35 deletions(-)\n create mode 100755 t/t9811-git-p4-label-import.sh\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex b7c7929..c3eedab 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -205,7 +205,11 @@ git repository:\n \n --detect-labels::\n \tQuery p4 for labels associated with the depot paths, and add\n-\tthem as tags in git.\n+\tthem as tags in git. Limited usefulness as only imports labels\n+\tassociated with new changelists. Deprecated.\n+\n+--import-labels::\n+\tImport labels from p4 into git.\n \n --import-local::\n \tBy default, p4 branches are stored in 'refs/remotes/p4/',\n@@ -434,6 +438,15 @@ git-p4.branchList::\n \tenabled.  Each entry should be a pair of branch names separated\n \tby a colon (:).  This example declares that both branchA and\n \tbranchB were created from main:\n+\n+git-p4.ignoredP4Labels::\n+\tList of p4 labels to ignore. This is built automatically as\n+\tunimportabled labels are discovered.\n+\n+git-p4.validLabelRegexp::\n+\tOnly labels matching this regular expression will be imported. The\n+\tdefault value is '[A-Z0-9_\\-]+$'.\n+\n -------------\n git config       git-p4.branchList main:branchA\n git config --add git-p4.branchList main:branchB\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex c5362c4..a57443d 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1544,6 +1544,7 @@ class P4Sync(Command, P4UserMap):\n                 optparse.make_option(\"--changesfile\", dest=\"changesFile\"),\n                 optparse.make_option(\"--silent\", dest=\"silent\", action=\"store_true\"),\n                 optparse.make_option(\"--detect-labels\", dest=\"detectLabels\", action=\"store_true\"),\n+                optparse.make_option(\"--import-labels\", dest=\"importLabels\", action=\"store_true\"),\n                 optparse.make_option(\"--verbose\", dest=\"verbose\", action=\"store_true\"),\n                 optparse.make_option(\"--import-local\", dest=\"importIntoRemotes\", action=\"store_false\",\n                                      help=\"Import into refs/heads/ , not refs/remotes\"),\n@@ -1568,6 +1569,7 @@ class P4Sync(Command, P4UserMap):\n         self.branch = \"\"\n         self.detectBranches = False\n         self.detectLabels = False\n+        self.importLabels = False\n         self.changesFile = \"\"\n         self.syncWithOrigin = True\n         self.verbose = False\n@@ -1829,6 +1831,33 @@ class P4Sync(Command, P4UserMap):\n         else:\n             return \"%s <a@b>\" % userid\n \n+    # Stream a p4 tag\n+    def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):\n+        if verbose:\n+            print \"writing tag %s for commit %s\" % (labelName, commit)\n+        gitStream.write(\"tag %s\\n\" % labelName)\n+        gitStream.write(\"from %s\\n\" % commit)\n+\n+        if labelDetails.has_key('Owner'):\n+            owner = labelDetails[\"Owner\"]\n+        else:\n+            owner = None\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+            email = self.make_email(self.p4UserId())\n+        tagger = \"%s %s %s\" % (email, epoch, self.tz)\n+\n+        gitStream.write(\"tagger %s\\n\" % tagger)\n+\n+        description = labelDetails[\"Description\"]\n+        gitStream.write(\"data %d\\n\" % len(description))\n+        gitStream.write(description)\n+        gitStream.write(\"\\n\")\n+\n     def commit(self, details, files, branch, branchPrefixes, parent = \"\"):\n         epoch = details[\"time\"]\n         author = details[\"user\"]\n@@ -1893,25 +1922,7 @@ class P4Sync(Command, P4UserMap):\n                     cleanedFiles[info[\"depotFile\"]] = info[\"rev\"]\n \n                 if cleanedFiles == labelRevisions:\n-                    self.gitStream.write(\"tag tag_%s\\n\" % labelDetails[\"label\"])\n-                    self.gitStream.write(\"from %s\\n\" % branch)\n-\n-                    owner = labelDetails[\"Owner\"]\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-                        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-                    self.gitStream.write(\"data %d\\n\" % len(description))\n-                    self.gitStream.write(description)\n-                    self.gitStream.write(\"\\n\")\n+                    self.streamTag(self.gitStream, 'tag_%s' % labelDetails['label'], labelDetails, branch, epoch)\n \n                 else:\n                     if not self.silent:\n@@ -1923,6 +1934,7 @@ class P4Sync(Command, P4UserMap):\n                     print (\"Tag %s does not match with change %s: file count is different.\"\n                            % (labelDetails[\"label\"], change))\n \n+    # Build a dictionary of changelists and labels, for \"detect-labels\" option.\n     def getLabels(self):\n         self.labels = {}\n \n@@ -1949,6 +1961,74 @@ class P4Sync(Command, P4UserMap):\n         if self.verbose:\n             print \"Label changes: %s\" % self.labels.keys()\n \n+    # Return a list of all p4 labels\n+    def getP4Labels(self):\n+        labels = []\n+        for l in p4CmdList([\"labels\"] + [\"%s...\" % p for p in self.depotPaths]):\n+            labels.append(l['label'])\n+        return labels\n+\n+    def getMissingLabels(self, p4Labels):\n+        gitLabels = {}\n+        for line in read_pipe_lines([\"git\", \"tag\"]):\n+            label = line.strip()\n+            gitLabels[label] = 1\n+\n+        missingGitLabels = []\n+        for p4label in p4Labels:\n+            if not gitLabels.has_key(p4label):\n+                missingGitLabels.append(p4label)\n+\n+        return missingGitLabels\n+\n+    # Import p4 labels as git tags. A direct mapping does not\n+    # exist, so assume that if all the files are at the same revision\n+    # then we can use that, or it's something more complicated we should\n+    # just ignore.\n+    def importP4Labels(self, stream, depotPaths):\n+        p4Labels = self.getMissingLabels(self.getP4Labels())\n+        ignoredP4Labels = gitConfigList(\"git-p4.ignoredP4Labels\")\n+        validLabelRegexp = gitConfig(\"git-p4.validLabelRegexp\")\n+        if len(validLabelRegexp) == 0:\n+            validLabelRegexp = r'[A-Z0-9_\\-]+$'\n+        m = re.compile(validLabelRegexp)\n+\n+        for name in p4Labels:\n+            commitFound = False\n+\n+            if not m.match(name):\n+                if verbose:\n+                    print \"label %s does not match regexp\" % name\n+                continue\n+\n+            if name in ignoredP4Labels:\n+                continue\n+\n+            labelDetails = p4CmdList(['label', \"-o\", name])[0]\n+\n+            # get the most recent changelist for each file in this label\n+            change = p4Cmd([\"changes\", \"-m\", \"1\"] + [\"%s...@%s\" % (p, name)\n+                                for p in depotPaths])\n+\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\", \"log\", \"-n\", \"1\", \"--reverse\", \"--grep\", \"\\[git-p4:.*change = %d\\]$\" % changelist, \"--format=format:%H\", 'p4/master'])\n+                if len(gitCommit) == 0:\n+                    print \"could not find git commit for changelist %d\" % changelist\n+                else:\n+                    commitFound = True\n+                    # Convert from p4 time format; raises ValueError if it fails\n+                    tmwhen = time.strptime(labelDetails['Update'], \"%Y/%m/%d %H:%M:%S\")\n+                    when = int(time.mktime(tmwhen))\n+                    self.streamTag(stream, name, labelDetails, gitCommit, when)\n+                    if verbose:\n+                        print \"p4 label %s mapped to git commit %s\" % (name, gitCommit)\n+\n+            if not commitFound:\n+                # We can't import this label; don't try again\n+                system([\"git\", \"config\", \"--add\", \"git-p4.ignoredP4Labels\", name])\n+\n     def guessProjectName(self):\n         for p in self.depotPaths:\n             if p.endswith(\"/\"):\n@@ -2425,7 +2505,6 @@ class P4Sync(Command, P4UserMap):\n \n         self.depotPaths = newPaths\n \n-\n         self.loadUserMapFromCache()\n         self.labels = {}\n         if self.detectLabels:\n@@ -2489,22 +2568,24 @@ class P4Sync(Command, P4UserMap):\n             if len(changes) == 0:\n                 if not self.silent:\n                     print \"No changes to import!\"\n-                return True\n+            else:\n+                if not self.silent and not self.detectBranches:\n+                    print \"Import destination: %s\" % self.branch\n \n-            if not self.silent and not self.detectBranches:\n-                print \"Import destination: %s\" % self.branch\n+                self.updatedBranches = set()\n \n-            self.updatedBranches = set()\n+                self.importChanges(changes)\n \n-            self.importChanges(changes)\n+                if not self.silent:\n+                    print \"\"\n+                    if len(self.updatedBranches) > 0:\n+                        sys.stdout.write(\"Updated branches: \")\n+                        for b in self.updatedBranches:\n+                            sys.stdout.write(\"%s \" % b)\n+                        sys.stdout.write(\"\\n\")\n \n-            if not self.silent:\n-                print \"\"\n-                if len(self.updatedBranches) > 0:\n-                    sys.stdout.write(\"Updated branches: \")\n-                    for b in self.updatedBranches:\n-                        sys.stdout.write(\"%s \" % b)\n-                    sys.stdout.write(\"\\n\")\n+        if self.importLabels:\n+            self.importP4Labels(self.gitStream, self.depotPaths)\n \n         self.gitStream.close()\n         if importProcess.wait() != 0:\n@@ -2523,13 +2604,16 @@ class P4Sync(Command, P4UserMap):\n class P4Rebase(Command):\n     def __init__(self):\n         Command.__init__(self)\n-        self.options = [ ]\n+        self.options = [\n+                optparse.make_option(\"--import-labels\", dest=\"importLabels\", action=\"store_true\"),\n+                optparse.make_option(\"--verbose\", dest=\"verbose\", action=\"store_true\"),\n+        ]\n         self.description = (\"Fetches the latest revision from perforce and \"\n                             + \"rebases the current work (branch) against it\")\n-        self.verbose = False\n \n     def run(self, args):\n         sync = P4Sync()\n+        sync.importLabels = self.importLabels\n         sync.run([])\n \n         return self.rebase()\ndiff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh\nnew file mode 100755\nindex 0000000..f496bda\n--- /dev/null\n+++ b/t/t9811-git-p4-label-import.sh\n@@ -0,0 +1,116 @@\n+#!/bin/sh\n+\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+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+\t\tp4 tag -l this_tag_will_be_skipped &&\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\" //depot@all &&\n+\t\tcd \"$git\" &&\n+\t\t\"$GITP4\" sync --import-labels &&\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_F1_ONLY &&\n+\t\t! test -f f2 &&\n+\t\tgit checkout TAG_WITH\\$_SHELL_CHAR &&\n+\t\ttest -f f1 && test -f f2 && test -f file_with_\\$metachar &&\n+\n+\t\tgit show 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; this kind of tag\n+#   cannot be imported (at least not easily).\n+\n+test_expect_success '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\" //depot@all &&\n+\t\tcd \"$git\" &&\n+\t\t\"$GITP4\" sync --import-labels &&\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_F1_1 &&\n+\t\tls &&\n+\t\ttest -f f1 &&\n+\n+\t\tgit checkout TAG_F1_2 &&\n+\t\tls &&\n+\t\ttest -f f1\n+\t)\n+'\n+\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.10.rc0.158.gd9e55\n"}]}