RFH: refactor read-tree

RFH: refactor read-tree

[Johannes Schindelin]

Hi,

the last thing to do with merge-recursive to speed it up, would be to 
avoid reading/writing the cache all the time.

Unfortunately, builtin-read-tree.c grew into a pretty big monster, with so 
many different options which completely change behaviour.

So, how should I go about it? Should I make a struct a la diff_options to 
hold the options to unpack_trees? Where should it go?

I also played a little with git-merge-tree, because it seems so much 
simpler and easier to refactor. But there is a problem: Either I call it 
the wrong way, or it does not yet work correctly: I tried

	git-merge-tree $(git-merge-base branch1 branch2) branch1 branch2

with what is in 'next'. But it only showed the _new_ files, not the 
modified ones.

Help, please?

Ciao,
Dscho

Re: RFH: refactor read-tree

[Linus Torvalds]

On Sun, 9 Jul 2006, Johannes Schindelin wrote:
> 
> I also played a little with git-merge-tree, because it seems so much 
> simpler and easier to refactor. But there is a problem: Either I call it 
> the wrong way, or it does not yet work correctly: I tried
> 
> 	git-merge-tree $(git-merge-base branch1 branch2) branch1 branch2
> 
> with what is in 'next'. But it only showed the _new_ files, not the 
> modified ones.

What git-merge-tree does is to show the _difference_ to "branch1".

So if the result of the merge would be totally identical to "branch1", 
then git-merge-tree should be totally silent.

The basic idea is that "branch1" should be your current branch, and it 
obviously is also expected to match (more or less) the current index. So 
you can do a merge by

 - reading in "branch1" into the index:

	GIT_INDEX_FILE=.git/tmp-index git-read-tree -m branch1

 - doing a "git-merge-tree $base $branch1 $branch2"

 - using the _result_ of "git-merge-tree" to modify the index you just 
   read in.

 - write out the end result as the result of the merge.

And yes, I agree 100% that "git-read-tree" has become an unholy mess. I 
looked at it, and I think it's unfixable. I considered re-writing it from 
scratch, at least for some specific cases, but I couldn't bring myself to 
do it.

			Linus

Re: RFH: refactor read-tree

[Alex Riesen]

Linus Torvalds, Sun, Jul 09, 2006 05:15:41 +0200:
> The basic idea is that "branch1" should be your current branch, and it 
> obviously is also expected to match (more or less) the current index. So 
> you can do a merge by
> 
>  - reading in "branch1" into the index:
> 
> 	GIT_INDEX_FILE=.git/tmp-index git-read-tree -m branch1

what is "-m" here for?

Re: RFH: refactor read-tree

[Johannes Schindelin]

Hi,

On Sun, 9 Jul 2006, Alex Riesen wrote:

> Linus Torvalds, Sun, Jul 09, 2006 05:15:41 +0200:
> > The basic idea is that "branch1" should be your current branch, and it 
> > obviously is also expected to match (more or less) the current index. So 
> > you can do a merge by
> > 
> >  - reading in "branch1" into the index:
> > 
> > 	GIT_INDEX_FILE=.git/tmp-index git-read-tree -m branch1
> 
> what is "-m" here for?

It means that git-read-tree tries to merge the current index with branch1.

Ciao,
Dscho

Re: RFH: refactor read-tree

[Linus Torvalds]

On Sun, 9 Jul 2006, Johannes Schindelin wrote:
> 
> On Sun, 9 Jul 2006, Alex Riesen wrote:
> 
> > Linus Torvalds, Sun, Jul 09, 2006 05:15:41 +0200:
> > > The basic idea is that "branch1" should be your current branch, and it 
> > > obviously is also expected to match (more or less) the current index. So 
> > > you can do a merge by
> > > 
> > >  - reading in "branch1" into the index:
> > > 
> > > 	GIT_INDEX_FILE=.git/tmp-index git-read-tree -m branch1
> > 
> > what is "-m" here for?
> 
> It means that git-read-tree tries to merge the current index with branch1.

Well, the current index always "merges" by just taking the timestamps from 
it. The actual _content_ doesn't matter for the single-tree case.

For the two- and three-tree case, "git-read-tree -m" will verify that the 
parts that got changed still _match_ in the index, but for a single-tree 
"git-read-tree", there's nothing to match against, just the target, so 
the only thing it does is that for matching index/target-tree entries it 
will re-use the index timestamps (and other stat info).

			Linus

Re: RFH: refactor read-tree

[Alex Riesen]

Linus Torvalds, Sun, Jul 09, 2006 17:30:26 +0200:
> > > > The basic idea is that "branch1" should be your current branch, and it 
> > > > obviously is also expected to match (more or less) the current index. So 
> > > > you can do a merge by
> > > > 
> > > >  - reading in "branch1" into the index:
> > > > 
> > > > 	GIT_INDEX_FILE=.git/tmp-index git-read-tree -m branch1
> > > 
> > > what is "-m" here for?
> > 
> > It means that git-read-tree tries to merge the current index with branch1.
> 
> Well, the current index always "merges" by just taking the timestamps from 
> it. The actual _content_ doesn't matter for the single-tree case.

But the name suggests it's a temporary index, which would not have
anything in it and even the .git/tmp-index is not supposed to exist.
So I'd actually understand this as creating an index from the tree-ish
branch1, without merging anything. And continue wondering what that -m
is for...

Re: RFH: refactor read-tree

[Johannes Schindelin]

Hi,

On Sat, 8 Jul 2006, Linus Torvalds wrote:

> On Sun, 9 Jul 2006, Johannes Schindelin wrote:
> > 
> > I also played a little with git-merge-tree, because it seems so much 
> > simpler and easier to refactor. But there is a problem: Either I call it 
> > the wrong way, or it does not yet work correctly: I tried
> > 
> > 	git-merge-tree $(git-merge-base branch1 branch2) branch1 branch2
> > 
> > with what is in 'next'. But it only showed the _new_ files, not the 
> > modified ones.
> 
> What git-merge-tree does is to show the _difference_ to "branch1".

I see my problem: branch1 is not the "upstream" branch, but my own. Tsk. 
Too easy.

Now, if only merge-tree knew about renames. *sigh*.

> And yes, I agree 100% that "git-read-tree" has become an unholy mess. I 
> looked at it, and I think it's unfixable. I considered re-writing it from 
> scratch, at least for some specific cases, but I couldn't bring myself to 
> do it.

Well, I think that at least the unpack_tree() thing should be relatively 
easy to extract. And the rest _should_ be relatively easy to clean up, 
provided we introduce a read_tree_options struct, which gets passed around 
a la "this" in Java/C++.

Ciao,
Dscho