2008/9/16 Karl Hasselström <kha@xxxxxxxxxxx>:
> On 2008-09-15 17:44:38 +0100, Catalin Marinas wrote:
>
>> 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.
>
> In my experimental branch, one of the patches adds the following piece
> of documentation:
>
> + @param allow_conflicts: Whether to allow pre-existing conflicts
> + @type allow_conflicts: bool or function of L{StackTransaction}"""
>
> That is, allow_conflicts decides whether to abort the transaction in
> case there already were conflicts -- undo and friends need to allow
> existing conflicts, but most other commands just want to abort in that
> case.
OK, it is clearer now.
> iw is the index+worktree object. The idea is that you provide one if
> your branch is checked out, and not if not. Operations that have no
> need of index+worktree, like pop, and push in case automatic merging
> succeeds, will just work anyway, while operations that need
> index+worktree, such as a conflicting push, will cause the whole
> transaction to abort.
Ah, that's the difference. I thought that even if iw isn't passed, it
uses the default one.
> ( Oh, and note that what I just said talks about the "patch stack
> log", meaning that I'm talking about the code in kha/experimental.
> The code in kha/safe doesn't look quite the same -- in particular,
> there's no obvious place to place code that ignores the conflicting
> push. Unless you really don't want your sink changes to depend on
> the stack log stuff (e.g. because you doubt you'll be merging it
> anytime soon), I suggest we do this: I'll prepare, and ask you to
> pull, a "stacklog" branch, and once you've pulled it we won't rebase
> it anymore. You can merge it directly to your master or publish it
> as a separate development branch, whichever you feel is best. )
I think we could merge your experimental branch into master. I gave it
a try and seems OK. The only issue I had was that I had an older
version of Git and it failed in really weird ways (stg pop still
busy-looping after 4 minutes and in another case it failed with broken
pipe). Once I pulled the latest Git, it was fine but we should try to
be compatible at least with the Git version in the Debian testing
distribution. It might be the patch at the top with diff-ing several
trees at once but I haven't checked.
BTW, I ran some benchmarks on stable/master/kha-experimental branches
with 300 patches from the 2.6.27-rc5-mm1 kernel. See attached for the
results. Since performance was my worry with the stack log stuff, it
turns out that there isn't a big difference with real patches. I think
pushing can be made even faster by trying a git-apply first and taking
the diff from the saved blobs in the log.
>> 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).
>
> Do you mean automatically detected dependencies, or dependencies that
> the user has told us about?
Automatic dependency - if two patches cannot be reorder with regards
to each-other, one of the depends on the other.
>> + if options.conflict:
>> + iw = stack.repository.default_iw
>> + else:
>> + iw = None
>
> As I said above, this doesn't (or at least isn't supposed to) work.
It should work since the trans.run() command without iw is equivalent
to trans.run(iw=None).
> Have you seen the reorder_patches() function last in transaction.py?
> It seems you could save a lot of work here by using it.
No, I haven't. I'll have a look.
>> + except transaction.TransactionHalted:
>> + if not options.conflict:
>> + raise
>
> Not catching TransactionHalted will have the effect of rolling back
> the whole transaction if it stops half-way through. But what you
> really wanted was the new flag I described above, I think.
OK, if you prepare the stack log, I'll merge it and have a look.
Thanks.
--
Catalin
CPU: Intel Pentium 4 @ 2.5GHz
Memory: 1GB
2.6.27-rc5-mm1 kernel, 300 patches uncommitted
pop/push ran a few times to heat the caches before running the
benchmarks.
Stable stgit (v0.14.3 + some fixes)
$ time stg pop -a
real 0m1.775s
user 0m0.956s
sys 0m0.724s
$ time stg push -a (fast-forward)
real 0m5.001s
user 0m1.844s
sys 0m2.860s
$ time stg push -a (no fast-forward)
real 1m27.133s
user 0m36.998s
sys 0m34.894s
Current stgit master (no stack log):
$ time stg pop -a
real 0m1.621s
user 0m0.820s
sys 0m0.688s
$ time stg push -a (fast-forward)
real 0m27.205s
user 0m8.741s
sys 0m16.849s
$ time stg push -a (no fast-forward)
real 2m8.209s
user 0m46.031s
sys 0m57.260s
kha/experimantal stgit (with stack log):
$ time stg pop -a
real 0m2.419s
user 0m1.144s
sys 0m1.132s
$ time stg push -a (fast-forward)
real 0m29.594s
user 0m9.217s
sys 0m17.145s
$ time stg push -a (no fast-forward)
real 2m10.270s
user 0m50.919s
sys 1m2.088s
