[PATCH] whitespace: symbolic links usually lack LF at the end

Subsystems: the rest

STALE208d

2 messages, 2 authors, 2026-02-05 · open the first message on its own page

[PATCH] whitespace: symbolic links usually lack LF at the end

From: Junio C Hamano <hidden>
Date: 2026-02-04 21:23:09

For a patch that touches a symbolic link, it is perfectly normal
that the payload ends with "\ No newline at end of file".  The
checks introduced recently to detect incomplete lines (i.e., a text
file that lack the newline on its final line) should not trigger.

Disable the check early for symbolic links, both in "git apply"
and "git diff" and test them.

Signed-off-by: Junio C Hamano <redacted>
---
 apply.c                    |  5 +++++
 diff.c                     | 20 ++++++++++++++++++--
 t/t4015-diff-whitespace.sh | 13 +++++++++++++
 t/t4124-apply-ws-rule.sh   | 31 +++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index 3de4aa4d2e..581aafb8be 100644
--- a/apply.c
+++ b/apply.c
@@ -2193,6 +2193,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
 		patch->ws_rule = whitespace_rule(state->repo->index,
 						 patch->old_name);
 
+	/* being an incomplete line is the norm for a symbolic link */
+	if ((patch->old_mode && S_ISLNK(patch->old_mode)) ||
+	    (patch->new_mode && S_ISLNK(patch->new_mode)))
+		patch->ws_rule &= ~WS_INCOMPLETE_LINE;
+
 	patchsize = parse_single_patch(state,
 				       buffer + offset + hdrsize,
 				       size - offset - hdrsize,
diff --git a/diff.c b/diff.c
index a68ddd2168..2b37432eed 100644
--- a/diff.c
+++ b/diff.c
@@ -1837,6 +1837,7 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *a_prefix, *b_prefix;
 	char *data_one, *data_two;
 	size_t size_one, size_two;
+	unsigned ws_rule;
 	struct emit_callback ecbdata;
 	struct strbuf out = STRBUF_INIT;
 
@@ -1859,9 +1860,14 @@ static void emit_rewrite_diff(const char *name_a,
 	size_one = fill_textconv(o->repo, textconv_one, one, &data_one);
 	size_two = fill_textconv(o->repo, textconv_two, two, &data_two);
 
+	ws_rule = whitespace_rule(o->repo->index, name_b);
+	if ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||
+	    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))
+		ws_rule &= ~WS_INCOMPLETE_LINE;
+
 	memset(&ecbdata, 0, sizeof(ecbdata));
 	ecbdata.color_diff = o->use_color;
-	ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
+	ecbdata.ws_rule = ws_rule;
 	ecbdata.opt = o;
 	if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
 		mmfile_t mf1, mf2;
@@ -3764,6 +3770,7 @@ static void builtin_diff(const char *name_a,
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
 		struct emit_callback ecbdata;
+		unsigned ws_rule;
 		const struct userdiff_funcname *pe;
 
 		if (must_show_header) {
@@ -3775,6 +3782,11 @@ static void builtin_diff(const char *name_a,
 		mf1.size = fill_textconv(o->repo, textconv_one, one, &mf1.ptr);
 		mf2.size = fill_textconv(o->repo, textconv_two, two, &mf2.ptr);
 
+		ws_rule = whitespace_rule(o->repo->index, name_b);
+		if ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||
+		    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))
+			ws_rule &= ~WS_INCOMPLETE_LINE;
+
 		pe = diff_funcname_pattern(o, one);
 		if (!pe)
 			pe = diff_funcname_pattern(o, two);
@@ -3786,7 +3798,7 @@ static void builtin_diff(const char *name_a,
 			lbl[0] = NULL;
 		ecbdata.label_path = lbl;
 		ecbdata.color_diff = o->use_color;
-		ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
+		ecbdata.ws_rule = ws_rule;
 		if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
 			check_blank_at_eof(&mf1, &mf2, &ecbdata);
 		ecbdata.opt = o;
@@ -3993,6 +4005,10 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 	data.ws_rule = whitespace_rule(o->repo->index, attr_path);
 	data.conflict_marker_size = ll_merge_marker_size(o->repo->index, attr_path);
 
+	if ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||
+	    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))
+		data.ws_rule &= ~WS_INCOMPLETE_LINE;
+
 	if (fill_mmfile(o->repo, &mf1, one) < 0 ||
 	    fill_mmfile(o->repo, &mf2, two) < 0)
 		die("unable to read files to diff");
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 3c8eb02e4f..903128f1d2 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -90,6 +90,19 @@ test_expect_success "new incomplete line in post-image" '
 	git -c core.whitespace=incomplete diff -R --check x
 '
 
