[PATCHv2 0/2] git apply: cope with whitespace differences

Giuseppe Bilotta· Jul 4, 2009, 11:53 UTC · re: Giuseppe Bilotta · lore

[PATCHv2 1/2] git apply: option to ignore whitespace differences

A test is included, and 'git am' and 'git rebase' are made aware of this
option and pass it through to 'git apply', and so is the bash git
completion.
---
 builtin-apply.c                        |   55 ++++++++++++++++-
 contrib/completion/git-completion.bash |    2 +
 git-am.sh                              |    3 +-
 git-rebase.sh                          |    3 +
 t/t4107-apply-ignore-whitespace.sh     |  107 ++++++++++++++++++++++++++++++++
 5 files changed, 167 insertions(+), 3 deletions(-)
 create mode 100755 t/t4107-apply-ignore-whitespace.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index dc0ff5e..01230f1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -39,6 +39,7 @@ static int diffstat;
 static int numstat;
 static int summary;
 static int check;
+static int ignore_whitespace;
 static int apply = 1;
 static int apply_in_reverse;
 static int apply_with_reject;
@@ -214,6 +215,52 @@ static uint32_t hash_line(const char *cp, size_t len)
 	return h;
 }
 
+/*
+ * Compare s1 to s2 up to length n, ignoring whitespace differences.
+ * It is acceptable if s2 is a substring of s1.
+ */
+static int memcmp_ignore_whitespace(const char *s1, const char *s2, size_t n)
+{
+	const char *stop = s2 + n;
+	int result;
+
+	if (!n)
+		return 0;
+
+	/* skip leading whitespace */
+	while (isspace(*s1))
+		s1++;
+	while (isspace(*s2))
+		s2++;
+	while (!result && s2 < stop) {
+		result = *s1++ - *s2++;
+		/*
+		 * skip whitespace inside if we have whitespace
+		 * on both buffers
+		 */
+		if (isspace(*s1) && isspace(*s2)) {
+			while (isspace(*s1))
+				s1++;
+			while (isspace(*s2))
+				s2++;
+		}
+	}
+
+	return result;
+}
+
+/*
+ * Returns true if the lines in s2 match the buffer in s1 up to length n,
+ * according to the ignore_whitespace setting
+ */
+static int lines_match(const char *s1, const char *s2, size_t n)
+{
+	if (ignore_whitespace)
+		return !memcmp_ignore_whitespace(s1, s2, n);
+	else
+		return !memcmp(s1, s2, n);
+}
+
 static void add_line_info(struct image *img, const char *bol, size_t len, unsigned flag)
 {
 	ALLOC_GROW(img->line_allocated, img->nr + 1, img->alloc);
@@ -1690,7 +1737,7 @@ static int match_fragment(struct image *img,
 	if ((match_end
 	     ? (try + preimage->len == img->len)
 	     : (try + preimage->len <= img->len)) &&
-	    !memcmp(img->buf + try, preimage->buf, preimage->len))
+	    lines_match(img->buf + try, preimage->buf, preimage->len))
 		return 1;
 
 	if (ws_error_action != correct_ws_error)
@@ -1698,7 +1745,9 @@ static int match_fragment(struct image *img,
 
 	/*
 	 * The hunk does not apply byte-by-byte, but the hash says
-	 * it might with whitespace fuzz.
+	 * 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.
 	 */
 	fixed_buf = xmalloc(preimage->len + 1);
 	buf = fixed_buf;
@@ -3304,6 +3353,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		{ OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
 			"detect new or modified lines that have whitespace errors",
 			0, option_parse_whitespace },
+		OPT_BOOLEAN(0, "ignore-whitespace", &ignore_whitespace,
+			"ignore whitespace differences when finding context"),
 		OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
 			"apply the patch in reverse"),
 		OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ddb71e2..d3415b5 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -663,6 +663,7 @@ _git_am ()
 	--*)
 		__gitcomp "
 			--3way --committer-date-is-author-date --ignore-date
+			--ignore-whitespace
 			--interactive --keep --no-utf8 --signoff --utf8
 			--whitespace=
 			"
@@ -684,6 +685,7 @@ _git_apply ()
 			--stat --numstat --summary --check --index
 			--cached --index-info --reverse --reject --unidiff-zero
 			--apply --no-add --exclude=
+			--ignore-whitespace
 			--whitespace= --inaccurate-eof --verbose
 			"
 		return
