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

From: Karl Hasselström <kha@treskal.com>
Date: Sep 16, 2008, 19:36 UTC
Message-ID: <20080916193647.GA12513@diana.vm.bytemark.co.uk>
In-Reply-To: <b0943d9e0809160759w5c9be510t3b33d5d983bff5a7@mail.gmail.com>

On 2008-09-16 15:59:31 +0100, Catalin Marinas wrote:

Show 11 quoted lines

> 2008/9/16 Karl Hasselström <kha@treskal.com>:
>
> > 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.

It wouldn't be clean of it to do that -- it would be accessing non-local state it had no business knowing about. I try hard to avoid that kind of thing.

Show 20 quoted lines

> > ( 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.

There are two patches that depend on new git versions. One needs git 1.5.6, which is in testing so I'll be pushing that to you; the other needs Junio's master branch, and i won't even consider asking you to take it until it's in a released git.

I hope to push it out to you tonight or tomorrow, but I have a small pet patch I'd like to finish first, so I might be late.

Show 6 quoted lines

> 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.

When benchmarking recent StGits, you'll want to try goto as well, since push and pop are not yet new-infrastructure-ized (meaning they're getting slowdowns from the stack log, but no speedups (if any) from the new infrastructure).

Show 12 quoted lines

> > On 2008-09-15 17:44:38 +0100, Catalin Marinas wrote:
> >
> > > +    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).

But passing iw = None means telling the transaction that you have no index+worktree, so it won't touch them. You'll get no detection of dirty tree, or checkout of result tree in the end. Which is not what you indended, IIUC.

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