+test_expect_success SYMLINKS "incomplete-line error is disabled for symlinks" '
+	test_when_finished "git reset --hard" &&
+	test_when_finished "rm -f mylink" &&
+	ln -s one mylink &&
+	git add mylink &&
+	ln -s -f two mylink &&
+
+	git -c core.whitespace=incomplete diff mylink &&
+	git -c core.whitespace=incomplete diff -R mylink &&
+	git -c core.whitespace=incomplete diff --check mylink &&
+	git -c core.whitespace=incomplete diff -R --check mylink
+'
+
 test_expect_success "Ray Lehtiniemi's example" '
 	cat <<-\EOF >x &&
 	do {
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 115a0f8579..f48d8bbf49 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -743,4 +743,35 @@ test_expect_success 'incomplete line modified at the end (error)' '
 	test_cmp sample target
 '
 
+test_expect_success "incomplete-line error is disabled for symlinks" '
+	test_when_finished "git reset" &&
+	test_when_finished "rm -f patch.txt" &&
+	oneblob=$(printf "one" | git hash-object --stdin -w -t blob) &&
+	twoblob=$(printf "two" | git hash-object --stdin -w -t blob) &&
+
+	git update-index --add --cacheinfo "120000,$oneblob,mylink" &&
+
+	oneshort=$(git rev-parse --short $oneblob) &&
+	twoshort=$(git rev-parse --short $twoblob) &&
+	cat >patch.txt <<-EOF &&
+	diff --git a/mylink b/mylink
+	index $oneshort..$twoshort 120000
+	--- a/mylink
+	+++ b/mylink
+	@@ -1 +1 @@
+	-one
+	\ No newline at end of file
+	+two
+	\ No newline at end of file
+	EOF
+
+	git -c core.whitespace=incomplete apply --cached --check patch.txt &&
+
+	git -c core.whitespace=incomplete apply --cached --whitespace=error \
+		patch.txt &&
+
+	git -c core.whitespace=incomplete apply --cached -R --whitespace=error \
+		patch.txt
+'
+
 test_done
-- 
2.53.0-169-ga09cd4eb64

Re: [PATCH] whitespace: symbolic links usually lack LF at the end

From: Patrick Steinhardt <hidden>
Date: 2026-02-05 12:21:25

On Wed, Feb 04, 2026 at 01:23:06PM -0800, Junio C Hamano wrote:
quoted hunk
diff --git a/apply.c b/apply.c
index 3de4aa4d2e..581aafb8be 100644
--- a/apply.c
+++ b/apply.c
@@ -2193,6 +2193,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
 		patch->ws_rule = whitespace_rule(state->repo->index,
 						 patch->old_name);
 
+	/* being an incomplete line is the norm for a symbolic link */
+	if ((patch->old_mode && S_ISLNK(patch->old_mode)) ||
+	    (patch->new_mode && S_ISLNK(patch->new_mode)))
+		patch->ws_rule &= ~WS_INCOMPLETE_LINE;
+
 	patchsize = parse_single_patch(state,
 				       buffer + offset + hdrsize,
 				       size - offset - hdrsize,
Hm. Wouldn't that mean that we disable this check for both sides of a
diff if either of them is a symlink? That's typically fine, but if the
diff also contains a mode change it might not be.

I'd suggest that we only disable this check in case either:

  - One side doesn't exist, the other is a symbolic link.

  - Both sides are a symbolic link.

Another question is whether we support symref targets that end in a
newline. I guess the answer is going to be some form of "yes", and in
that case we could of course loose some information. But honestly, this
is so much of an edge case that I don't really worry about it too much.

Thanks!

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help