git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [StGit PATCH] Convert "sink" to the new infrastructure

From
CMCatalin Marinas <catalin.marinas@gmail.com>
Date
Sep 15, 2008, 16:44 UTC
Message-ID
<b0943d9e0809150944o71acafe7ndeda500b1fba97df@mail.gmail.com>
In-Reply-To
<20080915075740.GB14452@diana.vm.bytemark.co.uk>
2008/9/15 Karl Hasselström <kha@treskal.com>:
Show 7 quoted lines
> On 2008-09-14 22:19:41 +0100, Catalin Marinas wrote:
>
>> I wasn't used to reading documentation in StGit files :-). Thanks
>> for the info, I'll repost.
>
> It was you who asked for in-code docs. :-) The new-infrastructure code
> actually looks half decent in epydoc nowadays.

Since we are talking about this, the transactions documentation doesn't explain when to use a iw and when to pass allow_conflicts. I kind of figured out but I'm not convinced. At a first look, passing allow_conflicts = True would seem that it may allow conflicts and not revert the changes, however, this only works if I pass an "iw". But passing it doesn't allow the default case where I want the changes reverted.

Please have a look at the attached patch which is my last version of the sink command rewriting. I'm not that happy (or maybe I don't understand the reasons) with setting iw = None if not options.conflict but that's the way I could get it to work.

Show 5 quoted lines
>> I'll make the default behaviour to cancel the transaction and revert
>> to the original state unless an option is given to allow conflicts.
>
> What I've always wanted is "sink this patch as far as it will go
> without conflicting". This comes awfully close.

But this means that sink would try several consecutive sinks until it can't find one. Not that it is try to implement but I wouldn't complicate "sink" for this. I would rather add support for patch dependency tracking (which used to be on the long term wish list). It might be useful for other things as well like mailing a patch together with those on which it depends (like darcs).

> BTW, this kind of flag might potentially be useful in many commands
> (with default value on or off depending on the command). Maybe
>
>  --conflicts=roll-back|stop-before|allow
ATM, I only added a --conflict option which has the "allow" meaning.
-- 
Catalin


Convert "sink" to the new infrastructure

From: Catalin Marinas <catalin.marinas@gmail.com>

This patch converts the sink command to use stgit.lib. By default, the
command doesn't allow conflicts and it cancels the operations if patches
cannot be reordered cleanly. With the --conflict options, the command
stops after the first conflict during the push operations.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
 stgit/commands/sink.py |   90 +++++++++++++++++++++++++++++++-----------------
 t/t1501-sink.sh        |   65 +++++++++++++++++++++++++++++------
 2 files changed, 112 insertions(+), 43 deletions(-)

diff --git a/stgit/commands/sink.py b/stgit/commands/sink.py
index d8f79b4..a799433 100644
--- a/stgit/commands/sink.py
+++ b/stgit/commands/sink.py
@@ -1,6 +1,6 @@
 
 __copyright__ = """
-Copyright (C) 2007, Yann Dirson <ydirson@altern.org>
+Copyright (C) 2008, Catalin Marinas <catalin.marinas@gmail.com>
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License version 2 as
@@ -16,13 +16,10 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
-from optparse import OptionParser, make_option
-
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit import stack, git
+from optparse import make_option
 
+from stgit.commands import common
+from stgit.lib import transaction
 
 help = 'send patches deeper down the stack'
 usage = """%prog [-t <target patch>] [-n] [<patches>]
