aboutsummaryrefslogtreecommitdiff
path: root/builtin-apply.c
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2008-01-30 15:24:34 -0800
committerJunio C Hamano <gitster@pobox.com>2008-02-05 00:38:41 -0800
commitc1beba5b479a39143ebef96ba10103bbd9a70089 (patch)
treec43e689c6f26dabe5fd929c40734686b8fbd808f /builtin-apply.c
parentc607aaa2f05b4dc38a6573f44b6f71db05dd8b39 (diff)
downloadgit-c1beba5b479a39143ebef96ba10103bbd9a70089.tar.gz
git-c1beba5b479a39143ebef96ba10103bbd9a70089.tar.xz
git-apply --whitespace=fix: fix whitespace fuzz introduced by previous run
When you have more than one patch series, an earlier one of which tries to introduce whitespace breakages and a later one of which has such a new line in its context, "git-apply --whitespace=fix" will apply and fix the whitespace breakages in the earlier one, making the resulting file not to match the context of the later patch. A short demonstration is in the new test, t4125. For example, suppose the first patch is: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -20,3 +20,3 @@ Hello world.$ -How Are you$ -Today?$ +How are you $ +today? $ to fix broken case in the string, but it introduces unwanted trailing whitespaces to the result (pretend you are looking at "cat -e" output of the patch --- '$' signs are not in the patch but are shown to make the EOL stand out). And the second patch is to change the wording of the greeting further: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -18,5 +18,5 @@ Greetings $ -Hello world.$ +Hello, everybody. $ How are you $ -today? $ +these days? $ If you apply the first one with --whitespace=fix, you will get this as the result: Hello world.$ How are you$ today?$ and this does not match the preimage of the second patch, which demands extra whitespace after "How are you" and "today?". This series is about teaching "git apply --whitespace=fix" to cope with this situation better. If the patch does not apply, it rewrites the second patch like this and retries: diff a/hello.txt b/hello.txt --- a/hello.txt +++ b/hello.txt @@ -18,5 +18,5 @@ Greetings$ -Hello world.$ +Hello, everybody.$ How are you$ -today?$ +these days?$ This is done by rewriting the preimage lines in the hunk (i.e. the lines that begin with ' ' or '-'), using the same whitespace fixing rules as it is using to apply the patches, so that it can notice what it did to the previous ones in the series. A careful reader may notice that the first patch in the example did not touch the "Greetings" line, so the trailing whitespace that is in the original preimage of the second patch is not from the series. Is rewriting this context line a problem? If you think about it, you will realize that the reason for the difference is because the submitter's tree was based on an earlier version of the file that had whitespaces wrong on that "Greetings" line, and the change that introduced the "Greetings" line was added independently of this two-patch series to our tree already with an earlier "git apply --whitespace=fix". So it may appear this logic is rewriting too much, it is not so. It is just rewriting what we would have rewritten in the past. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin-apply.c')
-rw-r--r--builtin-apply.c133
1 files changed, 128 insertions, 5 deletions
diff --git a/builtin-apply.c b/builtin-apply.c
index 5f3c047f2..fccf4a40c 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1516,7 +1516,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
}
static int copy_wsfix(char *output, const char *patch, int plen,
- unsigned ws_rule)
+ unsigned ws_rule, int count_error)
{
/*
* plen is number of bytes to be copied from patch, starting
@@ -1604,11 +1604,74 @@ static int copy_wsfix(char *output, const char *patch, int plen,
memcpy(output, patch, plen);
if (add_nl_to_tail)
output[plen++] = '\n';
- if (fixed)
+ if (fixed && count_error)
applied_after_fixing_ws++;
return output + plen - buf;
}
+static void update_pre_post_images(struct image *preimage,
+ struct image *postimage,
+ char *buf,
+ size_t len)
+{
+ int i, ctx;
+ char *new, *old, *fixed;
+ struct image fixed_preimage;
+
+ /*
+ * Update the preimage with whitespace fixes. Note that we
+ * are not losing preimage->buf -- apply_one_fragment() will
+ * free "oldlines".
+ */
+ prepare_image(&fixed_preimage, buf, len, 1);
+ assert(fixed_preimage.nr == preimage->nr);
+ for (i = 0; i < preimage->nr; i++)
+ fixed_preimage.line[i].flag = preimage->line[i].flag;
+ free(preimage->line_allocated);
+ *preimage = fixed_preimage;
+
+ /*
+ * Adjust the common context lines in postimage, in place.
+ * This is possible because whitespace fixing does not make
+ * the string grow.
+ */
+ new = old = postimage->buf;
+ fixed = preimage->buf;
+ for (i = ctx = 0; i < postimage->nr; i++) {
+ size_t len = postimage->line[i].len;
+ if (!(postimage->line[i].flag & LINE_COMMON)) {
+ /* an added line -- no counterparts in preimage */
+ memmove(new, old, len);
+ old += len;
+ new += len;
+ continue;
+ }
+
+ /* a common context -- skip it in the original postimage */
+ old += len;
+
+ /* and find the corresponding one in the fixed preimage */
+ while (ctx < preimage->nr &&
+ !(preimage->line[ctx].flag & LINE_COMMON)) {
+ fixed += preimage->line[ctx].len;
+ ctx++;
+ }
+ if (preimage->nr <= ctx)
+ die("oops");
+
+ /* and copy it in, while fixing the line length */
+ len = preimage->line[ctx].len;
+ memcpy(new, fixed, len);
+ new += len;
+ fixed += len;
+ postimage->line[i].len = len;
+ ctx++;
+ }
+
+ /* Fix the length of the whole thing */
+ postimage->len = new - postimage->buf;
+}
+
static int match_fragment(struct image *img,
struct image *preimage,
struct image *postimage,
@@ -1618,6 +1681,7 @@ static int match_fragment(struct image *img,
int match_beginning, int match_end)
{
int i;
+ char *fixed_buf, *buf, *orig, *target;
if (preimage->nr + try_lno > img->nr)
return 0;
@@ -1646,10 +1710,68 @@ static int match_fragment(struct image *img,
!memcmp(img->buf + try, preimage->buf, preimage->len))
return 1;
+ if (ws_error_action != correct_ws_error)
+ return 0;
+
+ /*
+ * The hunk does not apply byte-by-byte, but the hash says
+ * it might with whitespace fuzz.
+ */
+ fixed_buf = xmalloc(preimage->len + 1);
+ buf = fixed_buf;
+ orig = preimage->buf;
+ target = img->buf + try;
+ for (i = 0; i < preimage->nr; i++) {
+ size_t fixlen; /* length after fixing the preimage */
+ size_t oldlen = preimage->line[i].len;
+ size_t tgtlen = img->line[try_lno + i].len;
+ size_t tgtfixlen; /* length after fixing the target line */
+ char tgtfixbuf[1024], *tgtfix;
+ int match;
+
+ /* Try fixing the line in the preimage */
+ fixlen = copy_wsfix(buf, orig, oldlen, ws_rule, 0);
+
+ /* Try fixing the line in the target */
+ if (sizeof(tgtfixbuf) < tgtlen)
+ tgtfix = tgtfixbuf;
+ else
+ tgtfix = xmalloc(tgtlen);
+ tgtfixlen = copy_wsfix(tgtfix, target, tgtlen, ws_rule, 0);
+
+ /*
+ * If they match, either the preimage was based on
+ * a version before our tree fixed whitespace breakage,
+ * or we are lacking a whitespace-fix patch the tree
+ * the preimage was based on already had (i.e. target
+ * has whitespace breakage, the preimage doesn't).
+ * In either case, we are fixing the whitespace breakages
+ * so we might as well take the fix together with their
+ * real change.
+ */
+ match = (tgtfixlen == fixlen && !memcmp(tgtfix, buf, fixlen));
+
+ if (tgtfix != tgtfixbuf)
+ free(tgtfix);
+ if (!match)
+ goto unmatch_exit;
+
+ orig += oldlen;
+ buf += fixlen;
+ target += tgtlen;
+ }
+
/*
- * NEEDSWORK: We can optionally match fuzzily here, but
- * that is for a later round.
+ * Yes, the preimage is based on an older version that still
+ * has whitespace breakages unfixed, and fixing them makes the
+ * hunk match. Update the context lines in the postimage.
*/
+ update_pre_post_images(preimage, postimage,
+ fixed_buf, buf - fixed_buf);
+ return 1;
+
+ unmatch_exit:
+ free(fixed_buf);
return 0;
}
@@ -1871,7 +1993,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
added = plen;
}
else {
- added = copy_wsfix(new, patch + 1, plen, ws_rule);
+ added = copy_wsfix(new, patch + 1, plen,
+ ws_rule, 1);
}
add_line_info(&postimage, new, added,
(first == '+' ? 0 : LINE_COMMON));