[StGit PATCH] Add a --tree flag to stg push

[StGit PATCH] Add a --tree flag to stg push

[David Kågedal]

This flag makes the push simply restore the tree that the patch used
before, rather than doing any kind of merge.
---

This scratches a long-time itch for me. The typical use case is when
you want to break up a larg patch inte smaller ones. You back out the
orignal patch, apply a small set of changes from it and then push the
patch back again. But then you don't want to do a merge, with the
possibility of conflict. You simply want to restore to the tree that
the patch had before so you can see what's left to create cleaned-up
patches of.  The command "stg push --tree" does just that.

The naming of flags and functions isn't very obvious, and suggestions
for improvements are welcome.

 stgit/commands/push.py   |   26 ++++++++++++-------
 stgit/lib/transaction.py |   28 ++++++++++++++++++++
 t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 10 deletions(-)
 create mode 100755 t/t1207-push-tree.sh

diff --git a/stgit/commands/push.py b/stgit/commands/push.py
index 0d25a65..8d4d3fc 100644
--- a/stgit/commands/push.py
+++ b/stgit/commands/push.py
@@ -43,7 +43,9 @@ options = [
     opt('-n', '--number', type = 'int',
         short = 'Push the specified number of patches'),
     opt('--reverse', action = 'store_true',
-        short = 'Push the patches in reverse order')
+        short = 'Push the patches in reverse order'),
+    opt('--tree', action = 'store_true',
+        short = 'Push the patch with the original tree')
     ] + argparse.keep_option() + argparse.merged_option()
 
 directory = common.DirectoryHasRepositoryLib()
@@ -74,14 +76,18 @@ def func(parser, options, args):
     if options.reverse:
         patches.reverse()
 
-    try:
-        if options.merged:
-            merged = set(trans.check_merged(patches))
-        else:
-            merged = set()
+    if options.tree:
         for pn in patches:
-            trans.push_patch(pn, iw, allow_interactive = True,
-                             already_merged = pn in merged)
-    except transaction.TransactionHalted:
-        pass
+            trans.push_tree(pn)
+    else:
+        try:
+            if options.merged:
+                merged = set(trans.check_merged(patches))
+            else:
+                merged = set()
+            for pn in patches:
+                trans.push_patch(pn, iw, allow_interactive = True,
+                                 already_merged = pn in merged)
+        except transaction.TransactionHalted:
+            pass
     return trans.run(iw)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 4148ff3..1c21938 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -372,6 +372,34 @@ class StackTransaction(object):
             # Update immediately.
             update()
 
+    def push_tree(self, pn):
+        """Push the named patch without updating its tree."""
+        orig_cd = self.patches[pn].data
+        cd = orig_cd.set_committer(None)
+        oldparent = cd.parent
+        cd = cd.set_parent(self.top)
+
+        s = ''
+        if any(getattr(cd, a) != getattr(orig_cd, a) for a in
+               ['parent', 'tree', 'author', 'message']):
+            comm = self.__stack.repository.commit(cd)
+            self.head = comm
+        else:
+            comm = None
+            s = ' (unmodified)'
+        if cd.is_nochange():
+            s = ' (empty)'
+        out.info('Pushed %s%s' % (pn, s))
+
+        if comm:
+            self.patches[pn] = comm
+        if pn in self.hidden:
+            x = self.hidden
+        else:
+            x = self.unapplied
+        del x[x.index(pn)]
+        self.applied.append(pn)
+
     def reorder_patches(self, applied, unapplied, hidden = None, iw = None):
         """Push and pop patches to attain the given ordering."""
         if hidden is None:
diff --git a/t/t1207-push-tree.sh b/t/t1207-push-tree.sh
new file mode 100755
index 0000000..83f5cbf
--- /dev/null
+++ b/t/t1207-push-tree.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 David Kågedal
+#
+
+test_description='Exercise pushing patches with --tree.'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Create initial patches' '
+    (
+        cd foo &&
+        stg init &&
+        stg new A -m A &&
+        echo hello world > a &&
+        git add a &&
+        stg refresh
+        stg new B -m B &&
+        echo HELLO WORLD > a &&
+        stg refresh
+    )
+'
+
+test_expect_success \
+    'Back up and create a partial patch' '
+    (
+        cd foo &&
+        stg pop &&
+        stg new C -m C &&
+        echo hello WORLD > a &&
+        stg refresh
+    )
+'
+
+test_expect_success \
+    'Reapply patch B' '
+    (
+        cd foo &&
+        stg push --tree B
+    )
+'
+
+test_expect_success \
+    'Compare results' '
+    (
+        cd foo &&
+        stg pop -a &&
+        stg push &&
+        test "$(echo $(cat a))" = "hello world" &&
+        stg push &&
+        test "$(echo $(cat a))" = "hello WORLD" &&
+        stg push &&
+        test "$(echo $(cat a))" = "HELLO WORLD"
+    )
+'
+
+test_done

Re: [StGit PATCH] Add a --tree flag to stg push

[Karl Hasselström]

