1. Stefan Saasen
  2. git

Commits

Junio C Hamano  committed 7b676b1 Merge

Merge branch 'bg/apply-fix-blank-at-eof' into maint

* bg/apply-fix-blank-at-eof:
t3417: Add test cases for "rebase --whitespace=fix"
t4124: Add additional tests of --whitespace=fix
apply: Allow blank context lines to match beyond EOF
apply: Remove the quick rejection test
apply: Don't unnecessarily update line lengths in the preimage

  • Participants
  • Parent commits 846b8f6, 59f5ced
  • Branches master

Comments (0)

Files changed (4)

File builtin-apply.c

View file
 {
 	int i;
 	char *fixed_buf, *buf, *orig, *target;
+	int preimage_limit;
 
-	if (preimage->nr + try_lno > img->nr)
+	if (preimage->nr + try_lno <= img->nr) {
+		/*
+		 * The hunk falls within the boundaries of img.
+		 */
+		preimage_limit = preimage->nr;
+		if (match_end && (preimage->nr + try_lno != img->nr))
+			return 0;
+	} else if (ws_error_action == correct_ws_error &&
+		   (ws_rule & WS_BLANK_AT_EOF) && match_end) {
+		/*
+		 * This hunk that matches at the end extends beyond
+		 * the end of img, and we are removing blank lines
+		 * at the end of the file.  This many lines from the
+		 * beginning of the preimage must match with img, and
+		 * the remainder of the preimage must be blank.
+		 */
+		preimage_limit = img->nr - try_lno;
+	} else {
+		/*
+		 * The hunk extends beyond the end of the img and
+		 * we are not removing blanks at the end, so we
+		 * should reject the hunk at this position.
+		 */
 		return 0;
+	}
 
 	if (match_beginning && try_lno)
 		return 0;
 
-	if (match_end && preimage->nr + try_lno != img->nr)
-		return 0;
-
 	/* Quick hash check */
-	for (i = 0; i < preimage->nr; i++)
+	for (i = 0; i < preimage_limit; i++)
 		if (preimage->line[i].hash != img->line[try_lno + i].hash)
 			return 0;
 
-	/*
-	 * Do we have an exact match?  If we were told to match
-	 * at the end, size must be exactly at try+fragsize,
-	 * otherwise try+fragsize must be still within the preimage,
-	 * and either case, the old piece should match the preimage
-	 * exactly.
-	 */
-	if ((match_end
-	     ? (try + preimage->len == img->len)
-	     : (try + preimage->len <= img->len)) &&
-	    !memcmp(img->buf + try, preimage->buf, preimage->len))
-		return 1;
+	if (preimage_limit == preimage->nr) {
+		/*
+		 * Do we have an exact match?  If we were told to match
+		 * at the end, size must be exactly at try+fragsize,
+		 * otherwise try+fragsize must be still within the preimage,
+		 * and either case, the old piece should match the preimage
+		 * exactly.
+		 */
+		if ((match_end
+		     ? (try + preimage->len == img->len)
+		     : (try + preimage->len <= img->len)) &&
+		    !memcmp(img->buf + try, preimage->buf, preimage->len))
+			return 1;
+	} else {
+		/*
+		 * The preimage extends beyond the end of img, so
+		 * there cannot be an exact match.
+		 *
+		 * There must be one non-blank context line that match
+		 * a line before the end of img.
+		 */
+		char *buf_end;
+
+		buf = preimage->buf;
+		buf_end = buf;
+		for (i = 0; i < preimage_limit; i++)
+			buf_end += preimage->line[i].len;
+
+		for ( ; buf < buf_end; buf++)
+			if (!isspace(*buf))
+				break;
+		if (buf == buf_end)
+			return 0;
+	}
 
 	/*
 	 * No exact match. If we are ignoring whitespace, run a line-by-line
 		size_t imgoff = 0;
 		size_t preoff = 0;
 		size_t postlen = postimage->len;
-		for (i = 0; i < preimage->nr; i++) {
+		size_t extra_chars;
+		char *preimage_eof;
+		char *preimage_end;
+		for (i = 0; i < preimage_limit; i++) {
 			size_t prelen = preimage->line[i].len;
 			size_t imglen = img->line[try_lno+i].len;
 
 		}
 
 		/*
-		 * Ok, the preimage matches with whitespace fuzz. Update it and
-		 * the common postimage lines to use the same whitespace as the
-		 * target. imgoff now holds the true length of the target that
-		 * matches the preimage, and we need to update the line lengths
-		 * of the preimage to match the target ones.
+		 * Ok, the preimage matches with whitespace fuzz.
+		 *
+		 * imgoff now holds the true length of the target that
+		 * matches the preimage before the end of the file.
+		 *
+		 * Count the number of characters in the preimage that fall
+		 * beyond the end of the file and make sure that all of them
+		 * are whitespace characters. (This can only happen if
+		 * we are removing blank lines at the end of the file.)
 		 */
-		fixed_buf = xmalloc(imgoff);
-		memcpy(fixed_buf, img->buf + try, imgoff);
-		for (i = 0; i < preimage->nr; i++)
-			preimage->line[i].len = img->line[try_lno+i].len;
+		buf = preimage_eof = preimage->buf + preoff;
+		for ( ; i < preimage->nr; i++)
+			preoff += preimage->line[i].len;
+		preimage_end = preimage->buf + preoff;
+		for ( ; buf < preimage_end; buf++)
+			if (!isspace(*buf))
+				return 0;
 
 		/*
-		 * Update the preimage buffer and the postimage context lines.
+		 * Update the preimage and the common postimage context
+		 * lines to use the same whitespace as the target.
+		 * If whitespace is missing in the target (i.e.
+		 * if the preimage extends beyond the end of the file),
+		 * use the whitespace from the preimage.
 		 */
+		extra_chars = preimage_end - preimage_eof;
+		fixed_buf = xmalloc(imgoff + extra_chars);
+		memcpy(fixed_buf, img->buf + try, imgoff);
+		memcpy(fixed_buf + imgoff, preimage_eof, extra_chars);
+		imgoff += extra_chars;
 		update_pre_post_images(preimage, postimage,
 				fixed_buf, imgoff, postlen);
 		return 1;
 	 * it might with whitespace fuzz. We haven't been asked to
 	 * ignore whitespace, we were asked to correct whitespace
 	 * errors, so let's try matching after whitespace correction.
+	 *
+	 * The preimage may extend beyond the end of the file,
+	 * but in this loop we will only handle the part of the
+	 * preimage that falls within the file.
 	 */
 	fixed_buf = xmalloc(preimage->len + 1);
 	buf = fixed_buf;
 	orig = preimage->buf;
 	target = img->buf + try;
-	for (i = 0; i < preimage->nr; i++) {
+	for (i = 0; i < preimage_limit; 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;
 		target += tgtlen;
 	}
 
+
+	/*
+	 * Now handle the lines in the preimage that falls beyond the
+	 * end of the file (if any). They will only match if they are
+	 * empty or only contain whitespace (if WS_BLANK_AT_EOL is
+	 * false).
+	 */
+	for ( ; i < preimage->nr; i++) {
+		size_t fixlen; /* length after fixing the preimage */
+		size_t oldlen = preimage->line[i].len;
+		int j;
+
+		/* Try fixing the line in the preimage */
+		fixlen = ws_fix_copy(buf, orig, oldlen, ws_rule, NULL);
+
+		for (j = 0; j < fixlen; j++)
+			if (!isspace(buf[j]))
+				goto unmatch_exit;
+
+		orig += oldlen;
+		buf += fixlen;
+	}
+
 	/*
 	 * Yes, the preimage is based on an older version that still
 	 * has whitespace breakages unfixed, and fixing them makes the
 	unsigned long backwards, forwards, try;
 	int backwards_lno, forwards_lno, try_lno;
 
-	if (preimage->nr > img->nr)
-		return -1;
-
 	/*
 	 * If match_beginning or match_end is specified, there is no
 	 * point starting from a wrong line that will never match and
 	else if (match_end)
 		line = img->nr - preimage->nr;
 
-	if (line > img->nr)
+	/*
+	 * Because the comparison is unsigned, the following test
+	 * will also take care of a negative line number that can
+	 * result when match_end and preimage is larger than the target.
+	 */
+	if ((size_t) line > img->nr)
 		line = img->nr;
 
 	try = 0;
 	int i, nr;
 	size_t remove_count, insert_count, applied_at = 0;
 	char *result;
+	int preimage_limit;
+
+	/*
+	 * If we are removing blank lines at the end of img,
+	 * the preimage may extend beyond the end.
+	 * If that is the case, we must be careful only to
+	 * remove the part of the preimage that falls within
+	 * the boundaries of img. Initialize preimage_limit
+	 * to the number of lines in the preimage that falls
+	 * within the boundaries.
+	 */
+	preimage_limit = preimage->nr;
+	if (preimage_limit > img->nr - applied_pos)
+		preimage_limit = img->nr - applied_pos;
 
 	for (i = 0; i < applied_pos; i++)
 		applied_at += img->line[i].len;
 
 	remove_count = 0;
-	for (i = 0; i < preimage->nr; i++)
+	for (i = 0; i < preimage_limit; i++)
 		remove_count += img->line[applied_pos + i].len;
 	insert_count = postimage->len;
 
 	result[img->len] = '\0';
 
 	/* Adjust the line table */
-	nr = img->nr + postimage->nr - preimage->nr;
-	if (preimage->nr < postimage->nr) {
+	nr = img->nr + postimage->nr - preimage_limit;
+	if (preimage_limit < postimage->nr) {
 		/*
 		 * NOTE: this knows that we never call remove_first_line()
 		 * on anything other than pre/post image.
 		img->line = xrealloc(img->line, nr * sizeof(*img->line));
 		img->line_allocated = img->line;
 	}
-	if (preimage->nr != postimage->nr)
+	if (preimage_limit != postimage->nr)
 		memmove(img->line + applied_pos + postimage->nr,
-			img->line + applied_pos + preimage->nr,
-			(img->nr - (applied_pos + preimage->nr)) *
+			img->line + applied_pos + preimage_limit,
+			(img->nr - (applied_pos + preimage_limit)) *
 			sizeof(*img->line));
 	memcpy(img->line + applied_pos,
 	       postimage->line,
 
 	if (applied_pos >= 0) {
 		if (new_blank_lines_at_end &&
-		    preimage.nr + applied_pos == img->nr &&
+		    preimage.nr + applied_pos >= img->nr &&
 		    (ws_rule & WS_BLANK_AT_EOF) &&
 		    ws_error_action != nowarn_ws_error) {
 			record_ws_error(WS_BLANK_AT_EOF, "+", 1, frag->linenr);

File t/t3417-rebase-whitespace-fix.sh

View file
+#!/bin/sh
+
+test_description='git rebase --whitespace=fix
+
+This test runs git rebase --whitespace=fix and make sure that it works.
+'
+
+. ./test-lib.sh
+
+# prepare initial revision of "file" with a blank line at the end
+cat >file <<EOF
+a
+b
+c
+
+EOF
+
+# expected contents in "file" after rebase
+cat >expect-first <<EOF
+a
+b
+c
+EOF
+
+# prepare second revision of "file"
+cat >second <<EOF
+a
+b
+c
+
+d
+e
+f
+
+
+
+
+EOF
+
+# expected contents in second revision after rebase
+cat >expect-second <<EOF
+a
+b
+c
+
+d
+e
+f
+EOF
+
+test_expect_success 'blank line at end of file; extend at end of file' '
+	git commit --allow-empty -m "Initial empty commit" &&
+	git add file && git commit -m first &&
+	mv second file &&
+	git add file &&	git commit -m second &&
+	git rebase --whitespace=fix HEAD^^ &&
+	git diff --exit-code HEAD^:file expect-first &&
+	test_cmp file expect-second
+'
+
+# prepare third revision of "file"
+sed -e's/Z//' >third <<EOF
+a
+b
+c
+
+d
+e
+f
+    Z
+ Z
+h
+i
+j
+k
+l
+EOF
+
+sed -e's/ //g' <third >expect-third
+
+test_expect_success 'two blanks line at end of file; extend at end of file' '
+	cp third file && git add file && git commit -m third &&
+	git rebase --whitespace=fix HEAD^^ &&
+	git diff --exit-code HEAD^:file expect-second &&
+	test_cmp file expect-third
+'
+
+test_expect_success 'same, but do not remove trailing spaces' '
+	git config core.whitespace "-blank-at-eol" &&
+	git reset --hard HEAD^ &&
+	cp third file && git add file && git commit -m third &&
+	git rebase --whitespace=fix HEAD^^
+	git diff --exit-code HEAD^:file expect-second &&
+	test_cmp file third
+'
+
+sed -e's/Z//' >beginning <<EOF
+a
+		    Z
+       Z
+EOF
+
+cat >expect-beginning <<EOF
+a
+
+
+1
+2
+3
+4
+5
+EOF
+
+test_expect_success 'at beginning of file' '
+	git config core.whitespace "blank-at-eol" &&
+	cp beginning file &&
+	git commit -m beginning file &&
+	for i in 1 2 3 4 5; do
+		echo $i
+	done >> file &&
+	git commit -m more file	&&
+	git rebase --whitespace=fix HEAD^^ &&
+	test_cmp file expect-beginning
+'
+
+test_done

File t/t4104-apply-boundary.sh

View file
 
 '
 
+test_expect_success 'apply patch with 3 context lines matching at end' '
+	{ echo a; echo b; echo c; echo d; } >file &&
+	git add file &&
+	echo e >>file &&
+	git diff >patch &&
+	>file &&
+	test_must_fail git apply patch
+'
+
 test_done

File t/t4124-apply-ws-rule.sh

View file
 	grep "new blank line at EOF" error
 '
 
+test_expect_success 'applying beyond EOF requires one non-blank context line' '
+	{ echo; echo; echo; echo; } >one &&
+	git add one &&
+	{ echo b; } >>one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	{ echo a; echo; } >one &&
+	cp one expect &&
+	test_must_fail git apply --whitespace=fix patch &&
+	test_cmp one expect &&
+	test_must_fail git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'tons of blanks at EOF should not apply' '
+	for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16; do
+		echo; echo; echo; echo;
+	done >one &&
+	git add one &&
+	echo a >>one &&
+	git diff -- one >patch &&
+
+	>one &&
+	test_must_fail git apply --whitespace=fix patch &&
+	test_must_fail git apply --ignore-space-change --whitespace=fix patch
+'
+
+test_expect_success 'missing blank line at end with --whitespace=fix' '
+	echo a >one &&
+	echo >>one &&
+	git add one &&
+	echo b >>one &&
+	cp one expect &&
+	git diff -- one >patch &&
+	echo a >one &&
+	cp one saved-one &&
+	test_must_fail git apply patch &&
+	git apply --whitespace=fix patch &&
+	test_cmp one expect &&
+	mv saved-one one &&
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'two missing blank lines at end with --whitespace=fix' '
+	{ echo a; echo; echo b; echo c; } >one &&
+	cp one no-blank-lines &&
+	{ echo; echo; } >>one &&
+	git add one &&
+	echo d >>one &&
+	cp one expect &&
+	echo >>one &&
+	git diff -- one >patch &&
+	cp no-blank-lines one &&
+	test_must_fail git apply patch &&
+	git apply --whitespace=fix patch &&
+	test_cmp one expect &&
+	mv no-blank-lines one &&
+	test_must_fail git apply patch &&
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'shrink file with tons of missing blanks at end of file' '
+	{ echo a; echo b; echo c; } >one &&
+	cp one no-blank-lines &&
+	for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16; do
+		echo; echo; echo; echo;
+	done >>one &&
+	git add one &&
+	echo a >one &&
+	cp one expect &&
+	git diff -- one >patch &&
+	cp no-blank-lines one &&
+	test_must_fail git apply patch &&
+	git apply --whitespace=fix patch &&
+	test_cmp one expect &&
+	mv no-blank-lines one &&
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'missing blanks at EOF must only match blank lines' '
+	{ echo a; echo b; } >one &&
+	git add one &&
+	{ echo c; echo d; } >>one &&
+	git diff -- one >patch &&
+
+	echo a >one &&
+	test_must_fail git apply patch
+	test_must_fail git apply --whitespace=fix patch &&
+	test_must_fail git apply --ignore-space-change --whitespace=fix patch
+'
+
+sed -e's/Z//' >one <<EOF
+a
+b
+c
+		      Z
+EOF
+
+test_expect_success 'missing blank line should match context line with spaces' '
+	git add one &&
+	echo d >>one &&
+	git diff -- one >patch &&
+	{ echo a; echo b; echo c; } >one &&
+	cp one expect &&
+	{ echo; echo d; } >>expect &&
+	git add one &&
+
+	git apply --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+sed -e's/Z//' >one <<EOF
+a
+b
+c
+		      Z
+EOF
+
+test_expect_success 'same, but with the --ignore-space-option' '
+	git add one &&
+	echo d >>one &&
+	cp one expect &&
+	git diff -- one >patch &&
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+
+	git checkout-index -f one &&
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'same, but with CR-LF line endings && cr-at-eol set' '
+	git config core.whitespace cr-at-eol &&
+	printf "a\r\n" >one &&
+	printf "b\r\n" >>one &&
+	printf "c\r\n" >>one &&
+	cp one save-one &&
+	printf "                 \r\n" >>one
+	git add one &&
+	printf "d\r\n" >>one &&
+	cp one expect &&
+	git diff -- one >patch &&
+	mv save-one one &&
+
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
+test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '
+	git config --unset core.whitespace &&
+	printf "a\r\n" >one &&
+	printf "b\r\n" >>one &&
+	printf "c\r\n" >>one &&
+	cp one save-one &&
+	printf "                 \r\n" >>one
+	git add one &&
+	cp one expect &&
+	printf "d\r\n" >>one &&
+	git diff -- one >patch &&
+	mv save-one one &&
+	echo d >>expect &&
+
+	git apply --ignore-space-change --whitespace=fix patch &&
+	test_cmp one expect
+'
+
 test_done