[PATCH v3 0/3] git-p4: Search for parent commit on branch creation

[PATCH v3 0/3] git-p4: Search for parent commit on branch creation

[Vitor Antunes]

I think this will, hopefully, be the final version of this series of
patches. This version includes the following changes since v2:

 - Move search algorithm into its own function.
 - Use lists instead of strings on shell commands.
 - Some small (almost cosmetic) updates to test cases.

Pete Wyckoff (1):
  git-p4: Change p4 command invocation

Vitor Antunes (2):
  git-p4: Search for parent commit on branch creation
  git-p4: Add test case for complex branch import

 contrib/fast-import/git-p4 |   48 +++++++++++++++++++++-
 t/t9801-git-p4-branch.sh   |   94 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 133 insertions(+), 9 deletions(-)

-- 
1.7.8.3

[PATCH v3 1/3] git-p4: Search for parent commit on branch creation

[Vitor Antunes]

To find out which is its parent the commit of the new branch is compared
sequentially to each blob of the parent branch from the newest to the
oldest. The first blob which results in a zero diff is considered the
parent commit. If none is found, then the commit is applied to the top
of the parent branch.

A fast-import "checkpoint" call is required because diff-tree is only
able to work with blobs on disk. But most of these commits will not be
part of the final imported tree, making fast-import fail. To avoid this,
the temporary branches are tracked and then removed at the end of the
import process.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4 |   46 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..584cc41 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1429,6 +1429,8 @@ class P4Sync(Command, P4UserMap):
         self.cloneExclude = []
         self.useClientSpec = False
         self.clientSpecDirs = None
+        self.tempBranches = []
+        self.tempBranchLocation = "git-p4-tmp"
 
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
@@ -1450,6 +1452,14 @@ class P4Sync(Command, P4UserMap):
                    .replace("%25", "%")
         return path
 
+    # Force a checkpoint in fast-import and wait for it to finish
+    def checkpoint(self):
+        self.gitStream.write("checkpoint\n\n")
+        self.gitStream.write("progress checkpoint\n\n")
+        out = self.gitOutput.readline()
+        if self.verbose:
+            print "checkpoint finished: " + out
+
     def extractFilesFromCommit(self, commit):
         self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
                              for path in self.cloneExclude]
@@ -1948,6 +1958,20 @@ class P4Sync(Command, P4UserMap):
         self.importChanges(changes)
         return True
 
+    def searchParent(self, parent, branch, target):
+        parentFound = False
+        for blob in read_pipe_lines(["git", "rev-list", "--reverse", "--no-merges", parent]):
+            blob = blob.strip()
+            if len(read_pipe(["git", "diff-tree", blob, target])) == 0:
+                parentFound = True
+                if self.verbose:
+                    print "Found parent of %s in commit %s" % (branch, blob)
+                break
+        if parentFound:
+            return blob
+        else:
+            return None
+
     def importChanges(self, changes):
         cnt = 1
         for change in changes:
@@ -2004,7 +2028,21 @@ class P4Sync(Command, P4UserMap):
                             parent = self.initialParents[branch]
                             del self.initialParents[branch]
 
-                        self.commit(description, filesForCommit, branch, [branchPrefix], parent)
+                        blob = None
+                        if len(parent) > 0:
+                            tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
+                            if self.verbose:
+                                print "Creating temporary branch: " + tempBranch
+                            self.commit(description, filesForCommit, tempBranch, [branchPrefix])
+                            self.tempBranches.append(tempBranch)
+                            self.checkpoint()
+                            blob = self.searchParent(parent, branch, tempBranch)
+                        if blob:
+                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)
+                        else:
+                            if self.verbose:
+                                print "Parent of %s not found. Committing into head of %s" % (branch, parent)
+                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)
                 else:
                     files = self.extractFilesFromCommit(description)
                     self.commit(description, files, self.branch, self.depotPaths,
@@ -2339,6 +2377,12 @@ class P4Sync(Command, P4UserMap):
         self.gitOutput.close()
         self.gitError.close()
 
+        # Cleanup temporary branches created during import
+        if self.tempBranches != []:
+            for branch in self.tempBranches:
+                read_pipe("git update-ref -d %s" % branch)
+            os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
+
         return True
 
 class P4Rebase(Command):
-- 
1.7.8.3

[PATCH v3 2/3] git-p4: Add test case for complex branch import

[Vitor Antunes]

Check if branches created from old changelists are correctly imported.
Also included some updates to simple branch test so that both are
coherent in respect to each other.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9801-git-p4-branch.sh |   94 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index a25f18d..6ff713b 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -172,9 +172,9 @@ test_expect_success 'add simple p4 branches' '
 		echo file1 >file1 &&
 		echo file2 >file2 &&
 		p4 add file1 file2 &&
-		p4 submit -d "branch1" &&
+		p4 submit -d "Create branch1" &&
 		p4 integrate //depot/branch1/... //depot/branch2/... &&
-		p4 submit -d "branch2" &&
+		p4 submit -d "Integrate branch2 from branch1" &&
 		echo file3 >file3 &&
 		p4 add file3 &&
 		p4 submit -d "add file3 in branch1" &&
@@ -182,7 +182,7 @@ test_expect_success 'add simple p4 branches' '
 		echo update >>file2 &&
 		p4 submit -d "update file2 in branch1" &&
 		p4 integrate //depot/branch1/... //depot/branch3/... &&
-		p4 submit -d "branch3"
+		p4 submit -d "Integrate branch3 from branch1"
 	)
 '
 
