[StGit PATCH 0/3] Fix uncommit with top != head

[StGit PATCH 0/3] Fix uncommit with top != head

[Karl Hasselström]

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

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

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

[Karl Hasselström]

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

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 t/t1300-uncommit.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh
index a906d13..a657ead 100755
--- a/t/t1300-uncommit.sh
+++ b/t/t1300-uncommit.sh
@@ -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))" = "" ]
 '
 

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

[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
<mandolaerik@gmail.com>.)

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.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 t/t1300-uncommit.sh |   11 +++++++++++
 t/t1303-commit.sh   |   20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100755 t/t1303-commit.sh


diff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh
index a657ead..d01eaaa 100755
--- a/t/t1300-uncommit.sh
+++ b/t/t1300-uncommit.sh
@@ -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
diff --git a/t/t1303-commit.sh b/t/t1303-commit.sh
new file mode 100755
index 0000000..d53b9f2
--- /dev/null
+++ b/t/t1303-commit.sh
@@ -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

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

[Karl Hasselström]

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

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/uncommit.py |    2 +-
 stgit/lib/transaction.py   |   19 ++++++++++---------
 t/t1300-uncommit.sh        |    2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)


diff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py
index eb39fcc..9d2dba9 100644
--- a/stgit/commands/uncommit.py
+++ b/stgit/commands/uncommit.py
@@ -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()
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index e47997e..23321c7 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -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)
diff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh
index d01eaaa..4a955f6 100755
--- a/t/t1300-uncommit.sh
+++ b/t/t1300-uncommit.sh
@@ -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)