{"thread":{"id":"21979","subject":"[RFC PATCH] Record a single transaction for conflicting push operations","startedAt":"2009-12-17T23:22:12Z","lastAt":"2009-12-22T18:33:44Z","messageCount":10,"participants":["Catalin Marinas","Karl Wiberg","Gustav Hållberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"130046","messageId":"20091217232212.4869.43002.stgit@toshiba-laptop","threadId":"21979","inReplyTo":null,"subject":"[RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-12-17T23:22:12Z","receivedAt":"2009-12-17T23:22:12Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"StGit commands resulting in a conflicting patch pushing record two\ntransactions in the log (with one of them being inconsistent with HEAD\n!= top). Undoing such operations requires two \"stg undo\" (possibly with\n--hard) commands which is unintuitive. This patch changes such\noperations to only record one log entry and \"stg undo\" reverts the stack\nto the state prior to the operation.\n\nSigned-off-by: Catalin Marinas \nCc: Gustav Hållberg \nCc: Karl Wiberg \n---\n stgit/lib/transaction.py | 5 +++--\n t/t3103-undo-hard.sh | 4 ++--\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 30a153b..fad5ab4 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -232,8 +232,9 @@ class StackTransaction(object):\n self.__stack.patchorder.hidden = self.__hidden\n log.log_entry(self.__stack, msg)\n old_applied = self.__stack.patchorder.applied\n- write(self.__msg)\n- if self.__conflicting_push != None:\n+ if not self.__conflicting_push:\n+ write(self.__msg)\n+ else:\n self.__patches = _TransPatchMap(self.__stack)\n self.__conflicting_push()\n write(self.__msg + ' (CONFLICT)')\ndiff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh\nindex 2d0f382..df14b1f 100755\n--- a/t/t3103-undo-hard.sh\n+++ b/t/t3103-undo-hard.sh\n@@ -46,11 +46,11 @@ test_expect_success 'Try to undo without --hard' '\n \n cat > expected.txt < actual.txt &&\n test_cmp expected.txt actual.txt &&\n- test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\" &&\n+ test \"$(echo $(stg series))\" = \"+ p1 + p2 > p3\" &&\n test \"$(stg id)\" = \"$(stg id $(stg top))\"\n '\n \n"},{"id":"130066","messageId":"b8197bcb0912180123l4657839ctc121636af3724bee@mail.gmail.com","threadId":"21979","inReplyTo":"20091217232212.4869.43002.stgit@toshiba-laptop","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Karl Wiberg","fromEmail":"kha@treskal.com","sentAt":"2009-12-18T09:23:38Z","receivedAt":"2009-12-18T09:23:38Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On Fri, Dec 18, 2009 at 12:22 AM, Catalin Marinas\n wrote:\n\n> StGit commands resulting in a conflicting patch pushing record two\n> transactions in the log (with one of them being inconsistent with\n> HEAD != top). Undoing such operations requires two \"stg undo\"\n> (possibly with --hard) commands which is unintuitive. This patch\n> changes such operations to only record one log entry and \"stg undo\"\n> reverts the stack to the state prior to the operation.\n\nHmm, OK. It was convenient to be able to undo just the last\nconflicting step, but I guess the increase in UI complexity wasn't\nworth it.\n\nI think your patch doesn't go quite far enough, though.\nself.__conflicting_push is currently set to a function that will do\nthe extra updates that take us from the first to the second state to\nsave in the log; if we'll be saving at only one point, we might as\nwell run those updates immediately instead of deferring them. In other\nwords, the entire __conflicting_push variable could be removed.\n\n-- \nKarl Wiberg, kha@treskal.com\n subrabbit.wordpress.com\n www.treskal.com/kalle\n"},{"id":"130075","messageId":"b0943d9e0912180749ga8857d9j975e119937db9674@mail.gmail.com","threadId":"21979","inReplyTo":"b8197bcb0912180123l4657839ctc121636af3724bee@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-12-18T15:49:46Z","receivedAt":"2009-12-18T15:49:46Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/12/18 Karl Wiberg :\n> On Fri, Dec 18, 2009 at 12:22 AM, Catalin Marinas\n> wrote:\n>\n>> StGit commands resulting in a conflicting patch pushing record two\n>> transactions in the log (with one of them being inconsistent with\n>> HEAD != top). Undoing such operations requires two \"stg undo\"\n>> (possibly with --hard) commands which is unintuitive. This patch\n>> changes such operations to only record one log entry and \"stg undo\"\n>> reverts the stack to the state prior to the operation.\n>\n> Hmm, OK. It was convenient to be able to undo just the last\n> conflicting step, but I guess the increase in UI complexity wasn't\n> worth it.\n>\n> I think your patch doesn't go quite far enough, though.\n> self.__conflicting_push is currently set to a function that will do\n> the extra updates that take us from the first to the second state to\n> save in the log; if we'll be saving at only one point, we might as\n> well run those updates immediately instead of deferring them. In other\n> words, the entire __conflicting_push variable could be removed.\n\nSee below for an updated patch:\n\n\nRecord a single transaction for conflicting push operations\n\nFrom: Catalin Marinas \n\nStGit commands resulting in a conflicting patch pushing record two\ntransactions in the log (with one of them being inconsistent with HEAD\n!= top). Undoing such operations requires two \"stg undo\" (possibly with\n--hard) commands which is unintuitive. This patch changes such\noperations to only record one log entry and \"stg undo\" reverts the stack\nto the state prior to the operation.\n\nSigned-off-by: Catalin Marinas \nCc: Gustav Hållberg \nCc: Karl Wiberg \n---\n stgit/lib/transaction.py | 14 +++++---------\n t/t3101-reset-hard.sh | 2 +-\n t/t3103-undo-hard.sh | 4 ++--\n 3 files changed, 8 insertions(+), 12 deletions(-)\n\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 30a153b..ea85d5d 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -90,7 +90,6 @@ class StackTransaction(object):\n self.__applied = list(self.__stack.patchorder.applied)\n self.__unapplied = list(self.__stack.patchorder.unapplied)\n self.__hidden = list(self.__stack.patchorder.hidden)\n- self.__conflicting_push = None\n self.__error = None\n self.__current_tree = self.__stack.head.data.tree\n self.__base = self.__stack.base\n@@ -232,10 +231,9 @@ class StackTransaction(object):\n self.__stack.patchorder.hidden = self.__hidden\n log.log_entry(self.__stack, msg)\n old_applied = self.__stack.patchorder.applied\n- write(self.__msg)\n- if self.__conflicting_push != None:\n- self.__patches = _TransPatchMap(self.__stack)\n- self.__conflicting_push()\n+ if not self.__conflicts:\n+ write(self.__msg)\n+ else:\n write(self.__msg + ' (CONFLICT)')\n if print_current_patch:\n _print_current_patch(old_applied, self.__applied)\n@@ -371,12 +369,10 @@ class StackTransaction(object):\n # We've just caused conflicts, so we must allow them in\n # the final checkout.\n self.__allow_conflicts = lambda trans: True\n-\n- # Save this update so that we can run it a little later.\n- self.__conflicting_push = update\n+ self.__patches = _TransPatchMap(self.__stack)\n+ update()\n self.__halt(\"%d merge conflict(s)\" % len(self.__conflicts))\n else:\n- # Update immediately.\n update()\n\n def push_tree(self, pn):\ndiff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh\nindex bd97b3a..45e86dc 100755\n--- a/t/t3101-reset-hard.sh\n+++ b/t/t3101-reset-hard.sh\n@@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '\n stg reset --hard master.stgit^~1 &&\n stg status a > actual.txt &&\n test_cmp expected.txt actual.txt &&\n- test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n+ test \"$(echo $(stg series))\" = \"+ p1 + p2 > p3\"\n '\n\n test_done\ndiff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh\nindex 2d0f382..df14b1f 100755\n--- a/t/t3103-undo-hard.sh\n+++ b/t/t3103-undo-hard.sh\n@@ -46,11 +46,11 @@ test_expect_success 'Try to undo without --hard' '\n\n cat > expected.txt < actual.txt &&\n test_cmp expected.txt actual.txt &&\n- test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\" &&\n+ test \"$(echo $(stg series))\" = \"+ p1 + p2 > p3\" &&\n test \"$(stg id)\" = \"$(stg id $(stg top))\"\n '\n\n\n-- \nCatalin\n"},{"id":"130166","messageId":"b8197bcb0912191550u300a9c20o351eba66c85292bb@mail.gmail.com","threadId":"21979","inReplyTo":"b0943d9e0912180749ga8857d9j975e119937db9674@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Karl Wiberg","fromEmail":"kha@treskal.com","sentAt":"2009-12-19T23:50:03Z","receivedAt":"2009-12-19T23:50:03Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On Fri, Dec 18, 2009 at 4:49 PM, Catalin Marinas\n wrote:\n\n> @@ -371,12 +369,10 @@ class StackTransaction(object):\n> # We've just caused conflicts, so we must allow them in\n> # the final checkout.\n> self.__allow_conflicts = lambda trans: True\n> -\n> - # Save this update so that we can run it a little later.\n> - self.__conflicting_push = update\n> + self.__patches = _TransPatchMap(self.__stack)\n> + update()\n> self.__halt(\"%d merge conflict(s)\" % len(self.__conflicts))\n> else:\n> - # Update immediately.\n> update()\n>\n> def push_tree(self, pn):\n\nBetter. But couldn't you remove the update function completely and\njust inline the code in it, since it's called immediately?\n\n-- \nKarl Wiberg, kha@treskal.com\n subrabbit.wordpress.com\n www.treskal.com/kalle\n"},{"id":"130198","messageId":"b0943d9e0912201521k73bdcb5fl333e845028954050@mail.gmail.com","threadId":"21979","inReplyTo":"b8197bcb0912191550u300a9c20o351eba66c85292bb@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-12-20T23:21:53Z","receivedAt":"2009-12-20T23:21:53Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/12/19 Karl Wiberg :\n> On Fri, Dec 18, 2009 at 4:49 PM, Catalin Marinas\n> wrote:\n>\n>> @@ -371,12 +369,10 @@ class StackTransaction(object):\n>>             # We've just caused conflicts, so we must allow them in\n>>             # the final checkout.\n>>             self.__allow_conflicts = lambda trans: True\n>> -\n>> -            # Save this update so that we can run it a little later.\n>> -            self.__conflicting_push = update\n>> +            self.__patches = _TransPatchMap(self.__stack)\n>> +            update()\n>>             self.__halt(\"%d merge conflict(s)\" % len(self.__conflicts))\n>>         else:\n>> -            # Update immediately.\n>>             update()\n>>\n>>     def push_tree(self, pn):\n>\n> Better. But couldn't you remove the update function completely and\n> just inline the code in it, since it's called immediately?\n\nOf course, I tried, but couldn't get it to work. I get HEAD and top\nnot equal unless I call update() between _TransPatchMap and\nself.__halt(). For the non-conflicting case we need to call update\nbefore or after this \"if merge_conflict\".\n\nOne solution is to split the \"if merge_conflict\" in two but maybe you\nhave a better idea.\n\nThanks,\n\n-- \nCatalin\n"},{"id":"130200","messageId":"b8197bcb0912202308p296207av416cd5590a11251b@mail.gmail.com","threadId":"21979","inReplyTo":"b0943d9e0912201521k73bdcb5fl333e845028954050@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Karl Wiberg","fromEmail":"kha@treskal.com","sentAt":"2009-12-21T07:08:46Z","receivedAt":"2009-12-21T07:08:46Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On Mon, Dec 21, 2009 at 12:21 AM, Catalin Marinas\n wrote:\n> 2009/12/19 Karl Wiberg :\n>\n>> Better. But couldn't you remove the update function completely and\n>> just inline the code in it, since it's called immediately?\n>\n> Of course, I tried, but couldn't get it to work. I get HEAD and top\n> not equal unless I call update() between _TransPatchMap and\n> self.__halt(). For the non-conflicting case we need to call update\n> before or after this \"if merge_conflict\".\n>\n> One solution is to split the \"if merge_conflict\" in two but maybe\n> you have a better idea.\n\nYes, duplicating the conditional was what I had in mind. But if you\ndon't find it to improve the readability of the code (as compared to\nhaving a function), I certainly won't insist.\n\nThanks for working on this.\n\nBy the way, you do realize there's another command that requires two\nsteps to undo completely: refresh? And that one is harder to get out\nof---undoing it all in one step would mean throwing away the updates\nto the patch.\n\n-- \nKarl Wiberg, kha@treskal.com\n subrabbit.wordpress.com\n www.treskal.com/kalle\n"},{"id":"130214","messageId":"b0943d9e0912210348o37b71935x5fad4f1a4be4b70@mail.gmail.com","threadId":"21979","inReplyTo":"b8197bcb0912202308p296207av416cd5590a11251b@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-12-21T11:48:32Z","receivedAt":"2009-12-21T11:48:32Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/12/21 Karl Wiberg :\n> By the way, you do realize there's another command that requires two\n> steps to undo completely: refresh? And that one is harder to get out\n> of---undoing it all in one step would mean throwing away the updates\n> to the patch.\n\nBut it looks to me like refresh does this by running separate\ntransactions. The push command does this in a single transaction, so\nthe quickest fix for the HEAD != top undo problem was to only record\none log per transaction.\n\nIf we keep the current behaviour with two logs per transaction, we\nneed to preserve the HEAD prior to the conflict so that logging\ndoesn't get the wrong HEAD (which is the new conflicting HEAD\ncurrently). The patch below appears to fix this problem and still\ngenerate two logs per transaction. While I'm more in favour of a\nsingle log per transaction, if people find it useful I'm happy to keep\nthe current behaviour.\n\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 30a153b..ba97c4f 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -197,18 +197,14 @@ class StackTransaction(object):\n exception) and do nothing.\"\"\"\n self.__check_consistency()\n log.log_external_mods(self.__stack)\n- new_head = self.head\n-\n- # Set branch head.\n- if set_head:\n- if iw:\n- try:\n- self.__checkout(new_head.data.tree, iw, allow_bad_head)\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 set_head and iw:\n+ try:\n+ self.__checkout(self.head.data.tree, iw, allow_bad_head)\n+ except git.CheckoutException:\n+ # We have to abort the transaction.\n+ self.abort(iw)\n+ self.__abort()\n\n if self.__error:\n if self.__conflicts:\n@@ -216,8 +212,11 @@ class StackTransaction(object):\n else:\n out.error(self.__error)\n\n- # Write patches.\n- def write(msg):\n+ # Write patches and update the branch head.\n+ def write(msg, new_head):\n+ # Set branch head.\n+ if new_head:\n+ self.__stack.set_head(new_head, self.__msg)\n for pn, commit in self.__patches.iteritems():\n if self.__stack.patches.exists(pn):\n p = self.__stack.patches.get(pn)\n@@ -231,12 +230,16 @@ class StackTransaction(object):\n self.__stack.patchorder.unapplied = self.__unapplied\n self.__stack.patchorder.hidden = self.__hidden\n log.log_entry(self.__stack, msg)\n+\n old_applied = self.__stack.patchorder.applied\n- write(self.__msg)\n if self.__conflicting_push != None:\n+ write(self.__msg, set_head and self.head)\n self.__patches = _TransPatchMap(self.__stack)\n self.__conflicting_push()\n- write(self.__msg + ' (CONFLICT)')\n+ write(self.__msg + ' (CONFLICT)', set_head and self.head)\n+ else:\n+ write(self.__msg, set_head and self.head)\n+\n if print_current_patch:\n _print_current_patch(old_applied, self.__applied)\n\n@@ -346,10 +349,10 @@ class StackTransaction(object):\n if merge_conflict:\n # When we produce a conflict, we'll run the update()\n # function defined below _after_ having done the\n- # checkout in run(). To make sure that we check out\n- # the real stack top (as it will look after update()\n- # has been run), set it hard here.\n- self.head = comm\n+ # checkout in run(). Make sure that we have a consistent\n+ # HEAD before the update function is called below (which\n+ # sets the real HEAD).\n+ self.head = self.top\n else:\n comm = None\n s = 'unmodified'\n@@ -367,6 +370,8 @@ class StackTransaction(object):\n x = self.unapplied\n del x[x.index(pn)]\n self.applied.append(pn)\n+ # Set the real conflicting HEAD.\n+ self.head = comm\n if merge_conflict:\n # We've just caused conflicts, so we must allow them in\n # the final checkout.\ndiff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh\nindex 2d0f382..a71cd32 100755\n--- a/t/t3103-undo-hard.sh\n+++ b/t/t3103-undo-hard.sh\n@@ -46,7 +46,7 @@ test_expect_success 'Try to undo without --hard' '\n\n cat > expected.txt < actual.txt &&\n test_cmp expected.txt actual.txt &&\n\n\n-- \nCatalin\n"},{"id":"130215","messageId":"b8197bcb0912210548q67c1da4bhe023bed2811394d4@mail.gmail.com","threadId":"21979","inReplyTo":"b0943d9e0912210348o37b71935x5fad4f1a4be4b70@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Karl Wiberg","fromEmail":"kha@treskal.com","sentAt":"2009-12-21T13:48:10Z","receivedAt":"2009-12-21T13:48:10Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On Mon, Dec 21, 2009 at 12:48 PM, Catalin Marinas\n wrote:\n\n> 2009/12/21 Karl Wiberg :\n>\n>> By the way, you do realize there's another command that requires\n>> two steps to undo completely: refresh? And that one is harder to\n>> get out of---undoing it all in one step would mean throwing away\n>> the updates to the patch.\n>\n> But it looks to me like refresh does this by running separate\n> transactions.\n\nYes. So it won't be affected by whatever you do here. (Unless you\nconsider that refresh -p needs to reorder patches, which can result in\nconflicts---right now, refresh -p can result in three log entries.)\n\n> The push command does this in a single transaction, so the quickest\n> fix for the HEAD != top undo problem was to only record one log per\n> transaction.\n\nI've seen more than one complaint that the current behavior is\nconfusing even if we don't count the bug, so I thought this was part\nof the motivation.\n\n> If we keep the current behaviour with two logs per transaction, we\n> need to preserve the HEAD prior to the conflict so that logging\n> doesn't get the wrong HEAD (which is the new conflicting HEAD\n> currently). The patch below appears to fix this problem and still\n> generate two logs per transaction. While I'm more in favour of a\n> single log per transaction, if people find it useful I'm happy to\n> keep the current behaviour.\n\nI haven't seen anyone but me defent the current design, and it's not a\nbig deal for me either, so I'd say go with just one transaction.\n\n-- \nKarl Wiberg, kha@treskal.com\n subrabbit.wordpress.com\n www.treskal.com/kalle\n"},{"id":"130216","messageId":"4B2F86BB.9090104@virtutech.com","threadId":"21979","inReplyTo":"b8197bcb0912210548q67c1da4bhe023bed2811394d4@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Gustav Hållberg","fromEmail":"gustav@virtutech.com","sentAt":"2009-12-21T14:31:23Z","receivedAt":"2009-12-21T14:31:23Z","isPatch":true,"sender":{"key":"gustav@virtutech.com","avatar":null},"body":"On 2009-12-21 14:48, Karl Wiberg wrote:\n> I've seen more than one complaint that the current behavior is\n> confusing even if we don't count the bug, so I thought this was part\n> of the motivation.\n\nI don't know if this would be better than the other suggested solutions, \nbut if \"stg log\" would clearly identify multi-stage entries as such, the \ncurrent confusion would probably mostly go away.\n\nCurrently this is done reasonably well for make_temp_patch(), which says \n\"refresh (create temporary patch)\" in the log, but I think this could be \ntaken further.\n\nFor example, if such annotations said \"foo: stage N\" or similar, \nindicating that this was the Nth step in the \"foo\" command (think \n\"rebase\" or whatever), it would be good enough for me least.\n\n- Gustav\n"},{"id":"130250","messageId":"b0943d9e0912221033p39375ae6n9593b7d2887cb1ba@mail.gmail.com","threadId":"21979","inReplyTo":"b8197bcb0912210548q67c1da4bhe023bed2811394d4@mail.gmail.com","subject":"Re: [RFC PATCH] Record a single transaction for conflicting push operations","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-12-22T18:33:44Z","receivedAt":"2009-12-22T18:33:44Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"Updated patch below:\n\n\nRecord a single transaction for conflicting push operations\n\nFrom: Catalin Marinas \n\nStGit commands resulting in a conflicting patch pushing record two\ntransactions in the log (with one of them being inconsistent with HEAD\n!= top). Undoing such operations requires two \"stg undo\" (possibly with\n--hard) commands which is unintuitive. This patch changes such\noperations to only record one log entry and \"stg undo\" reverts the stack\nto the state prior to the operation.\n\nSigned-off-by: Catalin Marinas \nCc: Gustav Hållberg \nCc: Karl Wiberg \n---\n stgit/lib/transaction.py | 35 ++++++++++++++++-------------------\n t/t3101-reset-hard.sh | 2 +-\n t/t3103-undo-hard.sh | 4 ++--\n 3 files changed, 19 insertions(+), 22 deletions(-)\n\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 30a153b..d82e724 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -90,7 +90,6 @@ class StackTransaction(object):\n self.__applied = list(self.__stack.patchorder.applied)\n self.__unapplied = list(self.__stack.patchorder.unapplied)\n self.__hidden = list(self.__stack.patchorder.hidden)\n- self.__conflicting_push = None\n self.__error = None\n self.__current_tree = self.__stack.head.data.tree\n self.__base = self.__stack.base\n@@ -232,10 +231,9 @@ class StackTransaction(object):\n self.__stack.patchorder.hidden = self.__hidden\n log.log_entry(self.__stack, msg)\n old_applied = self.__stack.patchorder.applied\n- write(self.__msg)\n- if self.__conflicting_push != None:\n- self.__patches = _TransPatchMap(self.__stack)\n- self.__conflicting_push()\n+ if not self.__conflicts:\n+ write(self.__msg)\n+ else:\n write(self.__msg + ' (CONFLICT)')\n if print_current_patch:\n _print_current_patch(old_applied, self.__applied)\n@@ -358,26 +356,25 @@ class StackTransaction(object):\n elif not merge_conflict and cd.is_nochange():\n s = 'empty'\n out.done(s)\n- def update():\n- if comm:\n- self.patches[pn] = comm\n- if pn in self.hidden:\n- x = self.hidden\n- else:\n- x = self.unapplied\n- del x[x.index(pn)]\n- self.applied.append(pn)\n+\n if merge_conflict:\n # We've just caused conflicts, so we must allow them in\n # the final checkout.\n self.__allow_conflicts = lambda trans: True\n+ self.__patches = _TransPatchMap(self.__stack)\n\n- # Save this update so that we can run it a little later.\n- self.__conflicting_push = update\n- self.__halt(\"%d merge conflict(s)\" % len(self.__conflicts))\n+ # Update the stack state\n+ if comm:\n+ self.patches[pn] = comm\n+ if pn in self.hidden:\n+ x = self.hidden\n else:\n- # Update immediately.\n- update()\n+ x = self.unapplied\n+ del x[x.index(pn)]\n+ self.applied.append(pn)\n+\n+ if merge_conflict:\n+ self.__halt(\"%d merge conflict(s)\" % len(self.__conflicts))\n\n def push_tree(self, pn):\n \"\"\"Push the named patch without updating its tree.\"\"\"\ndiff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh\nindex bd97b3a..45e86dc 100755\n--- a/t/t3101-reset-hard.sh\n+++ b/t/t3101-reset-hard.sh\n@@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '\n stg reset --hard master.stgit^~1 &&\n stg status a > actual.txt &&\n test_cmp expected.txt actual.txt &&\n- test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n+ test \"$(echo $(stg series))\" = \"+ p1 + p2 > p3\"\n '\n\n test_done\ndiff --git a/t/t3103-undo-hard.sh b/t/t3103-undo-hard.sh\nindex 2d0f382..df14b1f 100755\n--- a/t/t3103-undo-hard.sh\n+++ b/t/t3103-undo-hard.sh\n@@ -46,11 +46,11 @@ test_expect_success 'Try to undo without --hard' '\n\n cat > expected.txt < actual.txt &&\n test_cmp expected.txt actual.txt &&\n- test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\" &&\n+ test \"$(echo $(stg series))\" = \"+ p1 + p2 > p3\" &&\n test \"$(stg id)\" = \"$(stg id $(stg top))\"\n '\n"}]}