From fed23693ba1a1ea7beb851cd385d458136e91664 Mon Sep 17 00:00:00 2001 From: Vitor Antunes Date: Wed, 25 Jan 2012 23:48:22 +0000 Subject: [PATCH 1/4] git-p4: Search for parent commit on branch creation 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 Acked-by: Pete Wyckoff Signed-off-by: Junio C Hamano --- contrib/fast-import/git-p4 | 46 +++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4 index bcb0e6cd23..0bf2625077 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] @@ -1957,6 +1967,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: @@ -2013,7 +2037,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, @@ -2348,6 +2386,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): From c5665efed2619fae9ec5a57c555c89a91804e886 Mon Sep 17 00:00:00 2001 From: Vitor Antunes Date: Wed, 25 Jan 2012 23:48:23 +0000 Subject: [PATCH 2/4] git-p4: Add test case for complex branch import 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 Acked-by: Pete Wyckoff Signed-off-by: Junio C Hamano --- t/t9801-git-p4-branch.sh | 94 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 87 insertions(+), 7 deletions(-) diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index a25f18d36a..6ff713b1f7 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 ) ' From 3558f32f1f5fdb3d566e39f6e07cd3f97d124da6 Mon Sep 17 00:00:00 2001 From: Pete Wyckoff Date: Wed, 25 Jan 2012 23:48:24 +0000 Subject: [PATCH 3/4] git-p4: Change p4 command invocation Change p4 command invocation to avoid going through the shell. This allows names with spaces and wildcards to work. Signed-off-by: Pete Wyckoff Signed-off-by: Vitor Antunes Signed-off-by: Junio C Hamano --- contrib/fast-import/git-p4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4 index 0bf2625077..b951ce58bb 100755 --- a/contrib/fast-import/git-p4 +++ b/contrib/fast-import/git-p4 @@ -1984,7 +1984,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: From e7d7a56796c457b0a96e58e7638950db824b52af Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 26 Jan 2012 11:40:09 -0800 Subject: [PATCH 4/4] t9801: do not overuse test_must_fail test_must_fail is to make sure a program we can potentially break during the course of updating git itself exits with a non-zero status in a clean and controlled way. When we expect a non-zero exit status from the commands we use from the underlying platform in tests, e.g. making sure a string "error: " does not appear in the output by running "grep 'error: '", just use "! grep" for readability. It is not like we will try to update Git and suddenly 'grep' we use from the system starts segfaulting. Signed-off-by: Junio C Hamano --- t/t9801-git-p4-branch.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 6ff713b1f7..d414705416 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -208,7 +208,7 @@ test_expect_success 'git-p4 clone simple branches' ' test -f file1 && test -f file2 && test ! -f file3 && - test_must_fail grep update file2 && + ! grep update file2 && git reset --hard p4/depot/branch3 && test -f file1 && test -f file2 && @@ -286,7 +286,7 @@ test_expect_success 'git-p4 clone complex branches' ' test_path_is_file file1 && test_path_is_file file2 && test_path_is_missing file3 && - test_must_fail grep update file2 && + ! grep update file2 && git reset --hard p4/depot/branch3 && test_path_is_file file1 && test_path_is_file file2 && @@ -296,12 +296,12 @@ test_expect_success 'git-p4 clone complex branches' ' test_path_is_file file1 && test_path_is_file file2 && test_path_is_missing file3 && - test_must_fail grep update file2 && + ! 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 && + ! grep update file2 && test_path_is_missing .git/git-p4-tmp ) '