@@ -203,17 +203,17 @@ test_expect_success 'git-p4 clone simple branches' '
 		test -f file1 &&
 		test -f file2 &&
 		test -f file3 &&
-		grep -q update file2 &&
+		grep update file2 &&
 		git reset --hard p4/depot/branch2 &&
 		test -f file1 &&
 		test -f file2 &&
 		test ! -f file3 &&
-		test_must_fail grep -q update file2 &&
+		test_must_fail grep update file2 &&
 		git reset --hard p4/depot/branch3 &&
 		test -f file1 &&
 		test -f file2 &&
 		test -f file3 &&
-		grep -q update file2 &&
+		grep update file2 &&
 		cd "$cli" &&
 		cd branch1 &&
 		p4 edit file2 &&
@@ -222,7 +222,87 @@ test_expect_success 'git-p4 clone simple branches' '
 		cd "$git" &&
 		git reset --hard p4/depot/branch1 &&
 		"$GITP4" rebase &&
-		grep -q file2_ file2
+		grep file2_ file2
+	)
+'
+
+# Create a complex branch structure in P4 depot to check if they are correctly
+# cloned. The branches are created from older changelists to check if git-p4 is
+# able to correctly detect them.
+# The final expected structure is:
+# `branch1
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch2
+# | `- file1
+# | `- file2
+# `branch3
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch4
+# | `- file1
+# | `- file2
+# `branch5
+#   `- file1
+#   `- file2
+#   `- file3
+test_expect_success 'git-p4 add complex branches' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$cli" &&
+		changelist=$(p4 changes -m1 //depot/... | cut -d" " -f2) &&
+		changelist=$(($changelist - 5)) &&
+		p4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&
+		p4 submit -d "Integrate branch4 from branch1@${changelist}" &&
+		changelist=$(($changelist + 2)) &&
+		p4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&
+		p4 submit -d "Integrate branch5 from branch1@${changelist}"
+	)
+'
+
+# Configure branches through git-config and clone them. git-p4 will only be able
+# to clone the original structure if it is able to detect the origin changelist
+# of each branch.
+test_expect_success 'git-p4 clone complex branches' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$git" &&
+		git config git-p4.branchList branch1:branch2 &&
+		git config --add git-p4.branchList branch1:branch3 &&
+		git config --add git-p4.branchList branch1:branch4 &&
+		git config --add git-p4.branchList branch1:branch5 &&
+		"$GITP4" clone --dest=. --detect-branches //depot@all &&
+		git log --all --graph --decorate --stat &&
+		git reset --hard p4/depot/branch1 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
+		grep update file2 &&
+		git reset --hard p4/depot/branch2 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_missing file3 &&
+		test_must_fail grep update file2 &&
+		git reset --hard p4/depot/branch3 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
+		grep update file2 &&
+		git reset --hard p4/depot/branch4 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_missing file3 &&
+		test_must_fail grep update file2 &&
+		git reset --hard p4/depot/branch5 &&
+		test_path_is_file file1 &&
+		test_path_is_file file2 &&
+		test_path_is_file file3 &&
+		test_must_fail grep update file2 &&
+		test_path_is_missing .git/git-p4-tmp
 	)
 '
 
-- 
1.7.8.3

[PATCH v3 3/3] git-p4: Change p4 command invocation

[Vitor Antunes]

From: Pete Wyckoff <pw@padd.com>

Change p4 command invocation to avoid going through the shell. This
allows names with spaces and wildcards to work.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 584cc41..74d3613 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1975,7 +1975,7 @@ class P4Sync(Command, P4UserMap):
     def importChanges(self, changes):
         cnt = 1
         for change in changes:
-            description = p4Cmd("describe %s" % change)
+            description = p4Cmd(["describe", str(change)])
             self.updateOptionDict(description)
 
             if not self.silent:
-- 
1.7.8.3

Re: [PATCH v3 0/3] git-p4: Search for parent commit on branch creation

[Pete Wyckoff]

vitor.hda@gmail.com wrote on Wed, 25 Jan 2012 23:48 +0000:
> I think this will, hopefully, be the final version of this series of
> patches. This version includes the following changes since v2:
> 
>  - Move search algorithm into its own function.
>  - Use lists instead of strings on shell commands.
>  - Some small (almost cosmetic) updates to test cases.

Whole series

Acked-by: Pete Wyckoff <pw@padd.com>

Thanks for making all the changes.

> Pete Wyckoff (1):
>   git-p4: Change p4 command invocation
> 
> Vitor Antunes (2):
>   git-p4: Search for parent commit on branch creation
>   git-p4: Add test case for complex branch import
> 
>  contrib/fast-import/git-p4 |   48 +++++++++++++++++++++-
>  t/t9801-git-p4-branch.sh   |   94 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 133 insertions(+), 9 deletions(-)
> 
> -- 
> 1.7.8.3
>