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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2009/12/21 Karl Wiberg <kha@xxxxxxxxxxx>:
> By the way, you do realize there's another command that requires two
> steps to undo completely: refresh? And that one is harder to get out
> of---undoing it all in one step would mean throwing away the updates
> to the patch.

But it looks to me like refresh does this by running separate
transactions. The push command does this in a single transaction, so
the quickest fix for the HEAD != top undo problem was to only record
one log per transaction.

If we keep the current behaviour with two logs per transaction, we
need to preserve the HEAD prior to the conflict so that logging
doesn't get the wrong HEAD (which is the new conflicting HEAD
currently). The patch below appears to fix this problem and still
generate two logs per transaction. While I'm more in favour of a
single log per transaction, if people find it useful I'm happy to keep
the current behaviour.

diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 30a153b..ba97c4f 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -197,18 +197,14 @@ class StackTransaction(object):
         exception) and do nothing."""
         self.__check_consistency()
         log.log_external_mods(self.__stack)
-        new_head = self.head
-
-        # Set branch head.
-        if set_head:
-            if iw:
-                try:
-                    self.__checkout(new_head.data.tree, iw, allow_bad_head)
-                except git.CheckoutException:
-                    # We have to abort the transaction.
-                    self.abort(iw)
-                    self.__abort()
-            self.__stack.set_head(new_head, self.__msg)
+
+        if set_head and iw:
+            try:
+                self.__checkout(self.head.data.tree, iw, allow_bad_head)
+            except git.CheckoutException:
+                # We have to abort the transaction.
+                self.abort(iw)
+                self.__abort()

         if self.__error:
             if self.__conflicts:
@@ -216,8 +212,11 @@ class StackTransaction(object):
             else:
                 out.error(self.__error)

-        # Write patches.
-        def write(msg):
+        # Write patches and update the branch head.
+        def write(msg, new_head):
+            # Set branch head.
+            if new_head:
+                self.__stack.set_head(new_head, self.__msg)
             for pn, commit in self.__patches.iteritems():
                 if self.__stack.patches.exists(pn):
                     p = self.__stack.patches.get(pn)
@@ -231,12 +230,16 @@ class StackTransaction(object):
             self.__stack.patchorder.unapplied = self.__unapplied
             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:
+            write(self.__msg, set_head and self.head)
             self.__patches = _TransPatchMap(self.__stack)
             self.__conflicting_push()
-            write(self.__msg + ' (CONFLICT)')
+            write(self.__msg + ' (CONFLICT)', set_head and self.head)
+        else:
+            write(self.__msg, set_head and self.head)
+
         if print_current_patch:
             _print_current_patch(old_applied, self.__applied)

@@ -346,10 +349,10 @@ class StackTransaction(object):
             if merge_conflict:
                 # When we produce a conflict, we'll run the update()
                 # function defined below _after_ having done the
-                # checkout in run(). To make sure that we check out
-                # the real stack top (as it will look after update()
-                # has been run), set it hard here.
-                self.head = comm
+                # checkout in run(). Make sure that we have a consistent
+                # HEAD before the update function is called below (which
+                # sets the real HEAD).
+                self.head = self.top
         else:
             comm = None
             s = 'unmodified'
@@ -367,6 +370,8 @@ class StackTransaction(object):
                 x = self.unapplied
             del x[x.index(pn)]
             self.applied.append(pn)
+            # Set the real conflicting HEAD.
+            self.head = comm
         if merge_conflict:
             # We've just caused conflicts, so we must allow them in
             # the final checkout.
diff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh
index 2d0f382..a71cd32 100755
--- a/t/t3103-undo-hard.sh
+++ b/t/t3103-undo-hard.sh
@@ -46,7 +46,7 @@ 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 &&


-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]