From 62a3c4848ec4d51e3a9f1a6be6ad0ef75a7df67e Mon Sep 17 00:00:00 2001
From: Luke Diamand <luke@diamand.org>
Date: Thu, 27 Aug 2015 08:18:56 +0100
Subject: [PATCH 1/3] git-p4: failing test for ignoring invalid p4 labels

When importing a label which references a commit that git-p4 does
not know about, git-p4 should skip it and go on to process other
labels that can be imported.

Instead it crashes when attempting to find the missing commit in
the git history. This test demonstrates the problem.

Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9811-git-p4-label-import.sh | 45 ++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index 095238fffe..22d1fd3e3e 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -214,6 +214,51 @@ test_expect_success 'use git config to enable import/export of tags' '
 	)
 '
 
+p4_head_revision() {
+	p4 changes -m 1 "$@" | awk '{print $2}'
+}
+
+# Importing a label that references a P4 commit that
+# has not been seen. The presence of a label on a commit
+# we haven't seen should not cause git-p4 to fail. It should
+# merely skip that label, and still import other labels.
+test_expect_failure 'importing labels with missing revisions' '
+	test_when_finished cleanup_git &&
+	(
+		rm -fr "$cli" "$git" &&
+		mkdir "$cli" &&
+		P4CLIENT=missing-revision &&
+		client_view "//depot/missing-revision/... //missing-revision/..." &&
+		cd "$cli" &&
+		>f1 && p4 add f1 && p4 submit -d "start" &&
+
+		p4 tag -l TAG_S0 ... &&
+
+		>f2 && p4 add f2 && p4 submit -d "second" &&
+
+		startrev=$(p4_head_revision //depot/missing-revision/...) &&
+
+		>f3 && p4 add f3 && p4 submit -d "third" &&
+
+		p4 edit f2 && date >f2 && p4 submit -d "change" f2 &&
+
+		endrev=$(p4_head_revision //depot/missing-revision/...) &&
+
+		p4 tag -l TAG_S1 ... &&
+
+		# we should skip TAG_S0 since it is before our startpoint,
+		# but pick up TAG_S1.
+
+		git p4 clone --dest="$git" --import-labels -v \
+			//depot/missing-revision/...@$startrev,$endrev &&
+		(
+			cd "$git" &&
+			git rev-parse TAG_S1 &&
+			! git rev-parse TAG_S0
+		)
+	)
+'
+
 
 test_expect_success 'kill p4d' '
 	kill_p4d

From 9ab1cfe505d43215a61dc5012632dde66fe109db Mon Sep 17 00:00:00 2001
From: Luke Diamand <luke@diamand.org>
Date: Thu, 27 Aug 2015 08:18:57 +0100
Subject: [PATCH 2/3] git-p4: do not terminate creating tag for unknown commit

If p4 reports a tag for a commit that git-p4 does not know
about (e.g. because it references a P4 changelist that was
imported prior to the point at which the repo was cloned into
git), make sure that the error is correctly caught and handled.
rather than just crashing.

Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-p4.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 073f87bbfd..a62611a919 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2494,9 +2494,9 @@ class P4Sync(Command, P4UserMap):
                 # find the corresponding git commit; take the oldest commit
                 changelist = int(change['change'])
                 gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
-                     "--reverse", ":/\[git-p4:.*change = %d\]" % changelist])
+                     "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
                 if len(gitCommit) == 0:
-                    print "could not find git commit for changelist %d" % changelist
+                    print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
                 else:
                     gitCommit = gitCommit.strip()
                     commitFound = True

From b43702ac56e602d5163ef662fb9caf382da90b94 Mon Sep 17 00:00:00 2001
From: Luke Diamand <luke@diamand.org>
Date: Thu, 27 Aug 2015 08:18:58 +0100
Subject: [PATCH 3/3] git-p4: fix P4 label import for unprocessed commits

With --detect-labels enabled, git-p4 will try to create tags
using git fast-import by writing a "tag" clause to the
fast-import stream.

If the commit that the tag references has not yet actually
been processed by fast-import, then the tag can't be created
and git-p4 fails to import the P4 label.

Teach git-p4 to use fast-import "marks" when creating tags
which reference commits created during the current run of the
program.

Commits created before the current run are still referenced
in the old way using a normal git commit.

Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-p4.py                      | 25 +++++++++++++++++--------
 t/t9811-git-p4-label-import.sh |  2 +-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index a62611a919..2018011ae5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2322,8 +2322,11 @@ class P4Sync(Command, P4UserMap):
         else:
             return "%s <a@b>" % userid
 
-    # Stream a p4 tag
     def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
+        """ Stream a p4 tag.
+        commit is either a git commit, or a fast-import mark, ":<p4commit>"
+        """
+
         if verbose:
             print "writing tag %s for commit %s" % (labelName, commit)
         gitStream.write("tag %s\n" % labelName)
@@ -2374,7 +2377,7 @@ class P4Sync(Command, P4UserMap):
             self.clientSpecDirs.update_client_spec_path_cache(files)
 
         self.gitStream.write("commit %s\n" % branch)
-#        gitStream.write("mark :%s\n" % details["change"])
+        self.gitStream.write("mark :%s\n" % details["change"])
         self.committedChanges.add(int(details["change"]))
         committer = ""
         if author not in self.users:
@@ -2493,13 +2496,19 @@ class P4Sync(Command, P4UserMap):
             if change.has_key('change'):
                 # find the corresponding git commit; take the oldest commit
                 changelist = int(change['change'])
-                gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
-                     "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
-                if len(gitCommit) == 0:
-                    print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
-                else:
-                    gitCommit = gitCommit.strip()
+                if changelist in self.committedChanges:
+                    gitCommit = ":%d" % changelist       # use a fast-import mark
                     commitFound = True
+                else:
+                    gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
+                        "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
+                    if len(gitCommit) == 0:
+                        print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
+                    else:
+                        commitFound = True
+                        gitCommit = gitCommit.strip()
+
+                if commitFound:
                     # Convert from p4 time format
                     try:
                         tmwhen = time.strptime(labelDetails['Update'], "%Y/%m/%d %H:%M:%S")
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index 22d1fd3e3e..decb66ba30 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -222,7 +222,7 @@ p4_head_revision() {
 # has not been seen. The presence of a label on a commit
 # we haven't seen should not cause git-p4 to fail. It should
 # merely skip that label, and still import other labels.
-test_expect_failure 'importing labels with missing revisions' '
+test_expect_success 'importing labels with missing revisions' '
 	test_when_finished cleanup_git &&
 	(
 		rm -fr "$cli" "$git" &&