On 2009-05-18 16:50:18 +0200, David Kågedal wrote:

> This scratches a long-time itch for me. The typical use case is when
> you want to break up a larg patch inte smaller ones. You back out
> the orignal patch, apply a small set of changes from it and then
> push the patch back again. But then you don't want to do a merge,
> with the possibility of conflict. You simply want to restore to the
> tree that the patch had before so you can see what's left to create
> cleaned-up patches of. The command "stg push --tree" does just that.

Thanks!

There's no sign-off.

> The naming of flags and functions isn't very obvious, and
> suggestions for improvements are welcome.

--set-tree maybe?

>  t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++

A test! Very good.

> +    opt('--tree', action = 'store_true',
> +        short = 'Push the patch with the original tree')

This probably deserves a long description as well. (That most existing
options lack them is unfortunate---the support for long descriptions
was added rather recently.)

> +        if any(getattr(cd, a) != getattr(orig_cd, a) for a in
> +               ['parent', 'tree', 'author', 'message']):
> +            comm = self.__stack.repository.commit(cd)
> +            self.head = comm
> +        else:
> +            comm = None
> +            s = ' (unmodified)'

Shouldn't self.head be set in both cases?

> +# Copyright (c) 2006 David Kågedal

Been sitting on this patch long? :-)

> +# don't need this repo, but better not drop it, see t1100
> +#rm -rf .git
> +
> +# Need a repo to clone
> +test_create_repo foo

Umm, your test doesn't seem to depend on using this separate repo
instead of the default one.

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

Re: [StGit PATCH] Add a --tree flag to stg push

[David Kågedal]

Karl Hasselström <kha@treskal.com> writes:

> On 2009-05-18 16:50:18 +0200, David Kågedal wrote:
>
>> This scratches a long-time itch for me. The typical use case is when
>> you want to break up a larg patch inte smaller ones. You back out
>> the orignal patch, apply a small set of changes from it and then
>> push the patch back again. But then you don't want to do a merge,
>> with the possibility of conflict. You simply want to restore to the
>> tree that the patch had before so you can see what's left to create
>> cleaned-up patches of. The command "stg push --tree" does just that.
>
> Thanks!
>
> There's no sign-off.

I counted on getting comments, so it's not finished yet...

>> The naming of flags and functions isn't very obvious, and
>> suggestions for improvements are welcome.
>
> --set-tree maybe?

Probably better. But perhaps there is a way to not have to talk about
"trees" at all?

>>  t/t1207-push-tree.sh     |   64 ++++++++++++++++++++++++++++++++++++++++++++++
>
> A test! Very good.
>
>> +    opt('--tree', action = 'store_true',
>> +        short = 'Push the patch with the original tree')
>
> This probably deserves a long description as well. (That most existing
> options lack them is unfortunate---the support for long descriptions
> was added rather recently.)

I didn't look putside push.py, and just followed the pattern
there. But a long description sounds like a good idea. It won't be
obvious what this does with just a short one.

>> +        if any(getattr(cd, a) != getattr(orig_cd, a) for a in
>> +               ['parent', 'tree', 'author', 'message']):
>> +            comm = self.__stack.repository.commit(cd)
>> +            self.head = comm
>> +        else:
>> +            comm = None
>> +            s = ' (unmodified)'
>
> Shouldn't self.head be set in both cases?

I guess so. I'm a bit unsure about the correctness of that whole
function.

>> +# Copyright (c) 2006 David Kågedal
>
> Been sitting on this patch long? :-)

Copy/paste error.

>> +# don't need this repo, but better not drop it, see t1100
>> +#rm -rf .git
>> +
>> +# Need a repo to clone
>> +test_create_repo foo
>
> Umm, your test doesn't seem to depend on using this separate repo
> instead of the default one.

Call it copy/paste programming or cargo cult programming. I will clean
up.

-- 
David Kågedal

Re: [StGit PATCH] Add a --tree flag to stg push

[Karl Hasselström]

On 2009-05-19 09:50:26 +0200, David Kågedal wrote:

> Karl Hasselström <kha@treskal.com> writes:
>
> > There's no sign-off.
>
> I counted on getting comments, so it's not finished yet...

