Re: [PATCH 2/9] merge-recursive: clarify code in was_tracked()
From: Junio C Hamano <hidden>
Date: 2016-07-06 15:30:18
Johannes Schindelin [off-list ref] writes:
To understand why we're not done yet, the crucial point is *not* that the return value encodes the insert position. The crucial point is that despite asking for an index entry matching a specific name, we might not find one, *even if there is one*.
I've been wondering why you keep saying "even though we didn't ask,
we look for stage#0", and now I see why. The cache_pos() interface
*is* about finding the stage#0 entry for the given path.
When it finds none, it indicates where a stage#0 entry of that path
would be inserted, which by the sort-order would give us where
higher stage entries for the path would be found (if there is any).
There is no parameter for you to tell it to find stage#2, and "even
though we didn't ask" is showing (and being the source of) the
confusion.
And I did not want a misleading comment to spread the confusion;
that is why I was reacting strongly.
As you pointed out, we can return early without falling into the
generic "we are still looking at the same path" codepath when we
find thestage#0 entry, so I wouldn't mind doing something like the
following.
static int was_tracked(const char *path)
{
int pos = cache_name_pos(path, strlen(path));
if (0 <= pos)
/* we have been tracking this path */
return 1;
/*
* Look for an unmerged entry for the path,
* specifically stage #2, which would indicate
* that "our" side before the merge started
* had the path tracked (and resulted in a conflict).
*/
for (pos = -1 - pos;
pos < active_nr && !strcmp(path, active_cache[pos]->name);
pos++)
if (ce_stage(active_cache[pos]) == 2)
return 1;
return 0;
}