Re: diff machinery cleanup

Jeff King [off-list ref] writes:

It seems clear that there's some global magic touched by the first diff
that impacts the second. I have to give up on finding it for tonight,
but I'm hoping somebody who knows more about the code will find it
obvious (or can tell me that I'm doing something else horribly wrong in
the above, or that these functions were never intended to be called
within the same program).

In general, run_diff_X are _not_ designed to run twice.

The run_diff_index() function munges the index while doing its
work (e.g. mark_merge_entries() hoists unmerged entries to stage
3 -- and worse yet creating duplicate entries for the same path
at stage 3; read_tree() reads the entries into stage 1 so that
it can be compared in-index with stage 0 entries).  The other
function, run_diff_files() have the same assumption but does not
touch index if I recall correctly.

If you are working in "next" branch where Johannes's merge-recur
work introduced discard_cache(), you could fake this somehow
stashing away a copy of the original index, and once you are
done with run_diff_index(), clean the slate by calling
discard_cache() once you are done, and swap the original index
in before running run_diff_files().

To solve this cleanly without doing the index munging hack, you
would (actually, I would) need to have a new path walker that
walks index, tree and working tree in parallel, which I was
working on in the git-status/git-commit rewrite I started and
discarded a few days ago.

On Thu, Aug 10, 2006 at 02:36:49AM -0700, Junio C Hamano wrote:

In general, run_diff_X are _not_ designed to run twice.

OK, makes sense. As you probably guessed, the reason is for the
run-status in C.

If you are working in "next" branch where Johannes's merge-recur
work introduced discard_cache(), you could fake this somehow
stashing away a copy of the original index, and once you are
done with run_diff_index(), clean the slate by calling
discard_cache() once you are done, and swap the original index
in before running run_diff_files().

OK, doing a discard_cache() between the call to run_diff_index and
run_diff_files seems to clear up the problem. But if I understand
correctly, are you saying that run_diff_index has munged the index on
disk, and I really need to be poking at a temporary copy? If so, why
isn't that a problem when running (e.g.) "git-diff-index; git-ls-files"?

To solve this cleanly without doing the index munging hack, you
would (actually, I would) need to have a new path walker that
walks index, tree and working tree in parallel, which I was
working on in the git-status/git-commit rewrite I started and
discarded a few days ago.

That does sound the cleanest, and it would enable a more useful status
message, as you mentioned before. What caused you to stop working on it?
Infeasible, or simply more infeasible than you would like right now?

-Peff