Re: [StGit PATCH 2/6] Don't try to merge files that merge-recursive already merged

Karl Hasselström [off-list ref] writes:

On 2007-08-20 10:11:59 +0200, David Kågedal wrote:

quoted

This avoid duplicating work, but also changes the resulting index
state so that the conflicts are left in the index in the three
stages.

After this patch, t1400-patch-history fails:

I think that the push(c) commits will never happen with my patches, so
the test case needs an update.

*   ok 1: Initialize the StGIT repository
*   ok 2: Create the first patch
*   ok 3: Create the second patch
*   ok 4: Check the "new" and "refresh" logs
*   ok 5: Check the log annotation
*   ok 6: Check the "push" log
*   ok 7: Check the "push(f)" log
*   ok 8: Check the "push(m)" log
* FAIL 9: Check the "push(c)" log

                echo bar > test && stg refresh &&
                stg pop &&
                echo foo > test && stg refresh &&
                ! stg push &&
                stg log --full | grep -q -e "^push(c) "

*   ok 10: Check the push "undo" log
*   ok 11: Check the refresh "undo" log
* failed 1 among 11 test(s)

I'll see if I can figure out what's wrong.

-- 
David Kågedal

On 2007-08-22 11:37:03 +0200, David Kågedal wrote:

Karl Hasselström [off-list ref] writes:

quoted

On 2007-08-20 10:11:59 +0200, David Kågedal wrote:

quoted

This avoid duplicating work, but also changes the resulting
index state so that the conflicts are left in the index in the
three stages.

After this patch, t1400-patch-history fails:

I think that the push(c) commits will never happen with my patches,
so the test case needs an update.

Actually, the failure vanishes once the next patch in the series is
applied. I thought this was a good thing, but if they're never
supposed to happen, maybe the test is still broken, just not in a way
that's detected?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

On 2007-08-22 11:44:13 +0200, Karl Hasselström wrote:

On 2007-08-22 11:37:03 +0200, David Kågedal wrote:

quoted

I think that the push(c) commits will never happen with my
patches, so the test case needs an update.

Actually, the failure vanishes once the next patch in the series is
applied. I thought this was a good thing, but if they're never
supposed to happen, maybe the test is still broken, just not in a
way that's detected?

No, after path 2 we don't get a push(c) entry, and the test fails, but
after patch 3 we get it again and the test succeeds.

Looking at the code (the string "push(c)" occurs exactly once, so it's
easy to find), it seems the log stuff continues to work as expected
after patch 3: it writes a "push(c)" entry when the patch becomes
empty due to conflicts, and its entire contents are dumped in the wd
(or the index+wd with your patches). So it is just patch 2 that needs
fixing -- and I suspect the fix might be to merge it with patch 3.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

From: David Kågedal <redacted>

This leaves the index and working tree in the state that merge-recursive
left it, with unmerged files in different stages, and the non-conflicting
changes in the index.

Signed-off-by: David Kågedal <redacted>
Signed-off-by: Karl Hasselström <redacted>

---

This is David's patch 2 and 3 clumped together, so that there is no
intermediate stage where the test suite fails.

 stgit/git.py   |   20 +-------------------
 stgit/stack.py |   18 +++++++++++++-----
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/stgit/git.py b/stgit/git.py
index 170c5d1..676314a 100644
--- a/stgit/git.py
+++ b/stgit/git.py

@@ -723,25 +723,7 @@ def merge_recursive(base, head1, head2):
         # error and we have to abort the merge
         raise GitException, err_output
 
-    # merge the unmerged files
-    errors = False
-    for path in files:
-        # remove additional files that might be generated for some
-        # newer versions of GIT
-        for suffix in [base, head1, head2]:
-            if not suffix:
-                continue
-            fname = path + '~' + suffix
-            if os.path.exists(fname):
-                os.remove(fname)
-
-        stages = files[path]
-        if gitmergeonefile.merge(stages['1'][1], stages['2'][1],
-                                 stages['3'][1], path, stages['1'][0],
-                                 stages['2'][0], stages['3'][0]) != 0:
-            errors = True
-
-    if errors:
+    if files:
         raise GitException, 'GIT index merging failed (possible conflicts)'
 
 def merge(base, head1, head2):

diff --git a/stgit/stack.py b/stgit/stack.py
index 95f20f1..123efac 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py

@@ -732,6 +732,7 @@ class Series(PatchSet):
         config.unset(self.format_version_key())
 
     def refresh_patch(self, files = None, message = None, edit = False,
+                      empty = False,
                       show_patch = False,
                       cache_update = True,
                       author_name = None, author_email = None,

@@ -781,9 +782,16 @@ class Series(PatchSet):
 
         bottom = patch.get_bottom()
 
+        if empty:
+            tree_id = git.get_commit(bottom).get_tree()
+        else:
+            tree_id = None
+
         commit_id = git.commit(files = files,
                                message = descr, parents = [bottom],
                                cache_update = cache_update,
+                               tree_id = tree_id,
+                               set_head = True,
                                allowempty = True,
                                author_name = author_name,
                                author_email = author_email,

@@ -1086,11 +1094,11 @@ class Series(PatchSet):
                     log = 'push'
                 self.refresh_patch(cache_update = False, log = log)
             else:
-                # we store the correctly merged files only for
-                # tracking the conflict history. Note that the
-                # git.merge() operations should always leave the index
-                # in a valid state (i.e. only stage 0 files)
-                self.refresh_patch(cache_update = False, log = 'push(c)')
+                # We make the patch empty, with the merged state in
+                # the working tree and index just like after a failed
+                # git merge.
+                self.refresh_patch(cache_update = False, empty = True,
+                                   log = 'push(c)')
                 raise StackException, str(ex)
 
         return modified