Re: [RFC PATCH] Record a single transaction for conflicting push operations

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

2009/12/18 Karl Wiberg <kha@treskal.com>:
> On Fri, Dec 18, 2009 at 12:22 AM, Catalin Marinas
> <catalin.marinas@gmail.com> wrote:
>
>> StGit commands resulting in a conflicting patch pushing record two
>> transactions in the log (with one of them being inconsistent with
>> HEAD != top). Undoing such operations requires two "stg undo"
>> (possibly with --hard) commands which is unintuitive. This patch
>> changes such operations to only record one log entry and "stg undo"
>> reverts the stack to the state prior to the operation.
>
> Hmm, OK. It was convenient to be able to undo just the last
> conflicting step, but I guess the increase in UI complexity wasn't
> worth it.
>
> I think your patch doesn't go quite far enough, though.
> self.__conflicting_push is currently set to a function that will do
> the extra updates that take us from the first to the second state to
> save in the log; if we'll be saving at only one point, we might as
> well run those updates immediately instead of deferring them. In other
> words, the entire __conflicting_push variable could be removed.

See below for an updated patch:


Record a single transaction for conflicting push operations

From: Catalin Marinas <catalin.marinas@gmail.com>

StGit commands resulting in a conflicting patch pushing record two
transactions in the log (with one of them being inconsistent with HEAD
!= top). Undoing such operations requires two "stg undo" (possibly with
--hard) commands which is unintuitive. This patch changes such
operations to only record one log entry and "stg undo" reverts the stack
to the state prior to the operation.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
Cc: Gustav Hållberg <gustav@virtutech.com>
Cc: Karl Wiberg <kha@treskal.com>
---
 stgit/lib/transaction.py |   14 +++++---------
 t/t3101-reset-hard.sh    |    2 +-
 t/t3103-undo-hard.sh     |    4 ++--
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 30a153b..ea85d5d 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -90,7 +90,6 @@ class StackTransaction(object):
         self.__applied = list(self.__stack.patchorder.applied)
         self.__unapplied = list(self.__stack.patchorder.unapplied)
         self.__hidden = list(self.__stack.patchorder.hidden)
-        self.__conflicting_push = None
         self.__error = None
         self.__current_tree = self.__stack.head.data.tree
         self.__base = self.__stack.base
@@ -232,10 +231,9 @@ class StackTransaction(object):
             self.__stack.patchorder.hidden = self.__hidden
             log.log_entry(self.__stack, msg)
         old_applied = self.__stack.patchorder.applied
-        write(self.__msg)
-        if self.__conflicting_push != None:
-            self.__patches = _TransPatchMap(self.__stack)
-            self.__conflicting_push()
+        if not self.__conflicts:
+            write(self.__msg)
+        else:
             write(self.__msg + ' (CONFLICT)')
         if print_current_patch:
             _print_current_patch(old_applied, self.__applied)
@@ -371,12 +369,10 @@ class StackTransaction(object):
             # We've just caused conflicts, so we must allow them in
             # the final checkout.
             self.__allow_conflicts = lambda trans: True
-
-            # Save this update so that we can run it a little later.
-            self.__conflicting_push = update
+            self.__patches = _TransPatchMap(self.__stack)
+            update()
             self.__halt("%d merge conflict(s)" % len(self.__conflicts))
         else:
-            # Update immediately.
             update()

     def push_tree(self, pn):
diff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh
index bd97b3a..45e86dc 100755
--- a/t/t3101-reset-hard.sh
+++ b/t/t3101-reset-hard.sh
@@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '
     stg reset --hard master.stgit^~1 &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p2 - p3"
+    test "$(echo $(stg series))" = "+ p1 + p2 > p3"
 '

 test_done
diff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh
index 2d0f382..df14b1f 100755
--- a/t/t3103-undo-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -46,11 +46,11 @@ test_expect_success 'Try to undo without --hard' '

 cat > expected.txt <<EOF
 EOF
-test_expect_failure 'Try to undo with --hard' '
+test_expect_success 'Try to undo with --hard' '
     stg undo --hard &&
     stg status a > actual.txt &&
     test_cmp expected.txt actual.txt &&
-    test "$(echo $(stg series))" = "> p1 - p2 - p3" &&
+    test "$(echo $(stg series))" = "+ p1 + p2 > p3" &&
     test "$(stg id)" = "$(stg id $(stg top))"
 '


-- 
Catalin