{"thread":{"id":"19406","subject":"[StGit PATCH] Add a --tree flag to stg push","startedAt":"2009-05-18T14:50:18Z","lastAt":"2009-05-19T10:27:43Z","messageCount":6,"participants":["David Kågedal","Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"114186","messageId":"20090518144754.30487.84132.stgit@krank","threadId":"19406","inReplyTo":null,"subject":"[StGit PATCH] Add a --tree flag to stg push","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2009-05-18T14:50:18Z","receivedAt":"2009-05-18T14:50:18Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"This flag makes the push simply restore the tree that the patch used\nbefore, rather than doing any kind of merge.\n---\n\nThis scratches a long-time itch for me. The typical use case is when\nyou want to break up a larg patch inte smaller ones. You back out the\norignal patch, apply a small set of changes from it and then push the\npatch back again. But then you don't want to do a merge, with the\npossibility of conflict. You simply want to restore to the tree that\nthe patch had before so you can see what's left to create cleaned-up\npatches of.  The command \"stg push --tree\" does just that.\n\nThe naming of flags and functions isn't very obvious, and suggestions\nfor improvements are welcome.\n\n stgit/commands/push.py   |   26 ++++++++++++-------\n stgit/lib/transaction.py |   28 ++++++++++++++++++++\n t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 108 insertions(+), 10 deletions(-)\n create mode 100755 t/t1207-push-tree.sh\n\ndiff --git a/stgit/commands/push.py b/stgit/commands/push.py\nindex 0d25a65..8d4d3fc 100644\n--- a/stgit/commands/push.py\n+++ b/stgit/commands/push.py\n@@ -43,7 +43,9 @@ options = [\n     opt('-n', '--number', type = 'int',\n         short = 'Push the specified number of patches'),\n     opt('--reverse', action = 'store_true',\n-        short = 'Push the patches in reverse order')\n+        short = 'Push the patches in reverse order'),\n+    opt('--tree', action = 'store_true',\n+        short = 'Push the patch with the original tree')\n     ] + argparse.keep_option() + argparse.merged_option()\n \n directory = common.DirectoryHasRepositoryLib()\n@@ -74,14 +76,18 @@ def func(parser, options, args):\n     if options.reverse:\n         patches.reverse()\n \n-    try:\n-        if options.merged:\n-            merged = set(trans.check_merged(patches))\n-        else:\n-            merged = set()\n+    if options.tree:\n         for pn in patches:\n-            trans.push_patch(pn, iw, allow_interactive = True,\n-                             already_merged = pn in merged)\n-    except transaction.TransactionHalted:\n-        pass\n+            trans.push_tree(pn)\n+    else:\n+        try:\n+            if options.merged:\n+                merged = set(trans.check_merged(patches))\n+            else:\n+                merged = set()\n+            for pn in patches:\n+                trans.push_patch(pn, iw, allow_interactive = True,\n+                                 already_merged = pn in merged)\n+        except transaction.TransactionHalted:\n+            pass\n     return trans.run(iw)\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 4148ff3..1c21938 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -372,6 +372,34 @@ class StackTransaction(object):\n             # Update immediately.\n             update()\n \n+    def push_tree(self, pn):\n+        \"\"\"Push the named patch without updating its tree.\"\"\"\n+        orig_cd = self.patches[pn].data\n+        cd = orig_cd.set_committer(None)\n+        oldparent = cd.parent\n+        cd = cd.set_parent(self.top)\n+\n+        s = ''\n+        if any(getattr(cd, a) != getattr(orig_cd, a) for a in\n+               ['parent', 'tree', 'author', 'message']):\n+            comm = self.__stack.repository.commit(cd)\n+            self.head = comm\n+        else:\n+            comm = None\n+            s = ' (unmodified)'\n+        if cd.is_nochange():\n+            s = ' (empty)'\n+        out.info('Pushed %s%s' % (pn, s))\n+\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     def reorder_patches(self, applied, unapplied, hidden = None, iw = None):\n         \"\"\"Push and pop patches to attain the given ordering.\"\"\"\n         if hidden is None:\ndiff --git a/t/t1207-push-tree.sh b/t/t1207-push-tree.sh\nnew file mode 100755\nindex 0000000..83f5cbf\n--- /dev/null\n+++ b/t/t1207-push-tree.sh\n@@ -0,0 +1,64 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 David Kågedal\n+#\n+\n+test_description='Exercise pushing patches with --tree.'\n+\n+. ./test-lib.sh\n+\n+# don't need this repo, but better not drop it, see t1100\n+#rm -rf .git\n+\n+# Need a repo to clone\n+test_create_repo foo\n+\n+test_expect_success \\\n+    'Create initial patches' '\n+    (\n+        cd foo &&\n+        stg init &&\n+        stg new A -m A &&\n+        echo hello world > a &&\n+        git add a &&\n+        stg refresh\n+        stg new B -m B &&\n+        echo HELLO WORLD > a &&\n+        stg refresh\n+    )\n+'\n+\n+test_expect_success \\\n+    'Back up and create a partial patch' '\n+    (\n+        cd foo &&\n+        stg pop &&\n+        stg new C -m C &&\n+        echo hello WORLD > a &&\n+        stg refresh\n+    )\n+'\n+\n+test_expect_success \\\n+    'Reapply patch B' '\n+    (\n+        cd foo &&\n+        stg push --tree B\n+    )\n+'\n+\n+test_expect_success \\\n+    'Compare results' '\n+    (\n+        cd foo &&\n+        stg pop -a &&\n+        stg push &&\n+        test \"$(echo $(cat a))\" = \"hello world\" &&\n+        stg push &&\n+        test \"$(echo $(cat a))\" = \"hello WORLD\" &&\n+        stg push &&\n+        test \"$(echo $(cat a))\" = \"HELLO WORLD\"\n+    )\n+'\n+\n+test_done\n"},{"id":"114236","messageId":"20090519072512.GA8451@diana.vm.bytemark.co.uk","threadId":"19406","inReplyTo":"20090518144754.30487.84132.stgit@krank","subject":"Re: [StGit PATCH] Add a --tree flag to stg push","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-05-19T07:25:12Z","receivedAt":"2009-05-19T07:25:12Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-05-18 16:50:18 +0200, David Kågedal wrote:\n\n> This scratches a long-time itch for me. The typical use case is when\n> you want to break up a larg patch inte smaller ones. You back out\n> the orignal patch, apply a small set of changes from it and then\n> push the patch back again. But then you don't want to do a merge,\n> with the possibility of conflict. You simply want to restore to the\n> tree that the patch had before so you can see what's left to create\n> cleaned-up patches of. The command \"stg push --tree\" does just that.\n\nThanks!\n\nThere's no sign-off.\n\n> The naming of flags and functions isn't very obvious, and\n> suggestions for improvements are welcome.\n\n--set-tree maybe?\n\n>  t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++\n\nA test! Very good.\n\n> +    opt('--tree', action = 'store_true',\n> +        short = 'Push the patch with the original tree')\n\nThis probably deserves a long description as well. (That most existing\noptions lack them is unfortunate---the support for long descriptions\nwas added rather recently.)\n\n> +        if any(getattr(cd, a) != getattr(orig_cd, a) for a in\n> +               ['parent', 'tree', 'author', 'message']):\n> +            comm = self.__stack.repository.commit(cd)\n> +            self.head = comm\n> +        else:\n> +            comm = None\n> +            s = ' (unmodified)'\n\nShouldn't self.head be set in both cases?\n\n> +# Copyright (c) 2006 David Kågedal\n\nBeen sitting on this patch long? :-)\n\n> +# don't need this repo, but better not drop it, see t1100\n> +#rm -rf .git\n> +\n> +# Need a repo to clone\n> +test_create_repo foo\n\nUmm, your test doesn't seem to depend on using this separate repo\ninstead of the default one.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"114238","messageId":"87d4a5fs59.fsf@krank.kagedal.org","threadId":"19406","inReplyTo":"20090519072512.GA8451@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH] Add a --tree flag to stg push","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2009-05-19T07:50:26Z","receivedAt":"2009-05-19T07:50:26Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"Karl Hasselström <kha@treskal.com> writes:\n\n> On 2009-05-18 16:50:18 +0200, David Kågedal wrote:\n>\n>> This scratches a long-time itch for me. The typical use case is when\n>> you want to break up a larg patch inte smaller ones. You back out\n>> the orignal patch, apply a small set of changes from it and then\n>> push the patch back again. But then you don't want to do a merge,\n>> with the possibility of conflict. You simply want to restore to the\n>> tree that the patch had before so you can see what's left to create\n>> cleaned-up patches of. The command \"stg push --tree\" does just that.\n>\n> Thanks!\n>\n> There's no sign-off.\n\nI counted on getting comments, so it's not finished yet...\n\n>> The naming of flags and functions isn't very obvious, and\n>> suggestions for improvements are welcome.\n>\n> --set-tree maybe?\n\nProbably better. But perhaps there is a way to not have to talk about\n\"trees\" at all?\n\n>>  t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++\n>\n> A test! Very good.\n>\n>> +    opt('--tree', action = 'store_true',\n>> +        short = 'Push the patch with the original tree')\n>\n> This probably deserves a long description as well. (That most existing\n> options lack them is unfortunate---the support for long descriptions\n> was added rather recently.)\n\nI didn't look putside push.py, and just followed the pattern\nthere. But a long description sounds like a good idea. It won't be\nobvious what this does with just a short one.\n\n>> +        if any(getattr(cd, a) != getattr(orig_cd, a) for a in\n>> +               ['parent', 'tree', 'author', 'message']):\n>> +            comm = self.__stack.repository.commit(cd)\n>> +            self.head = comm\n>> +        else:\n>> +            comm = None\n>> +            s = ' (unmodified)'\n>\n> Shouldn't self.head be set in both cases?\n\nI guess so. I'm a bit unsure about the correctness of that whole\nfunction.\n\n>> +# Copyright (c) 2006 David Kågedal\n>\n> Been sitting on this patch long? :-)\n\nCopy/paste error.\n\n>> +# don't need this repo, but better not drop it, see t1100\n>> +#rm -rf .git\n>> +\n>> +# Need a repo to clone\n>> +test_create_repo foo\n>\n> Umm, your test doesn't seem to depend on using this separate repo\n> instead of the default one.\n\nCall it copy/paste programming or cargo cult programming. I will clean\nup.\n\n-- \nDavid Kågedal\n"},{"id":"114239","messageId":"20090519080338.GB8451@diana.vm.bytemark.co.uk","threadId":"19406","inReplyTo":"87d4a5fs59.fsf@krank.kagedal.org","subject":"Re: [StGit PATCH] Add a --tree flag to stg push","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-05-19T08:03:38Z","receivedAt":"2009-05-19T08:03:38Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-05-19 09:50:26 +0200, David Kågedal wrote:\n\n> Karl Hasselström <kha@treskal.com> writes:\n>\n> > There's no sign-off.\n>\n> I counted on getting comments, so it's not finished yet...\n\nThe sign-off has nothing to do with the patch being finished. It's\nsimply you saying \"I promise I have the right to distribute the\nfollowing patch under GPLv2.\" See Documentation/SubmittingPatches\n(which we've stolen from git and hardly changed).\n\n> > --set-tree maybe?\n>\n> Probably better. But perhaps there is a way to not have to talk\n> about \"trees\" at all?\n\nHmm, maybe. Personally, I find that \"tree\" is a very descriptive word\nfor this operation, but that may be because I know git too well.\n\nSomething like \"overwrite\" maybe?\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"114245","messageId":"20090519093506.22242.59442.stgit@krank","threadId":"19406","inReplyTo":"20090519072512.GA8451@diana.vm.bytemark.co.uk","subject":"[StGit PATCH v2] Add a --set-tree flag to stg push","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2009-05-19T09:35:48Z","receivedAt":"2009-05-19T09:35:48Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"This flag makes the push simply restore the tree that the patch used\nbefore, rather than doing any kind of merge.\n\nSigned-off-by: David Kågedal <davidk@lysator.liu.se>\n---\nHere's an updated patch, based on feedback and discussion with Karl.\n\n stgit/commands/push.py   |   34 ++++++++++++++++++++++++----------\n stgit/lib/transaction.py |   22 ++++++++++++++++++++++\n t/t1207-push-tree.sh     |   46 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 92 insertions(+), 10 deletions(-)\n create mode 100755 t/t1207-push-tree.sh\n\ndiff --git a/stgit/commands/push.py b/stgit/commands/push.py\nindex 0d25a65..dbecf1f 100644\n--- a/stgit/commands/push.py\n+++ b/stgit/commands/push.py\n@@ -43,7 +43,17 @@ options = [\n     opt('-n', '--number', type = 'int',\n         short = 'Push the specified number of patches'),\n     opt('--reverse', action = 'store_true',\n-        short = 'Push the patches in reverse order')\n+        short = 'Push the patches in reverse order'),\n+    opt('--set-tree', action = 'store_true',\n+        short = 'Push the patch with the original tree', long = \"\"\"\n+        Push the patches, but don't perform a merge. Instead, the\n+        resulting tree will be identical to the tree that the patch\n+        previously created. This can be useful when splitting a patch\n+        by first popping the patch and creating a new patch with some\n+        of the changes. Pushing the original patch with --set-tree\n+        will avoid conflicts and only the remaining changes will be in\n+        the patch.\n+        \"\"\")\n     ] + argparse.keep_option() + argparse.merged_option()\n \n directory = common.DirectoryHasRepositoryLib()\n@@ -74,14 +84,18 @@ def func(parser, options, args):\n     if options.reverse:\n         patches.reverse()\n \n-    try:\n-        if options.merged:\n-            merged = set(trans.check_merged(patches))\n-        else:\n-            merged = set()\n+    if options.set_tree:\n         for pn in patches:\n-            trans.push_patch(pn, iw, allow_interactive = True,\n-                             already_merged = pn in merged)\n-    except transaction.TransactionHalted:\n-        pass\n+            trans.push_tree(pn)\n+    else:\n+        try:\n+            if options.merged:\n+                merged = set(trans.check_merged(patches))\n+            else:\n+                merged = set()\n+            for pn in patches:\n+                trans.push_patch(pn, iw, allow_interactive = True,\n+                                 already_merged = pn in merged)\n+        except transaction.TransactionHalted:\n+            pass\n     return trans.run(iw)\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 4148ff3..bce3df1 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -372,6 +372,28 @@ class StackTransaction(object):\n             # Update immediately.\n             update()\n \n+    def push_tree(self, pn):\n+        \"\"\"Push the named patch without updating its tree.\"\"\"\n+        orig_cd = self.patches[pn].data\n+        cd = orig_cd.set_committer(None).set_parent(self.top)\n+\n+        s = ''\n+        if any(getattr(cd, a) != getattr(orig_cd, a) for a in\n+               ['parent', 'tree', 'author', 'message']):\n+            self.patches[pn] = self.__stack.repository.commit(cd)\n+        else:\n+            s = ' (unmodified)'\n+        if cd.is_nochange():\n+            s = ' (empty)'\n+        out.info('Pushed %s%s' % (pn, s))\n+\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     def reorder_patches(self, applied, unapplied, hidden = None, iw = None):\n         \"\"\"Push and pop patches to attain the given ordering.\"\"\"\n         if hidden is None:\ndiff --git a/t/t1207-push-tree.sh b/t/t1207-push-tree.sh\nnew file mode 100755\nindex 0000000..9d0b1cc\n--- /dev/null\n+++ b/t/t1207-push-tree.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2009 David Kågedal\n+#\n+\n+test_description='Exercise pushing patches with --set-tree.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \\\n+    'Create initial patches' '\n+    stg init &&\n+    stg new A -m A &&\n+    echo hello world > a &&\n+    git add a &&\n+    stg refresh\n+    stg new B -m B &&\n+    echo HELLO WORLD > a &&\n+    stg refresh\n+'\n+\n+test_expect_success \\\n+    'Back up and create a partial patch' '\n+    stg pop &&\n+    stg new C -m C &&\n+    echo hello WORLD > a &&\n+    stg refresh\n+'\n+\n+test_expect_success \\\n+    'Reapply patch B' '\n+    stg push --set-tree B\n+'\n+\n+test_expect_success \\\n+    'Compare results' '\n+    stg pop -a &&\n+    stg push &&\n+    test \"$(echo $(cat a))\" = \"hello world\" &&\n+    stg push &&\n+    test \"$(echo $(cat a))\" = \"hello WORLD\" &&\n+    stg push &&\n+    test \"$(echo $(cat a))\" = \"HELLO WORLD\"\n+'\n+\n+test_done\n"},{"id":"114248","messageId":"20090519102743.GA11135@diana.vm.bytemark.co.uk","threadId":"19406","inReplyTo":"20090519093506.22242.59442.stgit@krank","subject":"Re: [StGit PATCH v2] Add a --set-tree flag to stg push","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-05-19T10:27:43Z","receivedAt":"2009-05-19T10:27:43Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-05-19 11:35:48 +0200, David Kågedal wrote:\n\n> +    opt('--set-tree', action = 'store_true',\n> +        short = 'Push the patch with the original tree', long = \"\"\"\n> +        Push the patches, but don't perform a merge. Instead, the\n> +        resulting tree will be identical to the tree that the patch\n> +        previously created. This can be useful when splitting a patch\n\nParagraph break after \"created\"?\n\n> +        by first popping the patch and creating a new patch with some\n> +        of the changes. Pushing the original patch with --set-tree\n> +        will avoid conflicts and only the remaining changes will be in\n\nThe long description is fed to asciidoc, which will translate -- to an\nen dash if I recall correctly. The long descriptions of other\ncommands' flags enclose flags in single quotes.\n\n> +        the patch.\n> +        \"\"\")\n\nUnnecessary line break.\n\nApart from those nitpicks,\n\nAcked-by: Karl Hasselström <kha@treskal.com>\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"}]}