diff --git a/git-am.sh b/git-am.sh
index d64d997..fe024b1 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -16,6 +16,7 @@ s,signoff       add a Signed-off-by line to the commit message
 u,utf8          recode into utf8 (default)
 k,keep          pass -k flag to git-mailinfo
 whitespace=     pass it through git-apply
+ignore-whitespace pass it through git-apply
 directory=      pass it through git-apply
 C=              pass it through git-apply
 p=              pass it through git-apply
@@ -303,7 +304,7 @@ do
 		git_apply_opt="$git_apply_opt $(sq "$1$2")"; shift ;;
 	--patch-format)
 		shift ; patch_format="$1" ;;
-	--reject)
+	--reject|--ignore-whitespace)
 		git_apply_opt="$git_apply_opt $1" ;;
 	--committer-date-is-author-date)
 		committer_date_is_author_date=t ;;
diff --git a/git-rebase.sh b/git-rebase.sh
index 18bc694..d741752 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -333,6 +333,9 @@ do
 			;;
 		esac
 		;;
+	--ignore-whitespace)
+		git_am_opt="$git_am_opt $1"
+		;;
 	--committer-date-is-author-date|--ignore-date)
 		git_am_opt="$git_am_opt $1"
 		force_rebase=t
diff --git a/t/t4107-apply-ignore-whitespace.sh b/t/t4107-apply-ignore-whitespace.sh
new file mode 100755
index 0000000..b9e6f14
--- /dev/null
+++ b/t/t4107-apply-ignore-whitespace.sh
@@ -0,0 +1,107 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+# Copyright (c) 2005 Robert Fitzsimons
+#
+
+test_description='git-apply --ignore-whitespace.
+
+'
+. ./test-lib.sh
+
+# setup
+
+cat > patch1.patch <<\EOF
+diff --git a/main.c b/main.c
+new file mode 100644
+--- /dev/null
++++ b/main.c
+@@ -0,0 +1,22 @@
++#include <stdio.h>
++
++void print_int(int num);
++int func(int num);
++
++int main() {
++       int i;
++
++       for (i = 0; i < 10; i++) {
++               print_int(func(i)); /* stuff */
++       }
++
++       return 0;
++}
++
++int func(int num) {
++       return num * num;
++}
++
++void print_int(int num) {
++       printf("%d", num);
++}
+EOF
+cat > patch2.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -10,6 +10,8 @@
+ 		print_int(func(i)); /* stuff */
+ 	}
+ 
++	printf("\n");
++
+ 	return 0;
+ }
+ 
+EOF
+
+cat > patch3.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -10,1 +10,1 @@
+ 		print_int(func(i)); 
+EOF
+
+cat > patch4.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -21,1 +21,1 @@
+	}; 
+\ No newline at end of file
+EOF
+
+cat > patch5.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -2,1 +2,1 @@
+ 	void print_int(int num);
+EOF
+
+test_expect_success "S = patch1" \
+    'git-apply patch1.patch'
+
+test_expect_failure "F = patch2" \
+    'git-apply patch2.patch'
+
+test_expect_success "S = patch2 (--ignore-whitespace)" \
+    'git-apply --ignore-whitespace patch2.patch'
+
+test_expect_failure "S = patch3 (missing string at EOL)" \
+    'git-apply --ignore-whitespace patch3.patch'
+
+test_expect_failure "S = patch4 (missing EOL at EOF)" \
+    'git-apply --ignore-whitespace patch4.patch'
+
+test_expect_success "S = patch5 (leading whitespace)" \
+    'git-apply --ignore-whitespace patch5.patch'
+
+rm -f main.c
+test_expect_success "S = patch1 (--ignore-whitespace)" \
+    'git-apply --ignore-whitespace patch1.patch'
+
+test_done
+
+

-- 
1.6.3.3.512.g4fff

Ramsay Jones· Jul 5, 2009, 23:21 UTC · re: Giuseppe Bilotta · lore

Re: [PATCHv2 1/2] git apply: option to ignore whitespace differences

