Re: [PATCH] log: speed up --follow following renames for non-linear history

[Junio C Hamano <gitster@pobox.com>]

Miklos Vajna <vmiklos@collabora.com> writes:

> +/* Compute a path to follow in each parent of a merge */
> +static void propagate_follow_pathspec_to_parents(struct rev_info *opt,
> +						 struct commit *commit,
> +						 struct commit_list *parents)
> +{
> +	const char *path = pathspec_single_path(&opt->diffopt.pathspec);
> +	struct commit_list *p;
> +	enum follow_path_change *changes;
> +	int nr = commit_list_count(parents), i;
> +	int has_same = 0;
> +
> +	if (!path)
> +		return;
> +
> +	ALLOC_ARRAY(changes, nr);
> +	for (p = parents, i = 0; p; p = p->next, i++) {
> +		changes[i] = get_follow_path_change(opt, commit, p->item);
> +		if (changes[i] == FOLLOW_PATH_SAME) {
> +			has_same = 1;
> +			break;
> +		}
> +	}
> +
> +	for (p = parents, i = 0; p; p = p->next, i++) {
> +		if (!has_same && changes[i] == FOLLOW_PATH_ADDED)
> +			/* Never the same, added here: expensive rename detection */
> +			propagate_follow_pathspec_to_parent(opt, commit, p->item);
> +		else
> +			remember_follow_pathspec(opt, p->item, path);
> +	}
> +	free(changes);
> +}

Hmph, so if you have two branches A and B that forked from a common
ancestor that had file F,

 - A updates F in place
 - B renames F to G in one commit
 - B then updates the renamed G on in another commit
 - A merges B but decides to keep the original name F and
   original content, deleting G.

then because has_same would be the same, you do not call
propagate_follow_pathspec_to_parent() to prepare the traversal into
branch B's history when you cross the merge, even when we are not
simplifying the history?

That somehow sounds broken to me.  Am I missing something?