Re: [PATCH 0/2] Reorganize read-tree
Daniel Barkalow [EMAIL PROTECTED] writes:
I got mostly done with this before Linus mentioned the possibility of
having multiple index entries in the same stage for a single path. I
finished it anyway, but I'm not sure that we won't want to know which of
the common ancestors contributed which, and, if some of them don't have a
path, we wouldn't be able to tell. The other advantages I see to this
approach are:
I've finished reading your patch, after beating it reasonably
heavily by feeding combinations of nonsense trees to make sure
it produces the same result as the original implementation. I
have not found any regression from the read-tree in master
branch, after you fixed the path ordering issues.
There are various potential refinements, plus removing a bunch of memory
leaks, still to do, but I think this is sufficiently close to review.
I am not so worried about the leaks right now; they are
something that could be fixed before it hits the master
branch.
I like your approach of reading the input trees, along with the
existing index contents, and re-populating the index one path at
a time. It probably is more readable.
I further think that you can get the best of both worlds, by
inventing a convention that mode=0 entry means 'this path does
not exist in this tree'. This would allow you to have multiple
entries at the same stage and still tell which one came from
which tree. Instead of calling fn in unpack_trees(), you could
make it only unpack the tree into the index, and then after
unpacking is done, call fn() repeatedly to resolve the resulting
index. Of course the semantics of merge_fn_t needs to change
but if you feed N trees, the caller of a merge_fn_t function
needs to pick up the first N entries (because you use mode=0
entry to mean 'missing from this tree', each path will always
have N entries) and feed them to the merge function in one call,
then pick up the next N entries and feed them in the next call,
and so on. I think that would simplify that part of the code
even further.
So if you are making an octopus of 4 trees (one being our
current branch) using 2 merge bases, your intermediate index
would look like:
mode SHA1 stage path
100644 X 0 foo from original index
00 0{40} 1 foo merge base #1 did not have foo
100644 Z 1 foo merge base #2 has it
100644 X 2 foo our current head
100644 Z 3 foo other head #1 being merged into us
100644 Y 3 foo other head #2 being merged into us
100644 Z 3 foo other head #3 being merged into us
We cannot write something like this out without breaking
backward compatibility, but I personally think this breakage is
OK, because what is being broken is the index file format, not
tree object format, and the index file is by definition local to
a repository. IOW, it is not too much to ask people not to use
old tools to read new index file they created using new tools.
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Reorganize read-tree
Daniel Barkalow [EMAIL PROTECTED] wrote: I got mostly done with this before Linus mentioned the possibility of having multiple index entries in the same stage for a single path. I finished it anyway, but I'm not sure that we won't want to know which of the common ancestors contributed which, and, if some of them don't have a path, we wouldn't be able to tell. I don't have time to look at the patch and I don't have a good knowledge of the GIT internals, so I will just ask. Does this patch changes the call convention for git-merge-one-file-script? I have my own script for StGIT and I would need to know whether it is affected or not. Thanks. -- Catalin - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Reorganize read-tree
On Tue, 30 Aug 2005, Junio C Hamano wrote: Dan, I really really *REALLY* wanted to try this out in pu branch and even was about to rig some torture chamber for testing before applying the patch, but you got the shiny blue bat X-. I'll send a replacement with the settings correct. A patch to SubmittingPatches, MUA specific help section for users of Pine 4.63 would be very much appreciated. Ah, it looks like a recent version changed the default behavior to do the right thing, and inverted the sense of the configuration option. (Either that or Gentoo did it.) So you need to set the no-strip-whitespace-before-send option, unless the option you have is strip-whitespace-before-send, in which case you should avoid checking it. I don't actually have things set up for preparing patches from work, although I can resend the patches I prepared earlier. -Daniel *This .sig left intentionally blank* - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Reorganize read-tree
On Wed, 31 Aug 2005, Catalin Marinas wrote: Daniel Barkalow [EMAIL PROTECTED] wrote: I got mostly done with this before Linus mentioned the possibility of having multiple index entries in the same stage for a single path. I finished it anyway, but I'm not sure that we won't want to know which of the common ancestors contributed which, and, if some of them don't have a path, we wouldn't be able to tell. I don't have time to look at the patch and I don't have a good knowledge of the GIT internals, so I will just ask. Does this patch changes the call convention for git-merge-one-file-script? I have my own script for StGIT and I would need to know whether it is affected or not. Nope, it only changes the trivial merge calling convention within read-tree.c; I think it's plausible that we might like to add information at some point, but the short-term goal is just to prevent a few bad cases in trivial merges. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Reorganize read-tree
I got mostly done with this before Linus mentioned the possibility of having multiple index entries in the same stage for a single path. I finished it anyway, but I'm not sure that we won't want to know which of the common ancestors contributed which, and, if some of them don't have a path, we wouldn't be able to tell. The other advantages I see to this approach are: - it uses the more common parser of tree objects, moving toward having only one (diff-cache still uses read_tree(), however). - it doesn't need to do very complicated things with the index; the original read-tree does a bunch of stuff with an index with a gap in the middle containing obsolete entries. - it uses a much simpler method of finding directory/file conflicts, which is possible because the struct trees represent directories as well as files. - it deals with each path completely before going on to the next one, instead of first dealing with each input tree and then dealing with each path. - it removes a lot of intimate knowledge of the index structure from the program. The general idea is that it figures out what trees you want, and then iterates through the entry lists together, recursing into directories, and calls the merge function with an array of the index entries (not yet added) for the path in each tree; the merge function adds the appropriate things to the index. Note that this set doesn't include calling merge functions with multiple ancestors or remotes; that can be done when we've decided on whether my version of read-tree is worth using. There are various potential refinements, plus removing a bunch of memory leaks, still to do, but I think this is sufficiently close to review. (Refinements: it ought to have two indices in memory, the old and the new, and never modify the old and only append to the new, to simplify things further; it ought to use a sentinal value for the index entry to indicate that there is something in the tree to conflict with there being a file at the given path; the --emu23 logic could be clearer) The first patch adds a few functions to the object library. The second patch changes read-tree around; It is essentially a rewrite, except for the merge functions and main(). -Daniel *This .sig left intentionally blank* - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
