{"thread":{"id":"9661","subject":"[StGIT PATCH 2/6] Leave working dir and index alone after failed (conflicting) push","startedAt":"2007-08-26T20:42:29Z","lastAt":"2007-08-26T20:43:05Z","messageCount":7,"participants":["Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"51597","messageId":"20070826203745.16700.5655.stgit@yoghurt","threadId":"9661","inReplyTo":null,"subject":"[StGIT PATCH 0/6] David's conflict series using my subprocess stuff","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:29Z","receivedAt":"2007-08-26T20:42:29Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This is David's conflict series, rebased on top of my subprocess\ncalling series. There was one place where it used to have to call\npopen2.Popen3 directly, since none of our existing ten subprocess\nfunctions could do that particular job. But the new subprocess\nmachinery can. (This was the original motivation for that refactoring,\nby the way.)\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51596","messageId":"20070826204237.16700.21445.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 1/6] Split git.merge into two functions","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:37Z","receivedAt":"2007-08-26T20:42:37Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"From: David Kågedal <davidk@lysator.liu.se>\n\nThis only prepares for later simplifications.\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/commands/pick.py |    2 +\n stgit/git.py           |   92 +++++++++++++++++++++++++++++++++++++-----------\n stgit/stack.py         |    2 +\n 3 files changed, 73 insertions(+), 23 deletions(-)\n\ndiff --git a/stgit/commands/pick.py b/stgit/commands/pick.py\nindex 1c3ef11..871879c 100644\n--- a/stgit/commands/pick.py\n+++ b/stgit/commands/pick.py\n@@ -99,7 +99,7 @@ def func(parser, options, args):\n \n         # try a direct git-apply first\n         if not git.apply_diff(bottom, top):\n-            git.merge(bottom, git.get_head(), top, recursive = True)\n+            git.merge_recursive(bottom, git.get_head(), top)\n \n         out.done()\n     elif options.update:\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 7962cdb..87551ee 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -634,36 +634,86 @@ def apply_diff(rev1, rev2, check_index = True, files = None):\n \n     return True\n \n-def merge(base, head1, head2, recursive = False):\n+stages_re = re.compile('^([0-7]+) ([0-9a-f]{40}) ([1-3])\\t(.*)$', re.S)\n+\n+def merge_recursive(base, head1, head2):\n     \"\"\"Perform a 3-way merge between base, head1 and head2 into the\n     local tree\n     \"\"\"\n     refresh_index()\n \n     err_output = None\n-    if recursive:\n-        # this operation tracks renames but it is slower (used in\n-        # general when pushing or picking patches)\n-        try:\n-            # discard output to mask the verbose prints of the tool\n-            GRun('git-merge-recursive', base, '--', head1, head2\n-                 ).discard_output()\n-        except GitRunException, ex:\n-            err_output = str(ex)\n-            pass\n-    else:\n-        # the fast case where we don't track renames (used when the\n-        # distance between base and heads is small, i.e. folding or\n-        # synchronising patches)\n-        try:\n-            GRun('git-read-tree', '-u', '-m', '--aggressive',\n-                 base, head1, head2).run()\n-        except GitRunException:\n-            raise GitException, 'git-read-tree failed (local changes maybe?)'\n+    # this operation tracks renames but it is slower (used in\n+    # general when pushing or picking patches)\n+    try:\n+        # discard output to mask the verbose prints of the tool\n+        GRun('git-merge-recursive', base, '--', head1, head2).discard_output()\n+    except GitRunException, ex:\n+        err_output = str(ex)\n+        pass\n+\n+    # check the index for unmerged entries\n+    files = {}\n+\n+    for line in GRun('git-ls-files', '--unmerged', '--stage', '-z'\n+                     ).raw_output().split('\\0'):\n+        if not line:\n+            continue\n+\n+        mode, hash, stage, path = stages_re.findall(line)[0]\n+\n+        if not path in files:\n+            files[path] = {}\n+            files[path]['1'] = ('', '')\n+            files[path]['2'] = ('', '')\n+            files[path]['3'] = ('', '')\n+\n+        files[path][stage] = (mode, hash)\n+\n+    if err_output and not files:\n+        # if no unmerged files, there was probably a different type of\n+        # error and we have to abort the merge\n+        raise GitException, err_output\n+\n+    # merge the unmerged files\n+    errors = False\n+    for path in files:\n+        # remove additional files that might be generated for some\n+        # newer versions of GIT\n+        for suffix in [base, head1, head2]:\n+            if not suffix:\n+                continue\n+            fname = path + '~' + suffix\n+            if os.path.exists(fname):\n+                os.remove(fname)\n+\n+        stages = files[path]\n+        if gitmergeonefile.merge(stages['1'][1], stages['2'][1],\n+                                 stages['3'][1], path, stages['1'][0],\n+                                 stages['2'][0], stages['3'][0]) != 0:\n+            errors = True\n+\n+    if errors:\n+        raise GitException, 'GIT index merging failed (possible conflicts)'\n+\n+def merge(base, head1, head2):\n+    \"\"\"Perform a 3-way merge between base, head1 and head2 into the\n+    local tree\n+    \"\"\"\n+    refresh_index()\n+\n+    err_output = None\n+    # the fast case where we don't track renames (used when the\n+    # distance between base and heads is small, i.e. folding or\n+    # synchronising patches)\n+    try:\n+        GRun('git-read-tree', '-u', '-m', '--aggressive', base, head1, head2\n+             ).run()\n+    except GitRunException:\n+        raise GitException, 'git-read-tree failed (local changes maybe?)'\n \n     # check the index for unmerged entries\n     files = {}\n-    stages_re = re.compile('^([0-7]+) ([0-9a-f]{40}) ([1-3])\\t(.*)$', re.S)\n \n     for line in GRun('git-ls-files', '--unmerged', '--stage', '-z'\n                      ).raw_output().split('\\0'):\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex 12c5091..a82392f 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -1066,7 +1066,7 @@ class Series(PatchSet):\n \n                 # merge can fail but the patch needs to be pushed\n                 try:\n-                    git.merge(bottom, head, top, recursive = True)\n+                    git.merge_recursive(bottom, head, top)\n                 except git.GitException, ex:\n                     out.error('The merge failed during \"push\".',\n                               'Use \"refresh\" after fixing the conflicts or'\n"},{"id":"51595","messageId":"20070826204242.16700.60432.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 2/6] Leave working dir and index alone after failed (conflicting) push","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:43Z","receivedAt":"2007-08-26T20:42:43Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"From: David Kågedal <davidk@lysator.liu.se>\n\nThis leaves the index and working tree in the state that merge-recursive\nleft it, with unmerged files in different stages, and the non-conflicting\nchanges in the index.\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/git.py   |   20 +-------------------\n stgit/stack.py |   18 +++++++++++++-----\n 2 files changed, 14 insertions(+), 24 deletions(-)\n\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 87551ee..82cb211 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -675,25 +675,7 @@ def merge_recursive(base, head1, head2):\n         # error and we have to abort the merge\n         raise GitException, err_output\n \n-    # merge the unmerged files\n-    errors = False\n-    for path in files:\n-        # remove additional files that might be generated for some\n-        # newer versions of GIT\n-        for suffix in [base, head1, head2]:\n-            if not suffix:\n-                continue\n-            fname = path + '~' + suffix\n-            if os.path.exists(fname):\n-                os.remove(fname)\n-\n-        stages = files[path]\n-        if gitmergeonefile.merge(stages['1'][1], stages['2'][1],\n-                                 stages['3'][1], path, stages['1'][0],\n-                                 stages['2'][0], stages['3'][0]) != 0:\n-            errors = True\n-\n-    if errors:\n+    if files:\n         raise GitException, 'GIT index merging failed (possible conflicts)'\n \n def merge(base, head1, head2):\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex a82392f..eb0114e 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -734,6 +734,7 @@ class Series(PatchSet):\n         config.unset(self.format_version_key())\n \n     def refresh_patch(self, files = None, message = None, edit = False,\n+                      empty = False,\n                       show_patch = False,\n                       cache_update = True,\n                       author_name = None, author_email = None,\n@@ -783,9 +784,16 @@ class Series(PatchSet):\n \n         bottom = patch.get_bottom()\n \n+        if empty:\n+            tree_id = git.get_commit(bottom).get_tree()\n+        else:\n+            tree_id = None\n+\n         commit_id = git.commit(files = files,\n                                message = descr, parents = [bottom],\n                                cache_update = cache_update,\n+                               tree_id = tree_id,\n+                               set_head = True,\n                                allowempty = True,\n                                author_name = author_name,\n                                author_email = author_email,\n@@ -1088,11 +1096,11 @@ class Series(PatchSet):\n                     log = 'push'\n                 self.refresh_patch(cache_update = False, log = log)\n             else:\n-                # we store the correctly merged files only for\n-                # tracking the conflict history. Note that the\n-                # git.merge() operations should always leave the index\n-                # in a valid state (i.e. only stage 0 files)\n-                self.refresh_patch(cache_update = False, log = 'push(c)')\n+                # We make the patch empty, with the merged state in\n+                # the working tree and index just like after a failed\n+                # git merge.\n+                self.refresh_patch(cache_update = False, empty = True,\n+                                   log = 'push(c)')\n                 raise StackException, str(ex)\n \n         return modified\n"},{"id":"51600","messageId":"20070826204248.16700.30974.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 3/6] Added a test case to check what happens when push finds a conflict","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:48Z","receivedAt":"2007-08-26T20:42:48Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"From: David Kågedal <davidk@lysator.liu.se>\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n t/t1203-push-conflict.sh |   64 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 64 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1203-push-conflict.sh b/t/t1203-push-conflict.sh\nnew file mode 100755\nindex 0000000..57fb477\n--- /dev/null\n+++ b/t/t1203-push-conflict.sh\n@@ -0,0 +1,64 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 David Kågedal\n+#\n+\n+test_description='Exercise push conflicts.\n+\n+Test that the index has no modifications after a push with conflicts.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \\\n+\t'Initialize the StGIT repository' \\\n+\t'stg init\n+'\n+\n+test_expect_success \\\n+\t'Create the first patch' \\\n+\t'\n+\tstg new foo -m foo &&\n+\techo foo > test &&\n+\techo fie > test2 &&\n+\tstg add test test2 &&\n+\tstg refresh &&\n+        stg pop\n+\t'\n+\n+test_expect_success \\\n+\t'Create the second patch' \\\n+\t'\n+\tstg new bar -m bar &&\n+\techo bar > test &&\n+\tstg add test &&\n+\tstg refresh\n+\t'\n+\n+test_expect_failure \\\n+\t'Push the first patch with conflict' \\\n+\t'\n+\tstg push foo\n+\t'\n+\n+test_expect_failure \\\n+\t'Show the, now empty, first patch' \\\n+\t'\n+\tstg show foo | grep -q -e \"^diff \"\n+\t'\n+\n+test_expect_success \\\n+\t'Check that the index has the non-conflict updates' \\\n+\t'\n+\tgit diff --cached --stat | grep -q -e \"^ test2 | *1 \"\n+\t'\n+\n+test_expect_success \\\n+\t'Resolve the conflict' \\\n+\t'\n+\techo resolved > test &&\n+\tgit add test &&\n+\tstg refresh\n+\t'\n+\n+test_done\n"},{"id":"51598","messageId":"20070826204254.16700.14590.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 4/6] Simplify merge_recursive","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:54Z","receivedAt":"2007-08-26T20:42:54Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"From: David Kågedal <davidk@lysator.liu.se>\n\nListing the unmerged files is unnecessary, since the information\nisn't really used anyway. Just note if the merge failed or succeeded.\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/git.py |   32 +-------------------------------\n 1 files changed, 1 insertions(+), 31 deletions(-)\n\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 82cb211..173cc4b 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -642,40 +642,10 @@ def merge_recursive(base, head1, head2):\n     \"\"\"\n     refresh_index()\n \n-    err_output = None\n-    # this operation tracks renames but it is slower (used in\n-    # general when pushing or picking patches)\n     try:\n         # discard output to mask the verbose prints of the tool\n         GRun('git-merge-recursive', base, '--', head1, head2).discard_output()\n-    except GitRunException, ex:\n-        err_output = str(ex)\n-        pass\n-\n-    # check the index for unmerged entries\n-    files = {}\n-\n-    for line in GRun('git-ls-files', '--unmerged', '--stage', '-z'\n-                     ).raw_output().split('\\0'):\n-        if not line:\n-            continue\n-\n-        mode, hash, stage, path = stages_re.findall(line)[0]\n-\n-        if not path in files:\n-            files[path] = {}\n-            files[path]['1'] = ('', '')\n-            files[path]['2'] = ('', '')\n-            files[path]['3'] = ('', '')\n-\n-        files[path][stage] = (mode, hash)\n-\n-    if err_output and not files:\n-        # if no unmerged files, there was probably a different type of\n-        # error and we have to abort the merge\n-        raise GitException, err_output\n-\n-    if files:\n+    except GitRunException:\n         raise GitException, 'GIT index merging failed (possible conflicts)'\n \n def merge(base, head1, head2):\n"},{"id":"51599","messageId":"20070826204259.16700.55770.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 5/6] Use the output from merge-recursive to list conflicts","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:42:59Z","receivedAt":"2007-08-26T20:42:59Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"From: David Kågedal <davidk@lysator.liu.se>\n\nmerge-recursive already has useful information about what the conflicts\nwere, so we reuse that when pushing.\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/git.py   |   22 +++++++++++++++++-----\n stgit/stack.py |    2 ++\n 2 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 173cc4b..a185f19 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -39,6 +39,14 @@ class GitRunException(GitException):\n class GRun(Run):\n     exc = GitRunException\n \n+class GitConflictException(GitException):\n+    def __init__(self, conflicts):\n+        GitException.__init__(self)\n+        self.conflicts = conflicts\n+    def __str__(self):\n+        return \"%d conflicts\" % len(self.conflicts)\n+    def list(self):\n+        out.info(*self.conflicts)\n \n #\n # Classes\n@@ -642,11 +650,15 @@ def merge_recursive(base, head1, head2):\n     \"\"\"\n     refresh_index()\n \n-    try:\n-        # discard output to mask the verbose prints of the tool\n-        GRun('git-merge-recursive', base, '--', head1, head2).discard_output()\n-    except GitRunException:\n-        raise GitException, 'GIT index merging failed (possible conflicts)'\n+    p = GRun('git-merge-recursive', base, '--', head1, head2).returns([0, 1])\n+    output = p.output_lines()\n+    if p.exitcode == 0:\n+        # No problems\n+        return\n+    else: # exitcode == 1\n+        # There were conflicts\n+        conflicts = [l for l in output if l.startswith('CONFLICT')]\n+        raise GitConflictException(conflicts)\n \n def merge(base, head1, head2):\n     \"\"\"Perform a 3-way merge between base, head1 and head2 into the\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex eb0114e..d2ca0e2 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -1075,6 +1075,8 @@ class Series(PatchSet):\n                 # merge can fail but the patch needs to be pushed\n                 try:\n                     git.merge_recursive(bottom, head, top)\n+                except git.GitConflictException, ex:\n+                    ex.list()\n                 except git.GitException, ex:\n                     out.error('The merge failed during \"push\".',\n                               'Use \"refresh\" after fixing the conflicts or'\n"},{"id":"51601","messageId":"20070826204305.16700.16459.stgit@yoghurt","threadId":"9661","inReplyTo":"20070826203745.16700.5655.stgit@yoghurt","subject":"[StGIT PATCH 6/6] Better error message if merge fails","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:43:05Z","receivedAt":"2007-08-26T20:43:05Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This message is no longer printed in case of conflicts, just in case\nthe merge really failed; so don't talk about conflicts in the error\nmessage.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/stack.py |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex d2ca0e2..080231f 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -1079,8 +1079,7 @@ class Series(PatchSet):\n                     ex.list()\n                 except git.GitException, ex:\n                     out.error('The merge failed during \"push\".',\n-                              'Use \"refresh\" after fixing the conflicts or'\n-                              ' revert the operation with \"push --undo\".')\n+                              'Revert the operation with \"push --undo\".')\n \n         append_string(self.__applied_file, name)\n \n"}]}