{"thread":{"id":"14666","subject":"[StGit PATCH 0/3] Fix uncommit with top != head","startedAt":"2008-07-25T00:52:49Z","lastAt":"2008-07-25T00:53:06Z","messageCount":4,"participants":["Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"84821","messageId":"20080725005154.13006.8908.stgit@yoghurt","threadId":"14666","inReplyTo":null,"subject":"[StGit PATCH 0/3] Fix uncommit with top != head","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-25T00:52:49Z","receivedAt":"2008-07-25T00:52:49Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"See http://gna.org/bugs/?12043\n\n---\n\nKarl Hasselström (3):\n      Make sure that stg uncommit doesn't touch the branch head\n      stg uncommit should never touch the branch head\n      Test for exit code with command_error()\n\n\n stgit/commands/uncommit.py |    2 +-\n stgit/lib/transaction.py   |   19 ++++++++++---------\n t/t1300-uncommit.sh        |   13 ++++++++++++-\n t/t1303-commit.sh          |   20 ++++++++++++++++++++\n 4 files changed, 43 insertions(+), 11 deletions(-)\n create mode 100755 t/t1303-commit.sh\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"84823","messageId":"20080725005254.13006.49490.stgit@yoghurt","threadId":"14666","inReplyTo":"20080725005154.13006.8908.stgit@yoghurt","subject":"[StGit PATCH 1/3] Test for exit code with command_error()","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-25T00:52:55Z","receivedAt":"2008-07-25T00:52:55Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"The helper function was made for occasions such as this, so use it.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n t/t1300-uncommit.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n\ndiff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh\nindex a906d13..a657ead 100755\n--- a/t/t1300-uncommit.sh\n+++ b/t/t1300-uncommit.sh\n@@ -79,7 +79,7 @@ test_expect_success \\\n '\n \n test_expect_success 'Uncommit a commit with not precisely one parent' '\n-    stg uncommit -n 5 ; [ $? = 2 ] &&\n+    command_error stg uncommit -n 5  &&\n     [ \"$(echo $(stg series))\" = \"\" ]\n '\n \n"},{"id":"84822","messageId":"20080725005300.13006.96166.stgit@yoghurt","threadId":"14666","inReplyTo":"20080725005154.13006.8908.stgit@yoghurt","subject":"[StGit PATCH 2/3] stg uncommit should never touch the branch head","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-25T00:53:00Z","receivedAt":"2008-07-25T00:53:00Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"However, currently, it will set head to top, potentially losing data\n(which can always be recovered via the reflog, but still). See\nhttps://gna.org/bugs/index.php?12043. Add a test to demonstrate the\nbad behavior. (Bug discovered by Erik Sandberg\n<mandolaerik@gmail.com>.)\n\nstg commit, on the other hand, should refuse to run if top != head,\nsince the committed patches might otherwise be lost. Add a test to\ndemonstrate that this is the case.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n t/t1300-uncommit.sh |   11 +++++++++++\n t/t1303-commit.sh   |   20 ++++++++++++++++++++\n 2 files changed, 31 insertions(+), 0 deletions(-)\n create mode 100755 t/t1303-commit.sh\n\n\ndiff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh\nindex a657ead..d01eaaa 100755\n--- a/t/t1300-uncommit.sh\n+++ b/t/t1300-uncommit.sh\n@@ -83,4 +83,15 @@ test_expect_success 'Uncommit a commit with not precisely one parent' '\n     [ \"$(echo $(stg series))\" = \"\" ]\n '\n \n+# stg uncommit should work even when top != head, and should not touch\n+# the head.\n+test_expect_failure 'Uncommit when top != head' '\n+    stg new -m foo &&\n+    git reset --hard HEAD^ &&\n+    h=$(git rev-parse HEAD)\n+    stg uncommit bar &&\n+    test $(git rev-parse HEAD) = $h &&\n+    test \"$(echo $(stg series))\" = \"+ bar > foo\"\n+'\n+\n test_done\ndiff --git a/t/t1303-commit.sh b/t/t1303-commit.sh\nnew file mode 100755\nindex 0000000..d53b9f2\n--- /dev/null\n+++ b/t/t1303-commit.sh\n@@ -0,0 +1,20 @@\n+#!/bin/sh\n+test_description='Test stg commit'\n+. ./test-lib.sh\n+\n+test_expect_success 'Initialize the StGIT repository' '\n+    stg init\n+'\n+\n+# stg commit with top != head should not succeed, since the committed\n+# patches are poptentially lost.\n+test_expect_success 'Commit when top != head (should fail)' '\n+    stg new -m foo &&\n+    git reset --hard HEAD^ &&\n+    h=$(git rev-parse HEAD)\n+    command_error stg commit &&\n+    test $(git rev-parse HEAD) = $h &&\n+    test \"$(echo $(stg series))\" = \"> foo\"\n+'\n+\n+test_done\n"},{"id":"84824","messageId":"20080725005306.13006.51259.stgit@yoghurt","threadId":"14666","inReplyTo":"20080725005154.13006.8908.stgit@yoghurt","subject":"[StGit PATCH 3/3] Make sure that stg uncommit doesn't touch the branch head","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-07-25T00:53:06Z","receivedAt":"2008-07-25T00:53:06Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Even if top != head. It used to set head to top; but with this patch,\nit doesn't anymore.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/commands/uncommit.py |    2 +-\n stgit/lib/transaction.py   |   19 ++++++++++---------\n t/t1300-uncommit.sh        |    2 +-\n 3 files changed, 12 insertions(+), 11 deletions(-)\n\n\ndiff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py\nindex eb39fcc..9d2dba9 100644\n--- a/stgit/commands/uncommit.py\n+++ b/stgit/commands/uncommit.py\n@@ -136,5 +136,5 @@ def func(parser, options, args):\n     for commit, pn in zip(commits, patchnames):\n         trans.patches[pn] = commit\n     trans.applied = list(reversed(patchnames)) + trans.applied\n-    trans.run()\n+    trans.run(set_head = False)\n     out.done()\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex e47997e..23321c7 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -138,21 +138,22 @@ class StackTransaction(object):\n         # The only state we need to restore is index+worktree.\n         if iw:\n             self.__checkout(self.__stack.head.data.tree, iw)\n-    def run(self, iw = None):\n+    def run(self, iw = None, set_head = True):\n         \"\"\"Execute the transaction. Will either succeed, or fail (with an\n         exception) and do nothing.\"\"\"\n         self.__check_consistency()\n         new_head = self.__head\n \n         # Set branch head.\n-        if iw:\n-            try:\n-                self.__checkout(new_head.data.tree, iw)\n-            except git.CheckoutException:\n-                # We have to abort the transaction.\n-                self.abort(iw)\n-                self.__abort()\n-        self.__stack.set_head(new_head, self.__msg)\n+        if set_head:\n+            if iw:\n+                try:\n+                    self.__checkout(new_head.data.tree, iw)\n+                except git.CheckoutException:\n+                    # We have to abort the transaction.\n+                    self.abort(iw)\n+                    self.__abort()\n+            self.__stack.set_head(new_head, self.__msg)\n \n         if self.__error:\n             out.error(self.__error)\ndiff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh\nindex d01eaaa..4a955f6 100755\n--- a/t/t1300-uncommit.sh\n+++ b/t/t1300-uncommit.sh\n@@ -85,7 +85,7 @@ test_expect_success 'Uncommit a commit with not precisely one parent' '\n \n # stg uncommit should work even when top != head, and should not touch\n # the head.\n-test_expect_failure 'Uncommit when top != head' '\n+test_expect_success 'Uncommit when top != head' '\n     stg new -m foo &&\n     git reset --hard HEAD^ &&\n     h=$(git rev-parse HEAD)\n"}]}