Skip to contentSkip to conversation

Fix uncommit with top != head

PATCH New Karl Hasselström 1 participant 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

cover Fix uncommit with top != head

See http://gna.org/bugs/?12043

---

Karl Hasselström (3):
Make sure that stg uncommit doesn't touch the branch head stg uncommit should never touch the branch head Test for exit code with command_error()

 stgit/commands/uncommit.py |    2 +-
 stgit/lib/transaction.py   |   19 ++++++++++---------
 t/t1300-uncommit.sh        |   13 ++++++++++++-
 t/t1303-commit.sh          |   20 ++++++++++++++++++++
 4 files changed, 43 insertions(+), 11 deletions(-)
 create mode 100755 t/t1303-commit.sh
Signature · 2 lines
Karl Hasselström, [email protected]
      www.treskal.com/kalle
#1 lore raw

PATCH 1/3 Test for exit code with command_error()

The helper function was made for occasions such as this, so use it.

---

 t/t1300-uncommit.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
t/t1300-uncommit.sh +1 -1
index a906d13..a657ead 100755
@@ -79,7 +79,7 @@ test_expect_success \
 '
 
 test_expect_success 'Uncommit a commit with not precisely one parent' '
-    stg uncommit -n 5 ; [ $? = 2 ] &&
+    command_error stg uncommit -n 5  &&
     [ "$(echo $(stg series))" = "" ]
 '
 
#2 lore raw

PATCH 2/3 stg uncommit should never touch the branch head

In reply to Karl Hasselström

However, currently, it will set head to top, potentially losing data (which can always be recovered via the reflog, but still). See
https://gna.org/bugs/index.php?12043. Add a test to demonstrate the bad behavior. (Bug discovered by Erik Sandberg <[email protected]>.)

stg commit, on the other hand, should refuse to run if top != head, since the committed patches might otherwise be lost. Add a test to demonstrate that this is the case.

---

 t/t1300-uncommit.sh |   11 +++++++++++
 t/t1303-commit.sh   |   20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100755 t/t1303-commit.sh
t/t1300-uncommit.sh +11 -0
index a657ead..d01eaaa 100755
@@ -83,4 +83,15 @@ test_expect_success 'Uncommit a commit with not precisely one parent' '
     [ "$(echo $(stg series))" = "" ]
 '
 
+# stg uncommit should work even when top != head, and should not touch
+# the head.
+test_expect_failure 'Uncommit when top != head' '
+    stg new -m foo &&
+    git reset --hard HEAD^ &&
+    h=$(git rev-parse HEAD)
+    stg uncommit bar &&
+    test $(git rev-parse HEAD) = $h &&
+    test "$(echo $(stg series))" = "+ bar > foo"
+'
+
 test_done
t/t1303-commit.sh new file +20 -0
new file mode 100755
index 0000000..d53b9f2
@@ -0,0 +1,20 @@
+#!/bin/sh
+test_description='Test stg commit'
+. ./test-lib.sh
+
+test_expect_success 'Initialize the StGIT repository' '
+    stg init
+'
+
+# stg commit with top != head should not succeed, since the committed
+# patches are poptentially lost.
+test_expect_success 'Commit when top != head (should fail)' '
+    stg new -m foo &&
+    git reset --hard HEAD^ &&
+    h=$(git rev-parse HEAD)
+    command_error stg commit &&
+    test $(git rev-parse HEAD) = $h &&
+    test "$(echo $(stg series))" = "> foo"
+'
+
+test_done
#3 lore raw

PATCH 3/3 Make sure that stg uncommit doesn't touch the branch head

In reply to Karl Hasselström

Even if top != head. It used to set head to top; but with this patch, it doesn't anymore.

---

 stgit/commands/uncommit.py |    2 +-
 stgit/lib/transaction.py   |   19 ++++++++++---------
 t/t1300-uncommit.sh        |    2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)
stgit/commands/uncommit.py +1 -1
index eb39fcc..9d2dba9 100644
@@ -136,5 +136,5 @@ def func(parser, options, args):
     for commit, pn in zip(commits, patchnames):
         trans.patches[pn] = commit
     trans.applied = list(reversed(patchnames)) + trans.applied
-    trans.run()
+    trans.run(set_head = False)
     out.done()
stgit/lib/transaction.py +10 -9
index e47997e..23321c7 100644
@@ -138,21 +138,22 @@ class StackTransaction(object):
         # The only state we need to restore is index+worktree.
         if iw:
             self.__checkout(self.__stack.head.data.tree, iw)
-    def run(self, iw = None):
+    def run(self, iw = None, set_head = True):
         """Execute the transaction. Will either succeed, or fail (with an
         exception) and do nothing."""
         self.__check_consistency()
         new_head = self.__head
 
         # Set branch head.
-        if iw:
-            try:
-                self.__checkout(new_head.data.tree, iw)
-            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:
+            if iw:
+                try:
+                    self.__checkout(new_head.data.tree, iw)
+                except git.CheckoutException:
+                    # We have to abort the transaction.
+                    self.abort(iw)
+                    self.__abort()
+            self.__stack.set_head(new_head, self.__msg)
 
         if self.__error:
             out.error(self.__error)
t/t1300-uncommit.sh +1 -1
index d01eaaa..4a955f6 100755
@@ -85,7 +85,7 @@ test_expect_success 'Uncommit a commit with not precisely one parent' '
 
 # stg uncommit should work even when top != head, and should not touch
 # the head.
-test_expect_failure 'Uncommit when top != head' '
+test_expect_success 'Uncommit when top != head' '
     stg new -m foo &&
     git reset --hard HEAD^ &&
     h=$(git rev-parse HEAD)