Re: [StGit PATCH] Add the --merged option to goto

[Catalin Marinas <catalin.marinas@gmail.com>]

2009/3/26 Karl Hasselström <kha@treskal.com>:
> On 2009-03-25 10:24:13 +0000, Catalin Marinas wrote:
>
>> BTW, why don't we keep the tree information directly in the Index
>> object? Since this object is modified only via its own interface, it
>> can do all the checks and avoid the managing of temp_index_tree in
>> the Transaction object.
>
> I guess that might be a good idea -- it should be doable without any
> extra overhead for users that don't want it.

I tried but gave up quickly. The IndexAndWorktree class also dirties
the Index with the merge operations, so it is not worth the hassle.

That's the updated patch:

    Add the --merged option to goto

    This patch adds support for checking which patches were already merged
    upstream. This checking is done by trying to reverse-apply the patches
    in the index before pushing them onto the stack.

    Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

diff --git a/stgit/argparse.py b/stgit/argparse.py
index 85ee6e3..765579c 100644
--- a/stgit/argparse.py
+++ b/stgit/argparse.py
@@ -225,6 +225,10 @@ def keep_option():
                 short = 'Keep the local changes',
                 default = config.get('stgit.autokeep') == 'yes')]

+def merged_option():
+    return [opt('-m', '--merged', action = 'store_true',
+                short = 'Check for patches merged upstream')]
+
 class CompgenBase(object):
     def actions(self, var): return set()
     def words(self, var): return set()
diff --git a/stgit/commands/goto.py b/stgit/commands/goto.py
index 66f49df..839b75c 100644
--- a/stgit/commands/goto.py
+++ b/stgit/commands/goto.py
@@ -28,7 +28,7 @@ Push/pop patches to/from the stack until the one
given on the command
 line becomes current."""

 args = [argparse.other_applied_patches, argparse.unapplied_patches]
-options = argparse.keep_option()
+options = argparse.keep_option() + argparse.merged_option()

 directory = common.DirectoryHasRepositoryLib()

@@ -47,8 +47,14 @@ def func(parser, options, args):
         assert not trans.pop_patches(lambda pn: pn in to_pop)
     elif patch in trans.unapplied:
         try:
-            for pn in trans.unapplied[:trans.unapplied.index(patch)+1]:
-                trans.push_patch(pn, iw, allow_interactive = True)
+            to_push = trans.unapplied[:trans.unapplied.index(patch)+1]
+            if options.merged:
+                merged = set(trans.check_merged(to_push))
+            else:
+                merged = set()
+            for pn in to_push:
+                trans.push_patch(pn, iw, allow_interactive = True,
+                                 already_merged = pn in merged)
         except transaction.TransactionHalted:
             pass
     elif patch in trans.hidden:
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index b146648..4148ff3 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -297,7 +297,8 @@ class StackTransaction(object):
                     out.info('Deleted %s%s' % (pn, s))
         return popped

-    def push_patch(self, pn, iw = None, allow_interactive = False):
+    def push_patch(self, pn, iw = None, allow_interactive = False,
+                   already_merged = False):
         """Attempt to push the named patch. If this results in conflicts,
         halts the transaction. If index+worktree are given, spill any
         conflicts to them."""
@@ -305,11 +306,15 @@ class StackTransaction(object):
         cd = orig_cd.set_committer(None)
         oldparent = cd.parent
         cd = cd.set_parent(self.top)
-        base = oldparent.data.tree
-        ours = cd.parent.data.tree
-        theirs = cd.tree
-        tree, self.temp_index_tree = self.temp_index.merge(
-            base, ours, theirs, self.temp_index_tree)
+        if already_merged:
+            # the resulting patch is empty
+            tree = cd.parent.data.tree
+        else:
+            base = oldparent.data.tree
+            ours = cd.parent.data.tree
+            theirs = cd.tree
+            tree, self.temp_index_tree = self.temp_index.merge(
+                base, ours, theirs, self.temp_index_tree)
         s = ''
         merge_conflict = False
         if not tree:
@@ -341,7 +346,9 @@ class StackTransaction(object):
         else:
             comm = None
             s = ' (unmodified)'
-        if not merge_conflict and cd.is_nochange():
+        if already_merged:
+            s = ' (merged)'
+        elif not merge_conflict and cd.is_nochange():
             s = ' (empty)'
         out.info('Pushed %s%s' % (pn, s))
         def update():
@@ -379,3 +386,25 @@ class StackTransaction(object):
         assert set(self.unapplied + self.hidden) == set(unapplied + hidden)
         self.unapplied = unapplied
         self.hidden = hidden
+
+    def check_merged(self, patches):
+        """Return a subset of patches already merged."""
+        merged = []
+        if self.temp_index_tree != self.stack.head.data.tree:
+            self.temp_index.read_tree(self.stack.head.data.tree)
+            self.temp_index_tree = self.stack.head.data.tree
+        for pn in reversed(patches):
+            # check whether patch changes can be reversed in the current index
+            cd = self.patches[pn].data
+            if cd.is_nochange():
+                continue
+            try:
+                self.temp_index.apply_treediff(cd.tree, cd.parent.data.tree,
+                                               quiet = True)
+                merged.append(pn)
+                # The self.temp_index was modified by apply_treediff() so
+                # force read_tree() the next time merge() is used.
+                self.temp_index_tree = None
+            except git.MergeException:
+                pass
+        return merged


-- 
Catalin