> + * Compare s1 to s2 up to length n, ignoring whitespace differences.
> + * It is acceptable if s2 is a substring of s1.
> + */
> +static int memcmp_ignore_whitespace(const char *s1, const char *s2, size_t n)
> +{
> +	const char *stop = s2 + n;
> +	int result;

result is uninitialized here...

> +
> +	if (!n)
> +		return 0;
> +
> +	/* skip leading whitespace */
> +	while (isspace(*s1))
> +		s1++;
> +	while (isspace(*s2))
> +		s2++;
> +	while (!result && s2 < stop) {

> +		result = *s1++ - *s2++;

... so this may or may not be set...

> +		/*
> +		 * skip whitespace inside if we have whitespace
> +		 * on both buffers
> +		 */
> +		if (isspace(*s1) && isspace(*s2)) {
> +			while (isspace(*s1))
> +				s1++;
> +			while (isspace(*s2))
> +				s2++;
> +		}
> +	}
> +
> +	return result;

... so this may still be uninitialized.

> +}
> +

Giuseppe Bilotta· Jul 4, 2009, 11:53 UTC · re: Giuseppe Bilotta · lore

[PATCHv2 2/2] git apply: preserve original whitespace with --ignore-whitespace

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

diff --git a/builtin-apply.c b/builtin-apply.c
index 01230f1..6c6ccbd 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1639,10 +1639,17 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
 	}
 }
 
+/*
+ * Update the preimage, and the common lines in postimage,
+ * from buffer buf of length len. If postlen is 0 the postimage
+ * is updated in place, otherwise it's updated on a new buffer
+ * of length postlen
+ */
+
 static void update_pre_post_images(struct image *preimage,
 				   struct image *postimage,
 				   char *buf,
-				   size_t len)
+				   size_t len, size_t postlen)
 {
 	int i, ctx;
 	char *new, *old, *fixed;
@@ -1661,11 +1668,19 @@ static void update_pre_post_images(struct image *preimage,
 	*preimage = fixed_preimage;
 
 	/*
-	 * Adjust the common context lines in postimage, in place.
-	 * This is possible because whitespace fixing does not make
-	 * the string grow.
+	 * Adjust the common context lines in postimage. This can be
+	 * done in-place when we are just doing whitespace fixing,
+	 * which does not make the string grow, but needs a new buffer
+	 * when ignore_whitespace causes the update, since in this case
+	 * we could have e.g. tabs converted to multiple spaces.
+	 * We trust the caller to tell us if the update can be done
+	 * in place (postlen==0) or not.
 	 */
-	new = old = postimage->buf;
+	old = postimage->buf;
+	if (postlen)
+		new = postimage->buf = xmalloc(postlen);
+	else
+		new = old;
 	fixed = preimage->buf;
 	for (i = ctx = 0; i < postimage->nr; i++) {
 		size_t len = postimage->line[i].len;
@@ -1737,8 +1752,34 @@ static int match_fragment(struct image *img,
 	if ((match_end
 	     ? (try + preimage->len == img->len)
 	     : (try + preimage->len <= img->len)) &&
-	    lines_match(img->buf + try, preimage->buf, preimage->len))
+	    lines_match(img->buf + try, preimage->buf, preimage->len)) {
+		/*
+		 * When ignoring whitespace, we want to preserve the
+		 * target image whitespace in lines that are not modified,
+		 * so we update the preimage and the common lines in
+		 * the postimage to the target whitespace.
+		 */
+		if (ignore_whitespace) {
+			/*
+			 * New length for the updated pre and postimages
+			 */
+			size_t prelen = 0;
+			size_t postlen = postimage->len;
+			for (i = 0; i < preimage->nr; i++) {
+				size_t len = img->line[try_lno + i].len;
+				if (preimage->line[i].flag & LINE_COMMON)
+					postlen += len - preimage->line[i].len;
+				prelen += preimage->line[i].len = len;
+			}
+			fixed_buf = xmalloc(prelen);
+			memcpy(fixed_buf, img->buf + try, prelen);
+
+			update_pre_post_images(
+				preimage, postimage,
+				fixed_buf, prelen, postlen);
+		}
 		return 1;
+	}
 
 	if (ws_error_action != correct_ws_error)
 		return 0;
@@ -1799,7 +1840,7 @@ static int match_fragment(struct image *img,
 	 * hunk match.  Update the context lines in the postimage.
 	 */
 	update_pre_post_images(preimage, postimage,
-			       fixed_buf, buf - fixed_buf);
+			       fixed_buf, buf - fixed_buf, 0);
 	return 1;
 
  unmatch_exit:

-- 
1.6.3.3.512.g4fff