Re: [StGit PATCH 03/14] Write to a stack log when stack is modified

From: Karl Hasselström <kha@treskal.com>
Date: Jun 18, 2008, 17:32 UTC
Message-ID: <20080618173246.GA1155@diana.vm.bytemark.co.uk>
In-Reply-To: <b0943d9e0806180916m4af3970ck347408661e95663@mail.gmail.com>

On 2008-06-18 17:16:10 +0100, Catalin Marinas wrote:

Show 8 quoted lines

> 2008/6/18 Karl Hasselström <kha@treskal.com>:
>
> > Do you mean remove hidden patches from StGit altogether, or just
> > not store them in the log?
>
> Remove them altogether. Is there anyone using them apart from me? I
> could create a "rotten" branch and pick those patches with
> --unapplied.

I've never heard about anyone else using them. So fine by me!

> It gets too complicated, really. A single commit with the proper
> parents could do the job.

Yes. The problem is that you need quite a lot of parents. Every system I could come up with that got all the corner cases right was even more complex (in my eyes) than the one I ended up implementing.

> We could also easily use the commit message directly for metadata
> instead of .git/patches/<branch>/.

Yes. This is true for any log scheme, though. (But I agree -- longer term, it would be very nice to have _only_ the log.)

> The time to create that tree and blobs worries me a bit, plus the
> (in my view) complicated structure.

You're right to worry. My log format makes things feel slightly slower when logging a 20-30 deep stack. If I can't make it faster, it's not viable. But I'm pretty sure I can -- it should be simple to reuse all the trees and blobs for the patches that weren't touched. (And operations that touch lots of patches, like rebase, have to write a lot of git objects anyway as part of their operation, so the relative slowdown should not be large.)

Anyway, benchmarking is the way to go here. Talk will only get us that far.

> Making the first log entry special gets difficult with log pruning
> (unless you prune the whole log rather than entries older than a
> chosen time or number) since you might have to re-create all the
> chained log entries as the first log's sha1 will probably change.

You have to re-create all the commits anyway, since they all are immutable, and all have a back pointer.

Show 5 quoted lines

> The applied patches are chained automatically via HEAD. For
> unapplied patches, we could add the correponding commits as parents
> of the logging commit (starting with the third parent as the first
> two are used for log chaining and applied patches). Do we hit any OS
> limit with the number of arguments?

Not until long after we hit git limits to the number of parents of a commit. I believe the octopus merge refuses to create merges with more than about 25 parents, and we probably shouldn't do more than that either. We'll have to do a tree of octopuses.

This is why my log format looks the way it does. :-)

> Since we only need the unapplied commits tracking for gc and pull
> reasons, we could only create that commit that references them when
> we prune the stack log and link it from the top one (but it won't be
> used by stgit).

Yes, we need to create an "unapplied octopus" if and only if we have unapplied patches that we can't prove are reachable from the stack top or the branch head (we have to save both, in case the user has done something such as git-committing on topp of the stack and caused head != top). Which is for the first log entry, and in situations such as "stg pick --unapplied", but not for "stg pop" and the like.

I do agree that we shouldn't try to use the octopuses to get hold of the commits, though -- just to keep them reachable. We save the sha1 along with the patch name elsewhere in a more convenient form. (My proposed format does precisely this.)

So. If I got it right, your proposal is:

  * Tree: just take the HEAD tree.

  * Commit message: list the applied and unapplied patches with their
    commit sha1s.

  * Parents: the previous log entry; branch head; something that
    (recusively) points to all unapplied commits, if necessary.

I'd add to that:

  * The stack top must be a parent too if head != top.

  * The commit message must include a version number, and the branch
    head sha1.

  * I'm pretty sure we want the kind of "simplified" log I have in my
    proposal. The full log in your proposal is going to look every bit
    as ugly as the one in mine.

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