On 2007-11-15 18:28:16 +0000, Catalin Marinas wrote:

quoted text
> I eventually managed to have a (not so in-depth) look at this

Thanks!

quoted text
> and I am OK with it (but merging after 0.14).

Yes, that seems like a good idea.

quoted text
> As you said the current structure and the new one will work in
> parallel for some time.

Yes. This is a very nice property that makes the whole enterprise
feasible.

quoted text
> It would even be possible to implement unit testing.

Yes. Unit testing would be easier with the new code, since it's more
modular.

quoted text
> Commitdata maybe should be written as CommitData (as in the e-mail
> text).

Probably. I've alredy made that spelling error myself at least once
when writing code.

quoted text
> The Repository.head property is not cached and it is probably OK to
> avoid some problems I had. We could run profiling afterwards to see
> how often it is called.

Yes. This applies to a few other things as well -- I try to be careful
to write things so that they could be cached transparently if it turns
out we need it. But I also try to not optimize prematurely.

In some cases the caching is more than just a speed optimization.
Commits and trees can be compared with regular Python object identity,
for example, because I never create duplicates.

quoted text
> > I started to implement a few StGit commands on top of gitlib.py
> > and stacklib.py, and then realized something very appealing:
> >
> >   Just about every StGit command can be accomplished by first
> >   creating a bunch of new commit objects with gitlib.py, then
> >   trying to check out the new HEAD, and then rewriting refs with
> >   stacklib.py. Only the first and second steps can possibly fail,
> >   and if they do, they do so without leaving any user-visible junk
> >   behind. This can be used to make all commands either succeed
> >   completely, or do nothing at all.
>
> As you said, that's a bit difficult for 'push' as it is made of a
> series of separate individual patch pushes. Can we not use a
> temporary index by setting GIT_INDEX_FILE during the whole
> transaction and freely update the working copy. Only if the
> transaction fails, we clean the working copy and check out the
> default index? This might slow down some operations though.

The sample code I posted does exactly this index trick. So yes, I
think it's a splendid idea. :-)

Basically, what it does (or eventually should do) is repeatedly use
the temp index to create the new commits without ever touching the
default index or the worktree. Only if a merge conflicts would we
check out one of the parents and spill the conflicts to the worktree.
(And once we're finished pushing we'd want to check out the new stack
top, obviously.)

The only problem is that while git-read-tree can do a three-way merge
in a temp index without touching the worktree, git-merge-recursive
assumes that the worktree reflects one of the trees to be merged and
can be written to. Which is _totally OK_ if there really is going to
be a conflict. But merge-recursive does fancier conflict resolution
than read-tree (I think -- I still have to look up exactly what, but I
don't think read-tree does rename detection for example), so it might
succeed even if read-tree failed.

quoted text
> In the future, it would be nice to record the stack state before
> transactions in a git object (via pickle) together with HEAD id and
> provide unlimited undo.

Yes, this was exactly what I was refering to when I talked about
transaction logging. The only difference is that I'd prefer a simple
text format instead of a pickle, but we can have a fight about that
detail later. :-)

The complete state that would have to be saved would be

  * (patchname, commit sha1) pairs for all patches

  * (branchname, branch head sha1) pair

  * the ordered lists of applied, unapplied, and eventually also
    hidden patches

This is somewhat redundant (applied/unapplied could be inferred from
the commit DAG, etc.) but it would allow us to represent relevant
illegal states as well, which is needed in order to be able to undo
repairs of inconsistent stacks.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html