The sign-off has nothing to do with the patch being finished. It's
simply you saying "I promise I have the right to distribute the
following patch under GPLv2." See Documentation/SubmittingPatches
(which we've stolen from git and hardly changed).

> > --set-tree maybe?
>
> Probably better. But perhaps there is a way to not have to talk
> about "trees" at all?

Hmm, maybe. Personally, I find that "tree" is a very descriptive word
for this operation, but that may be because I know git too well.

Something like "overwrite" maybe?

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

[StGit PATCH v2] Add a --set-tree flag to stg push

[David Kågedal]

This flag makes the push simply restore the tree that the patch used
before, rather than doing any kind of merge.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
Here's an updated patch, based on feedback and discussion with Karl.

 stgit/commands/push.py   |   34 ++++++++++++++++++++++++----------
 stgit/lib/transaction.py |   22 ++++++++++++++++++++++
 t/t1207-push-tree.sh     |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 10 deletions(-)
 create mode 100755 t/t1207-push-tree.sh

diff --git a/stgit/commands/push.py b/stgit/commands/push.py
index 0d25a65..dbecf1f 100644
--- a/stgit/commands/push.py
+++ b/stgit/commands/push.py
@@ -43,7 +43,17 @@ options = [
     opt('-n', '--number', type = 'int',
         short = 'Push the specified number of patches'),
     opt('--reverse', action = 'store_true',
-        short = 'Push the patches in reverse order')
+        short = 'Push the patches in reverse order'),
+    opt('--set-tree', action = 'store_true',
+        short = 'Push the patch with the original tree', long = """
+        Push the patches, but don't perform a merge. Instead, the
+        resulting tree will be identical to the tree that the patch
+        previously created. This can be useful when splitting a patch
+        by first popping the patch and creating a new patch with some
+        of the changes. Pushing the original patch with --set-tree
+        will avoid conflicts and only the remaining changes will be in
+        the patch.
+        """)
     ] + argparse.keep_option() + argparse.merged_option()
 
 directory = common.DirectoryHasRepositoryLib()
@@ -74,14 +84,18 @@ def func(parser, options, args):
     if options.reverse:
         patches.reverse()
 
-    try:
-        if options.merged:
-            merged = set(trans.check_merged(patches))
-        else:
-            merged = set()
+    if options.set_tree:
         for pn in patches:
-            trans.push_patch(pn, iw, allow_interactive = True,
-                             already_merged = pn in merged)
-    except transaction.TransactionHalted:
-        pass
+            trans.push_tree(pn)
+    else:
+        try:
+            if options.merged:
+                merged = set(trans.check_merged(patches))
+            else:
+                merged = set()
+            for pn in patches:
+                trans.push_patch(pn, iw, allow_interactive = True,
+                                 already_merged = pn in merged)
+        except transaction.TransactionHalted:
+            pass
     return trans.run(iw)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 4148ff3..bce3df1 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -372,6 +372,28 @@ class StackTransaction(object):
             # Update immediately.
             update()
 
+    def push_tree(self, pn):
+        """Push the named patch without updating its tree."""
+        orig_cd = self.patches[pn].data
+        cd = orig_cd.set_committer(None).set_parent(self.top)
+
+        s = ''
+        if any(getattr(cd, a) != getattr(orig_cd, a) for a in
+               ['parent', 'tree', 'author', 'message']):
+            self.patches[pn] = self.__stack.repository.commit(cd)
+        else:
+            s = ' (unmodified)'
+        if cd.is_nochange():
+            s = ' (empty)'
+        out.info('Pushed %s%s' % (pn, s))
+
+        if pn in self.hidden:
+            x = self.hidden
+        else:
+            x = self.unapplied
+        del x[x.index(pn)]
+        self.applied.append(pn)
+
     def reorder_patches(self, applied, unapplied, hidden = None, iw = None):
         """Push and pop patches to attain the given ordering."""
         if hidden is None:
diff --git a/t/t1207-push-tree.sh b/t/t1207-push-tree.sh
new file mode 100755
index 0000000..9d0b1cc
--- /dev/null
+++ b/t/t1207-push-tree.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 David Kågedal
+#
+
+test_description='Exercise pushing patches with --set-tree.'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'Create initial patches' '
+    stg init &&
+    stg new A -m A &&
+    echo hello world > a &&
+    git add a &&
+    stg refresh
+    stg new B -m B &&
+    echo HELLO WORLD > a &&
+    stg refresh
+'
+
+test_expect_success \
+    'Back up and create a partial patch' '
+    stg pop &&
+    stg new C -m C &&
+    echo hello WORLD > a &&
+    stg refresh
+'
+
+test_expect_success \
+    'Reapply patch B' '
+    stg push --set-tree B
+'
+
+test_expect_success \
+    'Compare results' '
+    stg pop -a &&
+    stg push &&
+    test "$(echo $(cat a))" = "hello world" &&
+    stg push &&
+    test "$(echo $(cat a))" = "hello WORLD" &&
+    stg push &&
+    test "$(echo $(cat a))" = "HELLO WORLD"
+'
+
+test_done

Re: [StGit PATCH v2] Add a --set-tree flag to stg push

[Karl Hasselström]

On 2009-05-19 11:35:48 +0200, David Kågedal wrote:

> +    opt('--set-tree', action = 'store_true',
> +        short = 'Push the patch with the original tree', long = """
> +        Push the patches, but don't perform a merge. Instead, the
> +        resulting tree will be identical to the tree that the patch
> +        previously created. This can be useful when splitting a patch

Paragraph break after "created"?

> +        by first popping the patch and creating a new patch with some
> +        of the changes. Pushing the original patch with --set-tree
> +        will avoid conflicts and only the remaining changes will be in

The long description is fed to asciidoc, which will translate -- to an
en dash if I recall correctly. The long descriptions of other
commands' flags enclose flags in single quotes.

> +        the patch.
> +        """)

Unnecessary line break.

Apart from those nitpicks,

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

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