{"thread":{"id":"29372","subject":"[PATCH 0/3] git-p4: Search for parent commit on branch creation","startedAt":"2012-01-16T00:39:17Z","lastAt":"2012-01-17T23:43:18Z","messageCount":10,"participants":["Vitor Antunes","Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"182585","messageId":"1326674360-2771-1-git-send-email-vitor.hda@gmail.com","threadId":"29372","inReplyTo":null,"subject":"[PATCH 0/3] git-p4: Search for parent commit on branch creation","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-16T00:39:17Z","receivedAt":"2012-01-16T00:39:17Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"This patch was originally sent as a RFC and was improved since then\naccording to feedback given by Pete Wyckoff. At the time it required\ncalling fast-import with the --force argument. To avoid this the new\nversion tracks the temporary branches created during post-processing and\nremoves them at the end of the import process.\n\nVitor Antunes (3):\n  git-p4: Add checkpoint() task\n  git-p4: Search for parent commit on branch creation\n  git-p4: Add test case for complex branch import\n\n contrib/fast-import/git-p4 |   39 ++++++++++++++++++++-\n t/t9801-git-p4-branch.sh   |   83 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 121 insertions(+), 1 deletions(-)\n\n-- \n1.7.8.3\n"},{"id":"182586","messageId":"1326674360-2771-2-git-send-email-vitor.hda@gmail.com","threadId":"29372","inReplyTo":"1326674360-2771-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 1/3] git-p4: Add checkpoint() task","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-16T00:39:18Z","receivedAt":"2012-01-16T00:39:18Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n contrib/fast-import/git-p4 |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 3e1aa27..417d119 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1450,6 +1450,14 @@ class P4Sync(Command, P4UserMap):\n                    .replace(\"%25\", \"%\")\n         return path\n \n+    # Force a checkpoint in fast-import and wait for it to finish\n+    def checkpoint(self):\n+        self.gitStream.write(\"checkpoint\\n\\n\")\n+        self.gitStream.write(\"progress checkpoint\\n\\n\")\n+        out = self.gitOutput.readline()\n+        if self.verbose:\n+            print \"checkpoint finished: \" + out\n+\n     def extractFilesFromCommit(self, commit):\n         self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n                              for path in self.cloneExclude]\n-- \n1.7.8.3\n"},{"id":"182587","messageId":"1326674360-2771-3-git-send-email-vitor.hda@gmail.com","threadId":"29372","inReplyTo":"1326674360-2771-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-16T00:39:19Z","receivedAt":"2012-01-16T00:39:19Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"To find out which is its parent the commit of the new branch is applied\nsequentially to each blob of the parent branch from the newest to the\noldest. The first blob which results in a zero diff is considered the\nparent commit. If none is found, then the commit is applied to the top\nof the parent branch.\n\nA fast-import \"checkpoint\" call is required for each comparison because\ndiff-tree is only able to work with blobs on disk. But most of these\ncommits will not be part of the final imported tree, making fast-import\nfail. To avoid this, the temporary branches are tracked and then removed\nat the end of the import process.\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n contrib/fast-import/git-p4 |   31 ++++++++++++++++++++++++++++++-\n 1 files changed, 30 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 417d119..e2f9165 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1429,6 +1429,8 @@ class P4Sync(Command, P4UserMap):\n         self.cloneExclude = []\n         self.useClientSpec = False\n         self.clientSpecDirs = None\n+        self.tempBranches = []\n+        self.tempBranchLocation = \"git-p4-tmp\"\n \n         if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n             self.syncWithOrigin = False\n@@ -2012,7 +2014,28 @@ class P4Sync(Command, P4UserMap):\n                             parent = self.initialParents[branch]\n                             del self.initialParents[branch]\n \n-                        self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n+                        parentFound = 0\n+                        if len(parent) > 0:\n+                            self.checkpoint()\n+                            for blob in read_pipe_lines(\"git rev-list --reverse --no-merges %s\" % parent):\n+                                blob = blob.strip()\n+                                tempBranch = self.tempBranchLocation + os.sep + \"%d-%s\" % (change, blob)\n+                                if self.verbose:\n+                                    print \"Creating temporary branch: \" + tempBranch\n+                                self.commit(description, filesForCommit, tempBranch, [branchPrefix], blob)\n+                                self.tempBranches.append(tempBranch)\n+                                self.checkpoint()\n+                                if len( read_pipe(\"git diff-tree %s %s\" % (blob, tempBranch)) ) == 0:\n+                                    parentFound = 1\n+                                    if self.verbose:\n+                                        print \"Found parent of %s in commit %s\" % (branch, blob)\n+                                    break\n+                        if parentFound:\n+                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)\n+                        else:\n+                            if self.verbose:\n+                                print \"Parent of %s not found. Committing into head of %s\" % (branch, parent)\n+                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n                 else:\n                     files = self.extractFilesFromCommit(description)\n                     self.commit(description, files, self.branch, self.depotPaths,\n@@ -2347,6 +2370,12 @@ class P4Sync(Command, P4UserMap):\n         self.gitOutput.close()\n         self.gitError.close()\n \n+        # Cleanup temporary branches created during import\n+        if self.tempBranches != []:\n+            for branch in self.tempBranches:\n+                os.remove(\".git\" + os.sep + branch)\n+            os.rmdir(\".git\" + os.sep + self.tempBranchLocation)\n+\n         return True\n \n class P4Rebase(Command):\n-- \n1.7.8.3\n"},{"id":"182588","messageId":"1326674360-2771-4-git-send-email-vitor.hda@gmail.com","threadId":"29372","inReplyTo":"1326674360-2771-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 3/3] git-p4: Add test case for complex branch import","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-16T00:39:20Z","receivedAt":"2012-01-16T00:39:20Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Check if branches created from old changelists are correctly imported.\n\nSigned-off-by: Vitor Antunes <vitor.hda@gmail.com>\n---\n t/t9801-git-p4-branch.sh |   83 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 83 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex a25f18d..0f10e00 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -226,6 +226,89 @@ test_expect_success 'git-p4 clone simple branches' '\n \t)\n '\n \n+# Create a complex branch structure in P4 depot to check if they are correctly\n+# cloned. The branches are created from older changelists to check if git-p4 is\n+# able to correctly detect them.\n+# The final expected structure is:\n+# `branch1\n+# | `- file1\n+# | `- file2 (updated)\n+# | `- file3\n+# `branch2\n+# | `- file1\n+# | `- file2\n+# `branch3\n+# | `- file1\n+# | `- file2 (updated)\n+# | `- file3\n+# `branch4\n+# | `- file1\n+# | `- file2\n+# `branch5\n+#   `- file1\n+#   `- file2\n+#   `- file3\n+test_expect_success 'git-p4 add complex branches' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tchangelist=$(p4 changes -m1 //depot/... | cut -d\" \" -f2) &&\n+\t\tchangelist=$((changelist - 5)) &&\n+\t\tp4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&\n+\t\tp4 submit -d \"branch4\" &&\n+\t\tchangelist=$((changelist + 2)) &&\n+\t\tp4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&\n+\t\tp4 submit -d \"branch5\" &&\n+\t\tcd \"$TRASH_DIRECTORY\"\n+\t)\n+'\n+\n+# Configure branches through git-config and clone them. git-p4 will only be able\n+# to clone the original structure if it is able to detect the origin changelist\n+# of each branch.\n+test_expect_success 'git-p4 clone complex branches' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\ttest_when_finished cleanup_git &&\n+\t\ttest_create_repo \"$git\" &&\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList branch1:branch2 &&\n+\t\tgit config --add git-p4.branchList branch1:branch3 &&\n+\t\tgit config --add git-p4.branchList branch1:branch4 &&\n+\t\tgit config --add git-p4.branchList branch1:branch5 &&\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/branch1 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest -f file3 &&\n+\t\tgrep -q update file2 &&\n+\t\tgit reset --hard p4/depot/branch2 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest ! -f file3 &&\n+\t\t! grep -q update file2 &&\n+\t\tgit reset --hard p4/depot/branch3 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest -f file3 &&\n+\t\tgrep -q update file2 &&\n+\t\tgit reset --hard p4/depot/branch4 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest ! -f file3 &&\n+\t\t! grep -q update file2 &&\n+\t\tgit reset --hard p4/depot/branch5 &&\n+\t\ttest -f file1 &&\n+\t\ttest -f file2 &&\n+\t\ttest -f file3 &&\n+\t\t! grep -q update file2 &&\n+\t\ttest ! -d .git/git-p4-tmp\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.7.8.3\n"},{"id":"182620","messageId":"20120116185738.GA21996@padd.com","threadId":"29372","inReplyTo":"1326674360-2771-3-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-16T18:57:38Z","receivedAt":"2012-01-16T18:57:38Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Mon, 16 Jan 2012 00:39 +0000:\n> To find out which is its parent the commit of the new branch is applied\n> sequentially to each blob of the parent branch from the newest to the\n> oldest. The first blob which results in a zero diff is considered the\n> parent commit. If none is found, then the commit is applied to the top\n> of the parent branch.\n> \n> A fast-import \"checkpoint\" call is required for each comparison because\n> diff-tree is only able to work with blobs on disk. But most of these\n> commits will not be part of the final imported tree, making fast-import\n> fail. To avoid this, the temporary branches are tracked and then removed\n> at the end of the import process.\n\nThis looks much better without the need for \"--force\".  It'll be\ngreat to fix this major branch detection problem.  Can you make a\ncouple of further minor changes?\n\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> @@ -2012,7 +2014,28 @@ class P4Sync(Command, P4UserMap):\n> -                        self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n> +                        parentFound = 0\n> +                        if len(parent) > 0:\n> +                            self.checkpoint()\n> +                            for blob in read_pipe_lines(\"git rev-list --reverse --no-merges %s\" % parent):\n> +                                blob = blob.strip()\n> +                                tempBranch = self.tempBranchLocation + os.sep + \"%d-%s\" % (change, blob)\n> +                                if self.verbose:\n> +                                    print \"Creating temporary branch: \" + tempBranch\n> +                                self.commit(description, filesForCommit, tempBranch, [branchPrefix], blob)\n> +                                self.tempBranches.append(tempBranch)\n> +                                self.checkpoint()\n> +                                if len( read_pipe(\"git diff-tree %s %s\" % (blob, tempBranch)) ) == 0:\n> +                                    parentFound = 1\n> +                                    if self.verbose:\n> +                                        print \"Found parent of %s in commit %s\" % (branch, blob)\n> +                                    break\n> +                        if parentFound:\n> +                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)\n> +                        else:\n> +                            if self.verbose:\n> +                                print \"Parent of %s not found. Committing into head of %s\" % (branch, parent)\n> +                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n\n1.  Move the tempBranch commit outside of the \"for blob\" loop.\n    It can have no parent, and the diff-tree will still tell you\n    if you found the same contents.  Instead of a ref for\n    each blob inspected for each change, you'll just have one ref\n    per change.  Only one checkpoint() after the tempBranch\n    commit should be needed.\n\n2.  Nit.  parentFound is boolean, use True/False, not 1/0.\n\n> @@ -2347,6 +2370,12 @@ class P4Sync(Command, P4UserMap):\n> +        # Cleanup temporary branches created during import\n> +        if self.tempBranches != []:\n> +            for branch in self.tempBranches:\n> +                os.remove(\".git\" + os.sep + branch)\n> +            os.rmdir(\".git\" + os.sep + self.tempBranchLocation)\n> +\n\n3.  Deleting refs should probably use \"git update-ref -d\"\n    just in case GIT_DIR is not \".git\".  I think you could just\n    leave the \"git-p4-tmp\" directory around, but use\n    os.environ[\"GIT_DIR\"] instead of \".git\" if you want to\n    delete it.\n\n4.  Paths are best manipulated with os.path.join(dir, file), to handle\n    weirdnesses like drive letters.\n\nEventually if the fast-import protocol learns to delete the refs\nit creates, we can clean up a bit more nicely.  I think there was\nagreement this was a good idea, just needs someone to do it\nsometime.\n\n\t\t-- Pete\n"},{"id":"182624","messageId":"20120116191254.GB21996@padd.com","threadId":"29372","inReplyTo":"1326674360-2771-4-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH 3/3] git-p4: Add test case for complex branch import","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-16T19:12:54Z","receivedAt":"2012-01-16T19:12:54Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Mon, 16 Jan 2012 00:39 +0000:\n> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n> +test_expect_success 'git-p4 add complex branches' '\n> +\ttest_when_finished cleanup_git &&\n> +\ttest_create_repo \"$git\" &&\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\tchangelist=$(p4 changes -m1 //depot/... | cut -d\" \" -f2) &&\n> +\t\tchangelist=$((changelist - 5)) &&\n> +\t\tp4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&\n> +\t\tp4 submit -d \"branch4\" &&\n> +\t\tchangelist=$((changelist + 2)) &&\n> +\t\tp4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&\n> +\t\tp4 submit -d \"branch5\" &&\n> +\t\tcd \"$TRASH_DIRECTORY\"\n> +\t)\n> +'\n\nSorry: I think I wanted the \"$\"s removed from inside $((..)).\nTurns out that some shells don't grok that.  The above should be:\n\n\tchangelist=$(($changelist - 5)) &&\n\nYou can drop the last cd to $TRASH_DIRECTORY since you're inside\na subshell.  (Nice addition of the subshells.)\n\n> +\n> +# Configure branches through git-config and clone them. git-p4 will only be able\n> +# to clone the original structure if it is able to detect the origin changelist\n> +# of each branch.\n> +test_expect_success 'git-p4 clone complex branches' '\n> +\ttest_when_finished cleanup_git &&\n> +\ttest_create_repo \"$git\" &&\n> +\t(\n> +\t\ttest_when_finished cleanup_git &&\n> +\t\ttest_create_repo \"$git\" &&\n\nThese two lines can go; you already did it outside the subshell.\n\n> +\t\tcd \"$git\" &&\n> +\t\tgit config git-p4.branchList branch1:branch2 &&\n> +\t\tgit config --add git-p4.branchList branch1:branch3 &&\n> +\t\tgit config --add git-p4.branchList branch1:branch4 &&\n> +\t\tgit config --add git-p4.branchList branch1:branch5 &&\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/branch1 &&\n> +\t\ttest -f file1 &&\n> +\t\ttest -f file2 &&\n> +\t\ttest -f file3 &&\n\nThere are preferred functions for these tests, I learned recently:\n\n\ttest_path_is_file file1 &&\n\n> +\t\tgrep -q update file2 &&\n> +\t\tgit reset --hard p4/depot/branch2 &&\n> +\t\ttest -f file1 &&\n> +\t\ttest -f file2 &&\n> +\t\ttest ! -f file3 &&\n\nSimilarly\n\n\ttest_path_is_missing file3 &&\n\n> +\t\t! grep -q update file2 &&\n> +\t\tgit reset --hard p4/depot/branch3 &&\n> +\t\ttest -f file1 &&\n> +\t\ttest -f file2 &&\n> +\t\ttest -f file3 &&\n> +\t\tgrep -q update file2 &&\n> +\t\tgit reset --hard p4/depot/branch4 &&\n> +\t\ttest -f file1 &&\n> +\t\ttest -f file2 &&\n> +\t\ttest ! -f file3 &&\n> +\t\t! grep -q update file2 &&\n> +\t\tgit reset --hard p4/depot/branch5 &&\n> +\t\ttest -f file1 &&\n> +\t\ttest -f file2 &&\n> +\t\ttest -f file3 &&\n> +\t\t! grep -q update file2 &&\n> +\t\ttest ! -d .git/git-p4-tmp\n> +\t)\n> +'\n"},{"id":"182654","messageId":"CAOpHH-UkyK-c_AHUOPbQQmW9cQQypDvirMR0Jb7vTGSQF7RZpw@mail.gmail.com","threadId":"29372","inReplyTo":"20120116185738.GA21996@padd.com","subject":"Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-16T23:41:39Z","receivedAt":"2012-01-16T23:41:39Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:\n> This looks much better without the need for \"--force\".  It'll be\n> great to fix this major branch detection problem.  Can you make a\n> couple of further minor changes?\n\nOf course I can :)\n\n>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n>> @@ -2012,7 +2014,28 @@ class P4Sync(Command, P4UserMap):\n>> -                        self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n>> +                        parentFound = 0\n>> +                        if len(parent) > 0:\n>> +                            self.checkpoint()\n>> +                            for blob in read_pipe_lines(\"git rev-list --reverse --no-merges %s\" % parent):\n>> +                                blob = blob.strip()\n>> +                                tempBranch = self.tempBranchLocation + os.sep + \"%d-%s\" % (change, blob)\n>> +                                if self.verbose:\n>> +                                    print \"Creating temporary branch: \" + tempBranch\n>> +                                self.commit(description, filesForCommit, tempBranch, [branchPrefix], blob)\n>> +                                self.tempBranches.append(tempBranch)\n>> +                                self.checkpoint()\n>> +                                if len( read_pipe(\"git diff-tree %s %s\" % (blob, tempBranch)) ) == 0:\n>> +                                    parentFound = 1\n>> +                                    if self.verbose:\n>> +                                        print \"Found parent of %s in commit %s\" % (branch, blob)\n>> +                                    break\n>> +                        if parentFound:\n>> +                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)\n>> +                        else:\n>> +                            if self.verbose:\n>> +                                print \"Parent of %s not found. Committing into head of %s\" % (branch, parent)\n>> +                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)\n>\n> 1.  Move the tempBranch commit outside of the \"for blob\" loop.\n>    It can have no parent, and the diff-tree will still tell you\n>    if you found the same contents.  Instead of a ref for\n>    each blob inspected for each change, you'll just have one ref\n>    per change.  Only one checkpoint() after the tempBranch\n>    commit should be needed.\n\nYou're right. Completely oversaw that. Will improve the code\naccordingly.\n\n> 2.  Nit.  parentFound is boolean, use True/False, not 1/0.\n\nThat was not a nice thing to do... thanks for noticing :)\n\n>> @@ -2347,6 +2370,12 @@ class P4Sync(Command, P4UserMap):\n>> +        # Cleanup temporary branches created during import\n>> +        if self.tempBranches != []:\n>> +            for branch in self.tempBranches:\n>> +                os.remove(\".git\" + os.sep + branch)\n>> +            os.rmdir(\".git\" + os.sep + self.tempBranchLocation)\n>> +\n>\n> 3.  Deleting refs should probably use \"git update-ref -d\"\n>    just in case GIT_DIR is not \".git\".  I think you could just\n>    leave the \"git-p4-tmp\" directory around, but use\n>    os.environ[\"GIT_DIR\"] instead of \".git\" if you want to\n>    delete it.\n\nWill use os.environ.get, which can be configured to return \".git\" if\n$GIT_DIR is not defined. Is this ok?\n\n> 4.  Paths are best manipulated with os.path.join(dir, file), to handle\n>    weirdnesses like drive letters.\n\nPerfect. I was completely unaware of that method. Thanks for the tip.\n\n> Eventually if the fast-import protocol learns to delete the refs\n> it creates, we can clean up a bit more nicely.  I think there was\n> agreement this was a good idea, just needs someone to do it\n> sometime.\n\nOne can always hope ;)\n\nThanks,\nVitor\n"},{"id":"182656","messageId":"CAOpHH-XUj7tF4O_kXfxq2e9Y4VmybNLCqGku_-9H1X+c7v=xwQ@mail.gmail.com","threadId":"29372","inReplyTo":"CAOpHH-UkyK-c_AHUOPbQQmW9cQQypDvirMR0Jb7vTGSQF7RZpw@mail.gmail.com","subject":"Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-17T00:10:17Z","receivedAt":"2012-01-17T00:10:17Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Mon, Jan 16, 2012 at 11:41 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:\n> On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:\n>> 1.  Move the tempBranch commit outside of the \"for blob\" loop.\n>>    It can have no parent, and the diff-tree will still tell you\n>>    if you found the same contents.  Instead of a ref for\n>>    each blob inspected for each change, you'll just have one ref\n>>    per change.  Only one checkpoint() after the tempBranch\n>>    commit should be needed.\n>\n> You're right. Completely oversaw that. Will improve the code\n> accordingly.\n\nApparently I did not oversee it. Assume you have added a new file to\nHEAD of parent branch, but you branched from a previous commit. When the\nnew branch is committed over HEAD the new file will, incorrectly, be\npart of it and diff-tree will not work as expected.\n\nI should avoid taking 6 months to submit a patch to avoid forgetting why\nI did what I did :)\n\nVitor\n"},{"id":"182701","messageId":"20120117221830.GA366@padd.com","threadId":"29372","inReplyTo":"CAOpHH-XUj7tF4O_kXfxq2e9Y4VmybNLCqGku_-9H1X+c7v=xwQ@mail.gmail.com","subject":"Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-01-17T22:18:30Z","receivedAt":"2012-01-17T22:18:30Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"vitor.hda@gmail.com wrote on Tue, 17 Jan 2012 00:10 +0000:\n> On Mon, Jan 16, 2012 at 11:41 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:\n> > On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:\n> >> 1.  Move the tempBranch commit outside of the \"for blob\" loop.\n> >>    It can have no parent, and the diff-tree will still tell you\n> >>    if you found the same contents.  Instead of a ref for\n> >>    each blob inspected for each change, you'll just have one ref\n> >>    per change.  Only one checkpoint() after the tempBranch\n> >>    commit should be needed.\n> >\n> > You're right. Completely oversaw that. Will improve the code\n> > accordingly.\n> \n> Apparently I did not oversee it. Assume you have added a new file to\n> HEAD of parent branch, but you branched from a previous commit. When the\n> new branch is committed over HEAD the new file will, incorrectly, be\n> part of it and diff-tree will not work as expected.\n\nI don't get it.  This algorithm works on the fact that a \"branch\"\nin p4 creates a new change that looks exactly like a previous\nchange.\n\nThe git-p4 sync step, when it detects a branch, starts by saving\nthe change in a commit with parent = null, so it is its own new\nbranch, an orphan, with no parents.\n\nNow the task is to find some commit that has an identical tree to\nthis temporary one.  You walk back all known p4 commits to try to\nfind one that is the same.  It doesn't matter if any of those p4\ncommits have other commits on top of them.\n\nAt each step in the backward walk, the comparison is against the\nunchanged orphan commit.\n\nAn ascii-art picture might help me.  Or even a test case.\n\n> I should avoid taking 6 months to submit a patch to avoid forgetting why\n> I did what I did :)\n\nYeah, and now you have to explain it all over to me again too.  :)\n\n\t\t-- Pete\n"},{"id":"182707","messageId":"CAOpHH-XCdBOXAAgSGz81oyaN0i0X72=scOvfrDwN31q59c_9QA@mail.gmail.com","threadId":"29372","inReplyTo":"20120117221830.GA366@padd.com","subject":"Re: [PATCH 2/3] git-p4: Search for parent commit on branch creation","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2012-01-17T23:43:18Z","receivedAt":"2012-01-17T23:43:18Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Tue, Jan 17, 2012 at 10:18 PM, Pete Wyckoff <pw@padd.com> wrote:\n> vitor.hda@gmail.com wrote on Tue, 17 Jan 2012 00:10 +0000:\n>> On Mon, Jan 16, 2012 at 11:41 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:\n>> > On Mon, Jan 16, 2012 at 6:57 PM, Pete Wyckoff <pw@padd.com> wrote:\n>> >> 1.  Move the tempBranch commit outside of the \"for blob\" loop.\n>> >>    It can have no parent, and the diff-tree will still tell you\n>> >>    if you found the same contents.  Instead of a ref for\n>> >>    each blob inspected for each change, you'll just have one ref\n>> >>    per change.  Only one checkpoint() after the tempBranch\n>> >>    commit should be needed.\n>> >\n>> > You're right. Completely oversaw that. Will improve the code\n>> > accordingly.\n>>\n>> Apparently I did not oversee it. Assume you have added a new file to\n>> HEAD of parent branch, but you branched from a previous commit. When the\n>> new branch is committed over HEAD the new file will, incorrectly, be\n>> part of it and diff-tree will not work as expected.\n>\n> I don't get it.  This algorithm works on the fact that a \"branch\"\n> in p4 creates a new change that looks exactly like a previous\n> change.\n>\n> The git-p4 sync step, when it detects a branch, starts by saving\n> the change in a commit with parent = null, so it is its own new\n> branch, an orphan, with no parents.\n>\n> Now the task is to find some commit that has an identical tree to\n> this temporary one.  You walk back all known p4 commits to try to\n> find one that is the same.  It doesn't matter if any of those p4\n> commits have other commits on top of them.\n>\n> At each step in the backward walk, the comparison is against the\n> unchanged orphan commit.\n>\n> An ascii-art picture might help me.  Or even a test case.\n\nI can see now that I did not understand the \"no parent\" in your original\nreply. What you just explained makes sense. So, let me try that and I'll\nupdate you tomorrow.\n\nVitor\n"}]}