From: Catalin Marinas Date: Mon, 15 Sep 2008 16:44:38 GMT Subject: Re: [StGit PATCH] Convert "sink" to the new infrastructure Message-ID: In-Reply-To: <20080915075740.GB14452@diana.vm.bytemark.co.uk> 2008/9/15 Karl Hasselström : > 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. >> 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 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 --- 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 +Copyright (C) 2008, Catalin Marinas 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 ] [-n] [] @@ -32,43 +29,72 @@ push the specified (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