@@ -32,43 +29,72 @@ push the specified <patches> (the current patch by default), and
 then push back into place the formerly-applied patches (unless -n
 is also given)."""
 
-directory = DirectoryGotoToplevel()
+directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-n', '--nopush',
                        help = 'do not push the patches back after sinking',
                        action = 'store_true'),
            make_option('-t', '--to', metavar = 'TARGET',
-                       help = 'sink patches below TARGET patch')]
+                       help = 'sink patches below TARGET patch'),
+           make_option('-c', '--conflict',
+                       help = 'allow conflicts during the push operations',
+                       action = 'store_true')]
 
 def func(parser, options, args):
     """Sink patches down the stack.
     """
+    stack = directory.repository.current_stack
 
-    check_local_changes()
-    check_conflicts()
-    check_head_top_equal(crt_series)
-
-    oldapplied = crt_series.get_applied()
-    unapplied = crt_series.get_unapplied()
-    all = unapplied + oldapplied
-
-    if options.to and not options.to in oldapplied:
-        raise CmdException('Cannot sink below %s, since it is not applied'
-                           % options.to)
+    if options.to and not options.to in stack.patchorder.applied:
+        raise common.CmdException('Cannot sink below %s since it is not applied'
+                                  % options.to)
 
     if len(args) > 0:
-        patches = parse_patches(args, all)
+        patches = common.parse_patches(args, stack.patchorder.all)
     else:
-        current = crt_series.get_current()
-        if not current:
-            raise CmdException('No patch applied')
-        patches = [current]
+        # current patch
+        patches = list(stack.patchorder.applied[-1:])
 
-    if oldapplied:
-        crt_series.pop_patch(options.to or oldapplied[0])
-    push_patches(crt_series, patches)
+    if not patches:
+        raise common.CmdException('No patches to sink')
+    if options.to and options.to in patches:
+        raise common.CmdException('Cannot have a sinked patch as target')
+
+    if options.conflict:
+        iw = stack.repository.default_iw
+    else:
+        iw = None
+    trans = transaction.StackTransaction(stack, 'sink')
+
+    # pop any patches to be sinked in case they are applied
+    to_push = trans.pop_patches(lambda pn: pn in patches)
+
+    if options.to:
+        if options.to in to_push:
+            # this is the case where sinking actually brings some
+            # patches forward
+            for p in to_push:
+                if p == options.to:
+                    del to_push[:to_push.index(p)]
+                    break
+                trans.push_patch(p, iw)
+        else:
+            # target patch below patches to be sinked
+            to_pop = trans.applied[trans.applied.index(options.to):]
+            to_push = to_pop + to_push
+            trans.pop_patches(lambda pn: pn in to_pop)
+    else:
+        # pop all the remaining patches
+        to_push = trans.applied + to_push
+        trans.pop_patches(lambda pn: True)
 
+    # push the sinked and other popped patches
     if not options.nopush:
-        newapplied = crt_series.get_applied()
-        def not_reapplied_yet(p):
-            return not p in newapplied
-        push_patches(crt_series, filter(not_reapplied_yet, oldapplied))
+        patches.extend(to_push)
+    try:
+        for p in patches:
+            trans.push_patch(p, iw)
+    except transaction.TransactionHalted:
+        if not options.conflict:
+            raise
+
+    return trans.run(iw)
diff --git a/t/t1501-sink.sh b/t/t1501-sink.sh
index 32931cd..b3e2eb3 100755
--- a/t/t1501-sink.sh
+++ b/t/t1501-sink.sh
@@ -5,24 +5,67 @@ test_description='Test "stg sink"'
 . ./test-lib.sh
 
 test_expect_success 'Initialize StGit stack' '
-    echo 000 >> x &&
-    git add x &&
+    echo 0 >> f0 &&
+    git add f0 &&
     git commit -m initial &&
-    echo 000 >> y &&
-    git add y &&
-    git commit -m y &&
+    echo 1 >> f1 &&
+    git add f1 &&
+    git commit -m p1 &&
+    echo 2 >> f2 &&
+    git add f2 &&
+    git commit -m p2 &&
+    echo 3 >> f3 &&
+    git add f3 &&
+    git commit -m p3 &&
+    echo 4 >> f4 &&
+    git add f4 &&
+    git commit -m p4 &&
+    echo 22 >> f2 &&
+    git add f2 &&
+    git commit -m p22 &&
     stg init &&
-    stg uncommit &&
-    stg pop
+    stg uncommit p22 p4 p3 p2 p1 &&
+    stg pop -a
 '
 
-test_expect_success 'sink without applied patches' '
+test_expect_success 'sink default without applied patches' '
     command_error stg sink
 '
 
-test_expect_success 'sink a specific patch without applied patches' '
-    stg sink y &&
-    test $(echo $(stg series --applied --noprefix)) = "y"
+test_expect_success 'sink and reorder specified without applied patches' '
+    stg sink p2 p1 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p2 p1"
+'
+
+test_expect_success 'sink patches to the bottom of the stack' '
+    stg sink p4 p3 p2 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p4 p3 p2 p1"
+'
+
+test_expect_success 'sink current below a target' '
+    stg sink --to=p2 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p4 p3 p1 p2"
+'
+
+test_expect_success 'bring patches forward' '
+    stg sink --to=p2 p3 p4 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p1 p3 p4 p2"
+'
+
+test_expect_success 'sink specified patch below a target' '
+    stg sink --to=p3 p2 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p1 p2 p3 p4"
+'
+
+test_expect_success 'sink with conflict and restore the stack' '
+    command_error stg sink --to=p2 p22 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p1 p2 p3 p4"
+'
+
+test_expect_success 'sink with conflict and do not restore the stack' '
+    conflict stg sink --conflict --to=p2 p22 &&
+    test "$(echo $(stg series --applied --noprefix))" = "p1 p22" &&
+    test "$(echo $(stg status --conflict))" = "f2"
 '
 
 test_done
Previous: Karl HasselströmNext: Karl Hasselström
Message 5 of 18 in “Convert "sink" to the new infrastructure”
  1. Convert "sink" to the new infrastructureCatalin Marinas, Sep 12, 2008
  2. Karl HasselströmSep 14, 2008
  3. Catalin MarinasSep 14, 2008
  4. Karl HasselströmSep 15, 2008
  5. Catalin MarinasSep 15, 2008
  6. Karl HasselströmSep 16, 2008
  7. Catalin MarinasSep 16, 2008
  8. Karl HasselströmSep 16, 2008
  9. Catalin MarinasSep 17, 2008
  10. Karl HasselströmSep 17, 2008
  11. Karl HasselströmSep 17, 2008
  12. Catalin MarinasSep 17, 2008
  13. Karl HasselströmSep 18, 2008
  14. Catalin MarinasSep 18, 2008
  15. Catalin MarinasSep 17, 2008
  16. Karl HasselströmSep 18, 2008
  17. Catalin MarinasSep 18, 2008
  18. Karl HasselströmSep 18, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.