{"thread":{"id":"64920","subject":"[PATCH] whitespace: symbolic links usually lack LF at the end","startedAt":"2026-02-04T21:23:09Z","lastAt":"2026-02-06T16:58:08Z","messageCount":7,"participants":["Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"535193","messageId":"xmqqecn0nqyt.fsf@gitster.g","threadId":"64920","inReplyTo":null,"subject":"[PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-04T21:23:06Z","receivedAt":"2026-02-04T21:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"For a patch that touches a symbolic link, it is perfectly normal\nthat the payload ends with \"\\ No newline at end of file\".  The\nchecks introduced recently to detect incomplete lines (i.e., a text\nfile that lack the newline on its final line) should not trigger.\n\nDisable the check early for symbolic links, both in \"git apply\"\nand \"git diff\" and test them.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n apply.c                    |  5 +++++\n diff.c                     | 20 ++++++++++++++++++--\n t/t4015-diff-whitespace.sh | 13 +++++++++++++\n t/t4124-apply-ws-rule.sh   | 31 +++++++++++++++++++++++++++++++\n 4 files changed, 67 insertions(+), 2 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 3de4aa4d2e..581aafb8be 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2193,6 +2193,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n \t\tpatch->ws_rule = whitespace_rule(state->repo->index,\n \t\t\t\t\t\t patch->old_name);\n \n+\t/* being an incomplete line is the norm for a symbolic link */\n+\tif ((patch->old_mode && S_ISLNK(patch->old_mode)) ||\n+\t    (patch->new_mode && S_ISLNK(patch->new_mode)))\n+\t\tpatch->ws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \tpatchsize = parse_single_patch(state,\n \t\t\t\t       buffer + offset + hdrsize,\n \t\t\t\t       size - offset - hdrsize,\ndiff --git a/diff.c b/diff.c\nindex a68ddd2168..2b37432eed 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1837,6 +1837,7 @@ static void emit_rewrite_diff(const char *name_a,\n \tconst char *a_prefix, *b_prefix;\n \tchar *data_one, *data_two;\n \tsize_t size_one, size_two;\n+\tunsigned ws_rule;\n \tstruct emit_callback ecbdata;\n \tstruct strbuf out = STRBUF_INIT;\n \n@@ -1859,9 +1860,14 @@ static void emit_rewrite_diff(const char *name_a,\n \tsize_one = fill_textconv(o->repo, textconv_one, one, &data_one);\n \tsize_two = fill_textconv(o->repo, textconv_two, two, &data_two);\n \n+\tws_rule = whitespace_rule(o->repo->index, name_b);\n+\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n+\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n+\t\tws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \tmemset(&ecbdata, 0, sizeof(ecbdata));\n \tecbdata.color_diff = o->use_color;\n-\tecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);\n+\tecbdata.ws_rule = ws_rule;\n \tecbdata.opt = o;\n \tif (ecbdata.ws_rule & WS_BLANK_AT_EOF) {\n \t\tmmfile_t mf1, mf2;\n@@ -3764,6 +3770,7 @@ static void builtin_diff(const char *name_a,\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n \t\tstruct emit_callback ecbdata;\n+\t\tunsigned ws_rule;\n \t\tconst struct userdiff_funcname *pe;\n \n \t\tif (must_show_header) {\n@@ -3775,6 +3782,11 @@ static void builtin_diff(const char *name_a,\n \t\tmf1.size = fill_textconv(o->repo, textconv_one, one, &mf1.ptr);\n \t\tmf2.size = fill_textconv(o->repo, textconv_two, two, &mf2.ptr);\n \n+\t\tws_rule = whitespace_rule(o->repo->index, name_b);\n+\t\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n+\t\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n+\t\t\tws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \t\tpe = diff_funcname_pattern(o, one);\n \t\tif (!pe)\n \t\t\tpe = diff_funcname_pattern(o, two);\n@@ -3786,7 +3798,7 @@ static void builtin_diff(const char *name_a,\n \t\t\tlbl[0] = NULL;\n \t\tecbdata.label_path = lbl;\n \t\tecbdata.color_diff = o->use_color;\n-\t\tecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);\n+\t\tecbdata.ws_rule = ws_rule;\n \t\tif (ecbdata.ws_rule & WS_BLANK_AT_EOF)\n \t\t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n \t\tecbdata.opt = o;\n@@ -3993,6 +4005,10 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \tdata.ws_rule = whitespace_rule(o->repo->index, attr_path);\n \tdata.conflict_marker_size = ll_merge_marker_size(o->repo->index, attr_path);\n \n+\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n+\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n+\t\tdata.ws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \tif (fill_mmfile(o->repo, &mf1, one) < 0 ||\n \t    fill_mmfile(o->repo, &mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 3c8eb02e4f..903128f1d2 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -90,6 +90,19 @@ test_expect_success \"new incomplete line in post-image\" '\n \tgit -c core.whitespace=incomplete diff -R --check x\n '\n \n+test_expect_success SYMLINKS \"incomplete-line error is disabled for symlinks\" '\n+\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"rm -f mylink\" &&\n+\tln -s one mylink &&\n+\tgit add mylink &&\n+\tln -s -f two mylink &&\n+\n+\tgit -c core.whitespace=incomplete diff mylink &&\n+\tgit -c core.whitespace=incomplete diff -R mylink &&\n+\tgit -c core.whitespace=incomplete diff --check mylink &&\n+\tgit -c core.whitespace=incomplete diff -R --check mylink\n+'\n+\n test_expect_success \"Ray Lehtiniemi's example\" '\n \tcat <<-\\EOF >x &&\n \tdo {\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex 115a0f8579..f48d8bbf49 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -743,4 +743,35 @@ test_expect_success 'incomplete line modified at the end (error)' '\n \ttest_cmp sample target\n '\n \n+test_expect_success \"incomplete-line error is disabled for symlinks\" '\n+\ttest_when_finished \"git reset\" &&\n+\ttest_when_finished \"rm -f patch.txt\" &&\n+\toneblob=$(printf \"one\" | git hash-object --stdin -w -t blob) &&\n+\ttwoblob=$(printf \"two\" | git hash-object --stdin -w -t blob) &&\n+\n+\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n+\n+\toneshort=$(git rev-parse --short $oneblob) &&\n+\ttwoshort=$(git rev-parse --short $twoblob) &&\n+\tcat >patch.txt <<-EOF &&\n+\tdiff --git a/mylink b/mylink\n+\tindex $oneshort..$twoshort 120000\n+\t--- a/mylink\n+\t+++ b/mylink\n+\t@@ -1 +1 @@\n+\t-one\n+\t\\ No newline at end of file\n+\t+two\n+\t\\ No newline at end of file\n+\tEOF\n+\n+\tgit -c core.whitespace=incomplete apply --cached --check patch.txt &&\n+\n+\tgit -c core.whitespace=incomplete apply --cached --whitespace=error \\\n+\t\tpatch.txt &&\n+\n+\tgit -c core.whitespace=incomplete apply --cached -R --whitespace=error \\\n+\t\tpatch.txt\n+'\n+\n test_done\n-- \n2.53.0-169-ga09cd4eb64\n\n"},{"id":"535235","messageId":"aYSLP1LqBiMwur3O@pks.im","threadId":"64920","inReplyTo":"xmqqecn0nqyt.fsf@gitster.g","subject":"Re: [PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-05T12:21:19Z","receivedAt":"2026-02-05T12:21:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Feb 04, 2026 at 01:23:06PM -0800, Junio C Hamano wrote:\n> diff --git a/apply.c b/apply.c\n> index 3de4aa4d2e..581aafb8be 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -2193,6 +2193,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n>  \t\tpatch->ws_rule = whitespace_rule(state->repo->index,\n>  \t\t\t\t\t\t patch->old_name);\n>  \n> +\t/* being an incomplete line is the norm for a symbolic link */\n> +\tif ((patch->old_mode && S_ISLNK(patch->old_mode)) ||\n> +\t    (patch->new_mode && S_ISLNK(patch->new_mode)))\n> +\t\tpatch->ws_rule &= ~WS_INCOMPLETE_LINE;\n> +\n>  \tpatchsize = parse_single_patch(state,\n>  \t\t\t\t       buffer + offset + hdrsize,\n>  \t\t\t\t       size - offset - hdrsize,\n\nHm. Wouldn't that mean that we disable this check for both sides of a\ndiff if either of them is a symlink? That's typically fine, but if the\ndiff also contains a mode change it might not be.\n\nI'd suggest that we only disable this check in case either:\n\n  - One side doesn't exist, the other is a symbolic link.\n\n  - Both sides are a symbolic link.\n\nAnother question is whether we support symref targets that end in a\nnewline. I guess the answer is going to be some form of \"yes\", and in\nthat case we could of course loose some information. But honestly, this\nis so much of an edge case that I don't really worry about it too much.\n\nThanks!\n\nPatrick\n"},{"id":"535247","messageId":"xmqqms1nmbog.fsf@gitster.g","threadId":"64920","inReplyTo":"aYSLP1LqBiMwur3O@pks.im","subject":"Re: [PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-05T15:50:55Z","receivedAt":"2026-02-05T15:50:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I'd suggest that we only disable this check in case either:\n>\n>   - One side doesn't exist, the other is a symbolic link.\n>\n>   - Both sides are a symbolic link.\n\nHmm.  That is indeed a thoguht.  But we do not want to complain in\ntext-to-symlink transition that postimage lacks the terminating LF,\nso the above rules may be a good start but will need further\ntweaking, I am afraid.\n\n> Another question is whether we support symref targets that end in a\n> newline. I guess the answer is going to be some form of \"yes\", and in\n> that case we could of course loose some information. But honestly, this\n> is so much of an edge case that I don't really worry about it too much.\n\nDo we track, apply and diff any symrefs?  I thought that we do not\ntouch anything inside .git/ and symrefs live inside .git/refs/\n(except for .git/HEAD)?\n"},{"id":"535311","messageId":"aYWKyOIMPLiDxqnj@pks.im","threadId":"64920","inReplyTo":"xmqqms1nmbog.fsf@gitster.g","subject":"Re: [PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T06:31:36Z","receivedAt":"2026-02-06T06:31:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 05, 2026 at 07:50:55AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > I'd suggest that we only disable this check in case either:\n> >\n> >   - One side doesn't exist, the other is a symbolic link.\n> >\n> >   - Both sides are a symbolic link.\n> \n> Hmm.  That is indeed a thoguht.  But we do not want to complain in\n> text-to-symlink transition that postimage lacks the terminating LF,\n> so the above rules may be a good start but will need further\n> tweaking, I am afraid.\n\nAh, right. Only the other way around, when converting from LF to text.\n\n> > Another question is whether we support symref targets that end in a\n> > newline. I guess the answer is going to be some form of \"yes\", and in\n> > that case we could of course loose some information. But honestly, this\n> > is so much of an edge case that I don't really worry about it too much.\n> \n> Do we track, apply and diff any symrefs?  I thought that we do not\n> touch anything inside .git/ and symrefs live inside .git/refs/\n> (except for .git/HEAD)?\n\nEh, I didn't mean symrefs here, but symbolic links :) Tools like ln(1)\nseem to strip trailing newlines, but if you try hard enough you'll\nprobably be able to create symlinks that have a target with trailing\nnewline.\n\nPatrick\n"},{"id":"535361","messageId":"xmqqv7g9hm9l.fsf@gitster.g","threadId":"64920","inReplyTo":"aYWKyOIMPLiDxqnj@pks.im","subject":"Re: [PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-06T16:25:42Z","receivedAt":"2026-02-06T16:25:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Thu, Feb 05, 2026 at 07:50:55AM -0800, Junio C Hamano wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> \n>> > I'd suggest that we only disable this check in case either:\n>> >\n>> >   - One side doesn't exist, the other is a symbolic link.\n>> >\n>> >   - Both sides are a symbolic link.\n>> \n>> Hmm.  That is indeed a thoguht.  But we do not want to complain in\n>> text-to-symlink transition that postimage lacks the terminating LF,\n>> so the above rules may be a good start but will need further\n>> tweaking, I am afraid.\n>\n> Ah, right. Only the other way around, when converting from LF to text.\n\nI've decided to use the \"disable only when the side that appears\npostimage (taking --reverse option into account) is a symbolic link\"\nrule.\n\nStrictly speaking, \"diff\" (but not \"apply\") has wsErrorHighlight\nfeature where it can be configured to complain about whitespace\nglitches in both pre- and postimage, so it is technically not\nsufficient, but it is not worth supporting diff.wsErrorHighlight\nthat is set to anything but \"new\" (or \"default\" which is its\nsynonym).\n\n> Eh, I didn't mean symrefs here, but symbolic links :) Tools like ln(1)\n> seem to strip trailing newlines, but if you try hard enough you'll\n> probably be able to create symlinks that have a target with trailing\n> newline.\n\nYes, as you can create a file whose name contains a newline, a name\nthat ends in a newline is a valid filename that \"ln -s\" may want to\nsupport.  I am reasonably sure that we do not want to flag such a\nsymbolic link as whitespace damaged.\n"},{"id":"535362","messageId":"xmqqpl6hhm96.fsf@gitster.g","threadId":"64920","inReplyTo":"xmqqecn0nqyt.fsf@gitster.g","subject":"[PATCH v2] whitespace: symbolic links usually lack LF at the end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-06T16:25:57Z","receivedAt":"2026-02-06T16:25:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"For a patch that touches a symbolic link, it is perfectly normal\nthat the contents ends with \"\\ No newline at end of file\".  The\nchecks introduced recently to detect incomplete lines (i.e., a text\nfile that lack the newline on its final line) should not trigger.\n\nDisable the check early for symbolic links, both in \"git apply\" and\n\"git diff\" and test them.  For \"git apply\", we check only when the\npostimage is a symbolic link regardless of the preimage, and we only\ncare about preimage when applying in reverse.  Similarly, \"git diff\"\nwould warn only when the postimage is a symbolic link, or the\npreimage when running \"git diff -R\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * v1 used to disable whitespace=incomplete-line when either side of\n   comparison is a symbolic link; this iteration only cares about\n   the case where the postimage is a symbolic link.  If you turn\n   what used to be a symlink into a text file, and if incomplete\n   line detection is turned on, you would want to make sure that the\n   resulting text file is without an incomplete line.\n\n apply.c                    | 20 +++++++++\n diff.c                     | 22 +++++++++-\n t/t4015-diff-whitespace.sh | 26 ++++++++++++\n t/t4124-apply-ws-rule.sh   | 86 ++++++++++++++++++++++++++++++++++++++\n 4 files changed, 152 insertions(+), 2 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex c9fb45247d..f01204d15b 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -1725,6 +1725,26 @@ static int parse_fragment(struct apply_state *state,\n \tunsigned long oldlines, newlines;\n \tunsigned long leading, trailing;\n \n+\t/* do not complain a symbolic link being an incomplete line */\n+\tif (patch->ws_rule & WS_INCOMPLETE_LINE) {\n+\t\t/*\n+\t\t * We want to figure out if the postimage is a\n+\t\t * symbolic link when applying the patch normally, or\n+\t\t * if the preimage is a symbolic link when applying\n+\t\t * the patch in reverse.  A normal patch only has\n+\t\t * old_mode without new_mode.  If it changes the\n+\t\t * filemode, new_mode has value, which is different\n+\t\t * from old_mode.\n+\t\t */\n+\t\tunsigned mode = (state->apply_in_reverse\n+\t\t\t\t ? patch->old_mode\n+\t\t\t\t : patch->new_mode\n+\t\t\t\t ? patch->new_mode\n+\t\t\t\t : patch->old_mode);\n+\t\tif (mode && S_ISLNK(mode))\n+\t\t\tpatch->ws_rule &= ~WS_INCOMPLETE_LINE;\n+\t}\n+\n \toffset = parse_fragment_header(line, len, fragment);\n \tif (offset < 0)\n \t\treturn -1;\ndiff --git a/diff.c b/diff.c\nindex 7b7cd50dc2..9e4b92ed69 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1834,6 +1834,7 @@ static void emit_rewrite_diff(const char *name_a,\n \tconst char *a_prefix, *b_prefix;\n \tchar *data_one, *data_two;\n \tsize_t size_one, size_two;\n+\tunsigned ws_rule;\n \tstruct emit_callback ecbdata;\n \tstruct strbuf out = STRBUF_INIT;\n \n@@ -1856,9 +1857,15 @@ static void emit_rewrite_diff(const char *name_a,\n \tsize_one = fill_textconv(o->repo, textconv_one, one, &data_one);\n \tsize_two = fill_textconv(o->repo, textconv_two, two, &data_two);\n \n+\tws_rule = whitespace_rule(o->repo->index, name_b);\n+\n+\t/* symlink being an incomplete line is not a news */\n+\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n+\t\tws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \tmemset(&ecbdata, 0, sizeof(ecbdata));\n \tecbdata.color_diff = o->use_color;\n-\tecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);\n+\tecbdata.ws_rule = ws_rule;\n \tecbdata.opt = o;\n \tif (ecbdata.ws_rule & WS_BLANK_AT_EOF) {\n \t\tmmfile_t mf1, mf2;\n@@ -3762,6 +3769,7 @@ static void builtin_diff(const char *name_a,\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n \t\tstruct emit_callback ecbdata;\n+\t\tunsigned ws_rule;\n \t\tconst struct userdiff_funcname *pe;\n \n \t\tif (must_show_header) {\n@@ -3773,6 +3781,12 @@ static void builtin_diff(const char *name_a,\n \t\tmf1.size = fill_textconv(o->repo, textconv_one, one, &mf1.ptr);\n \t\tmf2.size = fill_textconv(o->repo, textconv_two, two, &mf2.ptr);\n \n+\t\tws_rule = whitespace_rule(o->repo->index, name_b);\n+\n+\t\t/* symlink being an incomplete line is not a news */\n+\t\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n+\t\t\tws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \t\tpe = diff_funcname_pattern(o, one);\n \t\tif (!pe)\n \t\t\tpe = diff_funcname_pattern(o, two);\n@@ -3784,7 +3798,7 @@ static void builtin_diff(const char *name_a,\n \t\t\tlbl[0] = NULL;\n \t\tecbdata.label_path = lbl;\n \t\tecbdata.color_diff = o->use_color;\n-\t\tecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);\n+\t\tecbdata.ws_rule = ws_rule;\n \t\tif (ecbdata.ws_rule & WS_BLANK_AT_EOF)\n \t\t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n \t\tecbdata.opt = o;\n@@ -3991,6 +4005,10 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \tdata.ws_rule = whitespace_rule(o->repo->index, attr_path);\n \tdata.conflict_marker_size = ll_merge_marker_size(o->repo->index, attr_path);\n \n+\t/* symlink being an incomplete line is not a news */\n+\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n+\t\tdata.ws_rule &= ~WS_INCOMPLETE_LINE;\n+\n \tif (fill_mmfile(o->repo, &mf1, one) < 0 ||\n \t    fill_mmfile(o->repo, &mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 3c8eb02e4f..b691d29479 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -90,6 +90,32 @@ test_expect_success \"new incomplete line in post-image\" '\n \tgit -c core.whitespace=incomplete diff -R --check x\n '\n \n+test_expect_success SYMLINKS \"incomplete-line error is disabled for symlinks\" '\n+\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"rm -f mylink\" &&\n+\n+\t# a regular file with an incomplete line\n+\tprintf \"%s\" one >mylink &&\n+\tgit add mylink &&\n+\n+\t# a symbolic link\n+\trm mylink &&\n+\tln -s two mylink &&\n+\n+\tgit -c diff.color=always -c core.whitespace=incomplete \\\n+\t\tdiff mylink >forward.raw &&\n+\ttest_decode_color >forward <forward.raw &&\n+\ttest_grep ! \"<BRED>\\\\\\\\ No newline at end of file<RESET>\" forward &&\n+\n+\tgit -c diff.color=always -c core.whitespace=incomplete \\\n+\t\tdiff -R mylink >reverse.raw &&\n+\ttest_decode_color >reverse <reverse.raw &&\n+\ttest_grep \"<BRED>\\\\\\\\ No newline at end of file<RESET>\" reverse &&\n+\n+\tgit -c core.whitespace=incomplete diff --check mylink &&\n+\ttest_must_fail git -c core.whitespace=incomplete diff --check -R mylink\n+'\n+\n test_expect_success \"Ray Lehtiniemi's example\" '\n \tcat <<-\\EOF >x &&\n \tdo {\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex 115a0f8579..29ea7d4268 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -743,4 +743,90 @@ test_expect_success 'incomplete line modified at the end (error)' '\n \ttest_cmp sample target\n '\n \n+test_expect_success \"incomplete-line error is disabled for symlinks\" '\n+\ttest_when_finished \"git reset\" &&\n+\ttest_when_finished \"rm -f patch.txt\" &&\n+\toneblob=$(printf \"one\" | git hash-object --stdin -w -t blob) &&\n+\ttwoblob=$(printf \"two\" | git hash-object --stdin -w -t blob) &&\n+\n+\toneshort=$(git rev-parse --short $oneblob) &&\n+\ttwoshort=$(git rev-parse --short $twoblob) &&\n+\n+\tcat >patch0.txt <<-EOF &&\n+\tdiff --git a/mylink b/mylink\n+\tindex $oneshort..$twoshort 120000\n+\t--- a/mylink\n+\t+++ b/mylink\n+\t@@ -1 +1 @@\n+\t-one\n+\t\\ No newline at end of file\n+\t+two\n+\t\\ No newline at end of file\n+\tEOF\n+\n+\t# the index has the preimage symlink\n+\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n+\n+\t# check the patch going forward and reverse\n+\tgit -c core.whitespace=incomplete apply --cached --check \\\n+\t\t--whitespace=error patch0.txt &&\n+\n+\tgit update-index --add --cacheinfo \"120000,$twoblob,mylink\" &&\n+\tgit -c core.whitespace=incomplete apply --cached --check \\\n+\t\t--whitespace=error -R patch0.txt &&\n+\n+\t# the patch turns it into the postimage symlink\n+\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n+\tgit -c core.whitespace=incomplete apply --cached --whitespace=error \\\n+\t\tpatch0.txt &&\n+\n+\t# and then back.\n+\tgit -c core.whitespace=incomplete apply --cached -R --whitespace=error \\\n+\t\tpatch0.txt &&\n+\n+\t# a text file turns into a symlink\n+\tcat >patch1.txt <<-EOF &&\n+\tdiff --git a/mylink b/mylink\n+\tdeleted file mode 100644\n+\tindex $oneshort..0000000\n+\t--- a/mylink\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-one\n+\t\\ No newline at end of file\n+\tdiff --git a/mylink b/mylink\n+\tnew file mode 120000\n+\tindex 0000000..$twoshort\n+\t--- /dev/null\n+\t+++ b/mylink\n+\t@@ -0,0 +1 @@\n+\t+two\n+\t\\ No newline at end of file\n+\tEOF\n+\n+\t# the index has the preimage text\n+\tgit update-index --cacheinfo \"100644,$oneblob,mylink\" &&\n+\n+\t# check\n+\tgit -c core.whitespace=incomplete apply --cached \\\n+\t\t--check --whitespace=error patch1.txt &&\n+\n+\t# reverse, leaving an incomplete text file, should error\n+\tgit update-index --cacheinfo \"120000,$twoblob,mylink\" &&\n+\ttest_must_fail git -c core.whitespace=incomplete \\\n+\t\tapply --cached --check --whitespace=error -R patch1.txt &&\n+\n+\t# apply to create a symbolic link\n+\tgit update-index --cacheinfo \"100644,$oneblob,mylink\" &&\n+\tgit -c core.whitespace=incomplete apply --cached --whitespace=error \\\n+\t\tpatch1.txt &&\n+\n+\t# turning it back into an incomplete text file is an error\n+\ttest_must_fail git -c core.whitespace=incomplete \\\n+\t\tapply --cached --whitespace=error -R patch1.txt\n+\n+\n+\n+'\n+\n test_done\n\nInterdiff against v1:\n  diff --git a/apply.c b/apply.c\n  index 81ea174637..f01204d15b 100644\n  --- a/apply.c\n  +++ b/apply.c\n  @@ -1725,6 +1725,26 @@ static int parse_fragment(struct apply_state *state,\n   \tunsigned long oldlines, newlines;\n   \tunsigned long leading, trailing;\n   \n  +\t/* do not complain a symbolic link being an incomplete line */\n  +\tif (patch->ws_rule & WS_INCOMPLETE_LINE) {\n  +\t\t/*\n  +\t\t * We want to figure out if the postimage is a\n  +\t\t * symbolic link when applying the patch normally, or\n  +\t\t * if the preimage is a symbolic link when applying\n  +\t\t * the patch in reverse.  A normal patch only has\n  +\t\t * old_mode without new_mode.  If it changes the\n  +\t\t * filemode, new_mode has value, which is different\n  +\t\t * from old_mode.\n  +\t\t */\n  +\t\tunsigned mode = (state->apply_in_reverse\n  +\t\t\t\t ? patch->old_mode\n  +\t\t\t\t : patch->new_mode\n  +\t\t\t\t ? patch->new_mode\n  +\t\t\t\t : patch->old_mode);\n  +\t\tif (mode && S_ISLNK(mode))\n  +\t\t\tpatch->ws_rule &= ~WS_INCOMPLETE_LINE;\n  +\t}\n  +\n   \toffset = parse_fragment_header(line, len, fragment);\n   \tif (offset < 0)\n   \t\treturn -1;\n  @@ -2193,11 +2213,6 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si\n   \t\tpatch->ws_rule = whitespace_rule(state->repo->index,\n   \t\t\t\t\t\t patch->old_name);\n   \n  -\t/* being an incomplete line is the norm for a symbolic link */\n  -\tif ((patch->old_mode && S_ISLNK(patch->old_mode)) ||\n  -\t    (patch->new_mode && S_ISLNK(patch->new_mode)))\n  -\t\tpatch->ws_rule &= ~WS_INCOMPLETE_LINE;\n  -\n   \tpatchsize = parse_single_patch(state,\n   \t\t\t\t       buffer + offset + hdrsize,\n   \t\t\t\t       size - offset - hdrsize,\n  diff --git a/diff.c b/diff.c\n  index c53aebc4a4..9e4b92ed69 100644\n  --- a/diff.c\n  +++ b/diff.c\n  @@ -1858,8 +1858,9 @@ static void emit_rewrite_diff(const char *name_a,\n   \tsize_two = fill_textconv(o->repo, textconv_two, two, &data_two);\n   \n   \tws_rule = whitespace_rule(o->repo->index, name_b);\n  -\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n  -\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n  +\n  +\t/* symlink being an incomplete line is not a news */\n  +\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n   \t\tws_rule &= ~WS_INCOMPLETE_LINE;\n   \n   \tmemset(&ecbdata, 0, sizeof(ecbdata));\n  @@ -3781,8 +3782,9 @@ static void builtin_diff(const char *name_a,\n   \t\tmf2.size = fill_textconv(o->repo, textconv_two, two, &mf2.ptr);\n   \n   \t\tws_rule = whitespace_rule(o->repo->index, name_b);\n  -\t\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n  -\t\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n  +\n  +\t\t/* symlink being an incomplete line is not a news */\n  +\t\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n   \t\t\tws_rule &= ~WS_INCOMPLETE_LINE;\n   \n   \t\tpe = diff_funcname_pattern(o, one);\n  @@ -4003,8 +4005,8 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n   \tdata.ws_rule = whitespace_rule(o->repo->index, attr_path);\n   \tdata.conflict_marker_size = ll_merge_marker_size(o->repo->index, attr_path);\n   \n  -\tif ((DIFF_FILE_VALID(one) && S_ISLNK(one->mode)) ||\n  -\t    (DIFF_FILE_VALID(two) && S_ISLNK(two->mode)))\n  +\t/* symlink being an incomplete line is not a news */\n  +\tif (DIFF_FILE_VALID(two) && S_ISLNK(two->mode))\n   \t\tdata.ws_rule &= ~WS_INCOMPLETE_LINE;\n   \n   \tif (fill_mmfile(o->repo, &mf1, one) < 0 ||\n  diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\n  index 903128f1d2..b691d29479 100755\n  --- a/t/t4015-diff-whitespace.sh\n  +++ b/t/t4015-diff-whitespace.sh\n  @@ -93,14 +93,27 @@ test_expect_success \"new incomplete line in post-image\" '\n   test_expect_success SYMLINKS \"incomplete-line error is disabled for symlinks\" '\n   \ttest_when_finished \"git reset --hard\" &&\n   \ttest_when_finished \"rm -f mylink\" &&\n  -\tln -s one mylink &&\n  +\n  +\t# a regular file with an incomplete line\n  +\tprintf \"%s\" one >mylink &&\n   \tgit add mylink &&\n  -\tln -s -f two mylink &&\n   \n  -\tgit -c core.whitespace=incomplete diff mylink &&\n  -\tgit -c core.whitespace=incomplete diff -R mylink &&\n  +\t# a symbolic link\n  +\trm mylink &&\n  +\tln -s two mylink &&\n  +\n  +\tgit -c diff.color=always -c core.whitespace=incomplete \\\n  +\t\tdiff mylink >forward.raw &&\n  +\ttest_decode_color >forward <forward.raw &&\n  +\ttest_grep ! \"<BRED>\\\\\\\\ No newline at end of file<RESET>\" forward &&\n  +\n  +\tgit -c diff.color=always -c core.whitespace=incomplete \\\n  +\t\tdiff -R mylink >reverse.raw &&\n  +\ttest_decode_color >reverse <reverse.raw &&\n  +\ttest_grep \"<BRED>\\\\\\\\ No newline at end of file<RESET>\" reverse &&\n  +\n   \tgit -c core.whitespace=incomplete diff --check mylink &&\n  -\tgit -c core.whitespace=incomplete diff -R --check mylink\n  +\ttest_must_fail git -c core.whitespace=incomplete diff --check -R mylink\n   '\n   \n   test_expect_success \"Ray Lehtiniemi's example\" '\n  diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\n  index f48d8bbf49..29ea7d4268 100755\n  --- a/t/t4124-apply-ws-rule.sh\n  +++ b/t/t4124-apply-ws-rule.sh\n  @@ -749,11 +749,10 @@ test_expect_success \"incomplete-line error is disabled for symlinks\" '\n   \toneblob=$(printf \"one\" | git hash-object --stdin -w -t blob) &&\n   \ttwoblob=$(printf \"two\" | git hash-object --stdin -w -t blob) &&\n   \n  -\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n  -\n   \toneshort=$(git rev-parse --short $oneblob) &&\n   \ttwoshort=$(git rev-parse --short $twoblob) &&\n  -\tcat >patch.txt <<-EOF &&\n  +\n  +\tcat >patch0.txt <<-EOF &&\n   \tdiff --git a/mylink b/mylink\n   \tindex $oneshort..$twoshort 120000\n   \t--- a/mylink\n  @@ -765,13 +764,69 @@ test_expect_success \"incomplete-line error is disabled for symlinks\" '\n   \t\\ No newline at end of file\n   \tEOF\n   \n  -\tgit -c core.whitespace=incomplete apply --cached --check patch.txt &&\n  +\t# the index has the preimage symlink\n  +\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n   \n  +\t# check the patch going forward and reverse\n  +\tgit -c core.whitespace=incomplete apply --cached --check \\\n  +\t\t--whitespace=error patch0.txt &&\n  +\n  +\tgit update-index --add --cacheinfo \"120000,$twoblob,mylink\" &&\n  +\tgit -c core.whitespace=incomplete apply --cached --check \\\n  +\t\t--whitespace=error -R patch0.txt &&\n  +\n  +\t# the patch turns it into the postimage symlink\n  +\tgit update-index --add --cacheinfo \"120000,$oneblob,mylink\" &&\n   \tgit -c core.whitespace=incomplete apply --cached --whitespace=error \\\n  -\t\tpatch.txt &&\n  +\t\tpatch0.txt &&\n   \n  +\t# and then back.\n   \tgit -c core.whitespace=incomplete apply --cached -R --whitespace=error \\\n  -\t\tpatch.txt\n  +\t\tpatch0.txt &&\n  +\n  +\t# a text file turns into a symlink\n  +\tcat >patch1.txt <<-EOF &&\n  +\tdiff --git a/mylink b/mylink\n  +\tdeleted file mode 100644\n  +\tindex $oneshort..0000000\n  +\t--- a/mylink\n  +\t+++ /dev/null\n  +\t@@ -1 +0,0 @@\n  +\t-one\n  +\t\\ No newline at end of file\n  +\tdiff --git a/mylink b/mylink\n  +\tnew file mode 120000\n  +\tindex 0000000..$twoshort\n  +\t--- /dev/null\n  +\t+++ b/mylink\n  +\t@@ -0,0 +1 @@\n  +\t+two\n  +\t\\ No newline at end of file\n  +\tEOF\n  +\n  +\t# the index has the preimage text\n  +\tgit update-index --cacheinfo \"100644,$oneblob,mylink\" &&\n  +\n  +\t# check\n  +\tgit -c core.whitespace=incomplete apply --cached \\\n  +\t\t--check --whitespace=error patch1.txt &&\n  +\n  +\t# reverse, leaving an incomplete text file, should error\n  +\tgit update-index --cacheinfo \"120000,$twoblob,mylink\" &&\n  +\ttest_must_fail git -c core.whitespace=incomplete \\\n  +\t\tapply --cached --check --whitespace=error -R patch1.txt &&\n  +\n  +\t# apply to create a symbolic link\n  +\tgit update-index --cacheinfo \"100644,$oneblob,mylink\" &&\n  +\tgit -c core.whitespace=incomplete apply --cached --whitespace=error \\\n  +\t\tpatch1.txt &&\n  +\n  +\t# turning it back into an incomplete text file is an error\n  +\ttest_must_fail git -c core.whitespace=incomplete \\\n  +\t\tapply --cached --whitespace=error -R patch1.txt\n  +\n  +\n  +\n   '\n   \n   test_done\n-- \n2.53.0-179-g8fac285501\n\n"},{"id":"535368","messageId":"aYYdmUd4uqgK2Z1_@pks.im","threadId":"64920","inReplyTo":"xmqqv7g9hm9l.fsf@gitster.g","subject":"Re: [PATCH] whitespace: symbolic links usually lack LF at the end","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-06T16:58:01Z","receivedAt":"2026-02-06T16:58:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Feb 06, 2026 at 08:25:42AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > On Thu, Feb 05, 2026 at 07:50:55AM -0800, Junio C Hamano wrote:\n> >> Patrick Steinhardt <ps@pks.im> writes:\n> >> \n> >> > I'd suggest that we only disable this check in case either:\n> >> >\n> >> >   - One side doesn't exist, the other is a symbolic link.\n> >> >\n> >> >   - Both sides are a symbolic link.\n> >> \n> >> Hmm.  That is indeed a thoguht.  But we do not want to complain in\n> >> text-to-symlink transition that postimage lacks the terminating LF,\n> >> so the above rules may be a good start but will need further\n> >> tweaking, I am afraid.\n> >\n> > Ah, right. Only the other way around, when converting from LF to text.\n> \n> I've decided to use the \"disable only when the side that appears\n> postimage (taking --reverse option into account) is a symbolic link\"\n> rule.\n> \n> Strictly speaking, \"diff\" (but not \"apply\") has wsErrorHighlight\n> feature where it can be configured to complain about whitespace\n> glitches in both pre- and postimage, so it is technically not\n> sufficient, but it is not worth supporting diff.wsErrorHighlight\n> that is set to anything but \"new\" (or \"default\" which is its\n> synonym).\n\nSounds sensible.\n\n> > Eh, I didn't mean symrefs here, but symbolic links :) Tools like ln(1)\n> > seem to strip trailing newlines, but if you try hard enough you'll\n> > probably be able to create symlinks that have a target with trailing\n> > newline.\n> \n> Yes, as you can create a file whose name contains a newline, a name\n> that ends in a newline is a valid filename that \"ln -s\" may want to\n> support.  I am reasonably sure that we do not want to flag such a\n> symbolic link as whitespace damaged.\n\nYeah, we certainly don't want that. The remark was rather about a reader\nnot being able to discern those two cases (does or does not end in a\nnewline) anymore. Or would they?\n\nPatrick\n"}]}