{"thread":{"id":"18660","subject":"[StGit PATCH] Convert \"pop\" to the lib infrastructure","startedAt":"2009-03-31T11:30:27Z","lastAt":"2009-04-03T10:36:07Z","messageCount":4,"participants":["Catalin Marinas","Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"110003","messageId":"20090331113027.2524.60993.stgit@pc1117.cambridge.arm.com","threadId":"18660","inReplyTo":null,"subject":"[StGit PATCH] Convert \"pop\" to the lib infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-31T11:30:27Z","receivedAt":"2009-03-31T11:30:27Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>\n---\n stgit/commands/pop.py |   67 ++++++++++++++++++++-----------------------------\n t/t3101-reset-hard.sh |    4 +--\n t/t3103-undo-hard.sh  |    4 +--\n 3 files changed, 32 insertions(+), 43 deletions(-)\n\ndiff --git a/stgit/commands/pop.py b/stgit/commands/pop.py\nindex 2c78ac2..eace090 100644\n--- a/stgit/commands/pop.py\n+++ b/stgit/commands/pop.py\n@@ -16,11 +16,10 @@ along with this program; if not, write to the Free Software\n Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n \"\"\"\n \n-import sys, os\n+from stgit.commands import common\n+from stgit.lib import transaction\n+from stgit import argparse\n from stgit.argparse import opt\n-from stgit.commands.common import *\n-from stgit.utils import *\n-from stgit import argparse, stack, git\n \n help = 'Pop one or more patches from the stack'\n kind = 'stack'\n@@ -40,50 +39,40 @@ options = [\n     opt('-a', '--all', action = 'store_true',\n         short = 'Pop all the applied patches'),\n     opt('-n', '--number', type = 'int',\n-        short = 'Pop the specified number of patches'),\n-    opt('-k', '--keep', action = 'store_true',\n-        short = 'Keep the local changes')]\n+        short = 'Pop the specified number of patches')\n+    ] + argparse.keep_option()\n \n-directory = DirectoryGotoToplevel(log = True)\n+directory = common.DirectoryHasRepositoryLib()\n \n def func(parser, options, args):\n-    \"\"\"Pop the topmost patch from the stack\n-    \"\"\"\n-    check_conflicts()\n-    check_head_top_equal(crt_series)\n+    \"\"\"Pop the given patches or the topmost one from the stack.\"\"\"\n+    stack = directory.repository.current_stack\n+    iw = stack.repository.default_iw\n+    clean_iw = (not options.keep and iw) or None\n+    trans = transaction.StackTransaction(stack, 'pop',\n+                                         check_clean_iw = clean_iw)\n \n-    if not options.keep:\n-        check_local_changes()\n-\n-    applied = crt_series.get_applied()\n-    if not applied:\n-        raise CmdException, 'No patches applied'\n+    if not trans.applied:\n+        raise common.CmdException('No patches applied')\n \n     if options.all:\n-        patches = applied\n+        patches = trans.applied\n     elif options.number:\n         # reverse it twice to also work with negative or bigger than\n         # the length numbers\n-        patches = applied[::-1][:options.number][::-1]\n-    elif len(args) == 0:\n-        patches = [applied[-1]]\n+        patches = trans.applied[::-1][:options.number][::-1]\n+    elif not args:\n+        patches = [trans.applied[-1]]\n     else:\n-        patches = parse_patches(args, applied, ordered = True)\n+        patches = common.parse_patches(args, trans.applied, ordered = True)\n \n     if not patches:\n-        raise CmdException, 'No patches to pop'\n-\n-    # pop to the most distant popped patch\n-    topop = applied[applied.index(patches[0]):]\n-    # push those not in the popped range\n-    topush = [p for p in topop if p not in patches]\n-\n-    if options.keep and topush:\n-        raise CmdException, 'Cannot pop arbitrary patches with --keep'\n-\n-    topop.reverse()\n-    pop_patches(crt_series, topop, options.keep)\n-    if topush:\n-        push_patches(crt_series, topush)\n-\n-    print_crt_patch(crt_series)\n+        raise common.CmdException('No patches to pop')\n+\n+    applied = [p for p in trans.applied if not p in set(patches)]\n+    unapplied = patches + trans.unapplied\n+    try:\n+        trans.reorder_patches(applied, unapplied, iw = iw)\n+    except transaction.TransactionException:\n+        pass\n+    return trans.run(iw)\ndiff --git a/t/t3101-reset-hard.sh b/t/t3101-reset-hard.sh\nindex 2807ba3..bd97b3a 100755\n--- a/t/t3101-reset-hard.sh\n+++ b/t/t3101-reset-hard.sh\n@@ -28,7 +28,7 @@ cat > expected.txt <<EOF\n C a\n EOF\n test_expect_success 'Pop middle patch, creating a conflict' '\n-    conflict_old stg pop p2 &&\n+    conflict stg pop p2 &&\n     stg status a > actual.txt &&\n     test_cmp expected.txt actual.txt &&\n     test \"$(echo $(stg series))\" = \"+ p1 > p3 - p2\"\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 - p3 - p2\"\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 599aa43..ce71668 100755\n--- a/t/t3103-undo-hard.sh\n+++ b/t/t3103-undo-hard.sh\n@@ -28,7 +28,7 @@ cat > expected.txt <<EOF\n C a\n EOF\n test_expect_success 'Pop middle patch, creating a conflict' '\n-    conflict_old stg pop p2 &&\n+    conflict stg pop p2 &&\n     stg status a > actual.txt &&\n     test_cmp expected.txt actual.txt &&\n     test \"$(echo $(stg series))\" = \"+ p1 > p3 - p2\"\n@@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '\n     stg undo --hard &&\n     stg status a > actual.txt &&\n     test_cmp expected.txt actual.txt &&\n-    test \"$(echo $(stg series))\" = \"> p1 - p3 - p2\"\n+    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n '\n \n test_done\n"},{"id":"110125","messageId":"20090401120515.GA30918@diana.vm.bytemark.co.uk","threadId":"18660","inReplyTo":"20090331113027.2524.60993.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH] Convert \"pop\" to the lib infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-04-01T12:05:15Z","receivedAt":"2009-04-01T12:05:15Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-31 12:30:27 +0100, Catalin Marinas wrote:\n\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 - p3 - p2\"\n> +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n>  '\n\nHmm, why this change in behavior? Something that should be noted in\nthe commit message?\n\n> @@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '\n>      stg undo --hard &&\n>      stg status a > actual.txt &&\n>      test_cmp expected.txt actual.txt &&\n> -    test \"$(echo $(stg series))\" = \"> p1 - p3 - p2\"\n> +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n>  '\n\nAnd I guess this is the same.\n\nOtherwise, this looks good.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"110225","messageId":"b0943d9e0904020920t1a5b87b3i6ac0b37fbcf2ec62@mail.gmail.com","threadId":"18660","inReplyTo":"20090401120515.GA30918@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH] Convert \"pop\" to the lib infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-04-02T16:20:45Z","receivedAt":"2009-04-02T16:20:45Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/4/1 Karl Hasselström <kha@treskal.com>:\n> On 2009-03-31 12:30:27 +0100, Catalin Marinas wrote:\n>\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 - p3 - p2\"\n>> +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n>>  '\n>\n> Hmm, why this change in behavior? Something that should be noted in\n> the commit message?\n>\n>> @@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '\n>>      stg undo --hard &&\n>>      stg status a > actual.txt &&\n>>      test_cmp expected.txt actual.txt &&\n>> -    test \"$(echo $(stg series))\" = \"> p1 - p3 - p2\"\n>> +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n>>  '\n>\n> And I guess this is the same.\n\nI think we now get a slightly different behaviour because of how the\ntransactions are generated with the new infrastructure. In the above\ncase, you have \"pop p2 p3\" and \"push p3\", the latter failing. The \"pop\np2 p3\" command results in the stack being \"> p1 - p2 - p3\" while \"push\np3\" performs a single step for pushing and reordering. The old push\ncaused a reorder followed by a push.\n\nSo I think I should place the push changes before the pop ones so that\npop itself doesn't fail.\n\nI'll try to push them tonight as I'll go on holiday soon for two weeks.\n\n-- \nCatalin\n"},{"id":"110273","messageId":"20090403103607.GA9113@diana.vm.bytemark.co.uk","threadId":"18660","inReplyTo":"b0943d9e0904020920t1a5b87b3i6ac0b37fbcf2ec62@mail.gmail.com","subject":"Re: [StGit PATCH] Convert \"pop\" to the lib infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-04-03T10:36:07Z","receivedAt":"2009-04-03T10:36:07Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-04-02 17:20:45 +0100, Catalin Marinas wrote:\n\n> 2009/4/1 Karl Hasselström <kha@treskal.com>:\n>\n> > On 2009-03-31 12:30:27 +0100, Catalin Marinas wrote:\n> >\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 - p3 - p2\"\n> > > +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n> > >  '\n> >\n> > Hmm, why this change in behavior? Something that should be noted\n> > in the commit message?\n> >\n> > > @@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '\n> > >      stg undo --hard &&\n> > >      stg status a > actual.txt &&\n> > >      test_cmp expected.txt actual.txt &&\n> > > -    test \"$(echo $(stg series))\" = \"> p1 - p3 - p2\"\n> > > +    test \"$(echo $(stg series))\" = \"> p1 - p2 - p3\"\n> > >  '\n> >\n> > And I guess this is the same.\n>\n> I think we now get a slightly different behaviour because of how the\n> transactions are generated with the new infrastructure. In the above\n> case, you have \"pop p2 p3\" and \"push p3\", the latter failing. The\n> \"pop p2 p3\" command results in the stack being \"> p1 - p2 - p3\"\n> while \"push p3\" performs a single step for pushing and reordering.\n> The old push caused a reorder followed by a push.\n\nAh, OK. Hmm, I guess either behavior has its pros and cons. (Though I\nguess the new behavior -- not changing the order when the push failed\n-- might be slightly more intuitive.)\n\nAdd that explanation to the commit message, and I'll award you a\n\nAcked-by: Karl Hasselström <kha@treskal.com>\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"}]}