"David D. Kilzer" <ddkilzer@kilzer.net> writes:

quoted text
> With diff.renames = copies, a 3-way merge (e.g. "git rebase") would
> fail with the following error:
>
>     fatal: mode change for <file>, which is not in current HEAD
>     Repository lacks necessary blobs to fall back on 3-way merge.
>     Cannot fall back to three-way merge.
>     Patch failed at 0001.
>
> The bug is a logic error added in ece7b749, which attempts to find
> an sha1 for a patch with no index line in build_fake_ancestor().
> Instead of failing unless an sha1 is found for both the old file and
> the new file, a failure should only be reported if neither the old
> file nor the new file is found.
>
> Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
> ---
>  builtin-apply.c   |    2 +-
>  t/t3400-rebase.sh |   17 +++++++++++++++++
>  2 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 4c4d1e1..cfeb6cc 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -2573,7 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
>  		else if (get_sha1(patch->old_sha1_prefix, sha1))
>  			/* git diff has no index line for mode/type changes */
>  			if (!patch->lines_added && !patch->lines_deleted) {
> -				if (get_current_sha1(patch->new_name, sha1) ||
> +				if (get_current_sha1(patch->new_name, sha1) &&
>  				    get_current_sha1(patch->old_name, sha1))
>  					die("mode change for %s, which is not "
>  						"in current HEAD", name);

Hmm.

The logic introduced by the blamed commit makes the --index-info
unreliable (I'd rather see it fail reliably if it does not have enough
information rather than pretending everything is Ok), and I think the
patch makes it slightly more so.

If new_name that is not related at all to old_name happens to exist in the
current tree you are applying the patch to, you can grab the contents of
the unrelated file as the preimage and try to merge the changes in.

When running --index-info for the purpose of "am -3" (hence rebase), the
expectation is that the tree you are applying the changes to is _similar_
to the preimage of the change, i.e. old_name.  Shouldn't missing old_name
be treated as a fatal condition?  new_name does not have to even exist
because otherwise you cannot accept a patch that creates the path.

Wouldn't this be a better patch, I wonder...

 builtin-apply.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git i/builtin-apply.c w/builtin-apply.c
index 4c4d1e1..7de70e9 100644
--- i/builtin-apply.c
+++ w/builtin-apply.c
@@ -2573,8 +2573,7 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
 		else if (get_sha1(patch->old_sha1_prefix, sha1))
 			/* git diff has no index line for mode/type changes */
 			if (!patch->lines_added && !patch->lines_deleted) {
-				if (get_current_sha1(patch->new_name, sha1) ||
-				    get_current_sha1(patch->old_name, sha1))
+				if (get_current_sha1(patch->old_name, sha1))
 					die("mode change for %s, which is not "
 						"in current HEAD", name);
 				sha1_ptr = sha1;

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html