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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Colin Guthrie <gmane@xxxxxxxxxxxxxx> writes:

> 'Twas brillig, and Junio C Hamano at 04/03/11 21:33 did gyre and gimble:
>> 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;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]