Re: [BUG] git-am silently applying patches incorrectly

Colin Guthrie [off-list ref] writes:

'Twas brillig, and Junio C Hamano at 04/03/11 21:33 did gyre and gimble:

quoted

In short, Linus and I both know what you are talking about, and we may
revisit that issue later, but the thing is that it would not be very
pleasant, and not something that can be done in one sitting during a
single discussion thread on the list.

As a simple option to avoid that, how about just printing out (by
default) the line offsets if hunks don't apply 100% cleanly? This would
at least alert you to the fact that some fixups were needed.

Yeah, that is what GNU patch does, and would be a small improvement that
might be in a right direction.

While we are at it, here is an alternate patch that does not lose the
ability to cope with the case where the target version has functions moved
around without introducing new ambiguous patch application sites.  Instead
of keeping the "last position was here -- we won't look beyond that", we
mark the lines that were brought into the target by the patch application
so far, and reject preimage matches against these lines.

A full solution for detecting a potential ambiguity and warning it would
be based on this version instead.  It would involve letting find_pos() not
stop at the first hit near the intended target, and marking the hunk that
can apply at more than one location.

A yet even more reliable alternative solution _might_ be to first scan the
original without applying any hunks just to find the possible sites to be
patched, warn ambiguities and decide & commit to these patch application
sites, and then apply the hunks.  If we did so, we wouldn't need this
patch nor the previous one.

But that would be a larger change, and would require a good test vector,
perhaps a large quilt series, to make sure it does not introduce
regression.


 builtin/apply.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 14951da..04f56f8 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c

@@ -204,6 +204,7 @@ struct line {
 	unsigned hash : 24;
 	unsigned flag : 8;
 #define LINE_COMMON     1
+#define LINE_PATCHED	2
 };
 
 /*

@@ -2085,7 +2086,8 @@ static int match_fragment(struct image *img,
 
 	/* Quick hash check */
 	for (i = 0; i < preimage_limit; i++)
-		if (preimage->line[i].hash != img->line[try_lno + i].hash)
+		if ((img->line[try_lno + i].flag & LINE_PATCHED) ||
+		    (preimage->line[i].hash != img->line[try_lno + i].hash))
 			return 0;
 
 	if (preimage_limit == preimage->nr) {

@@ -2428,6 +2430,9 @@ static void update_image(struct image *img,
 	memcpy(img->line + applied_pos,
 	       postimage->line,
 	       postimage->nr * sizeof(*img->line));
+	for (i = 0; i < postimage->nr; i++)
+		img->line[applied_pos + i].flag |= LINE_PATCHED;
+
 	img->nr = nr;
 }