whitespace: symbolic links usually lack LF at the end

For a patch that touches a symbolic link, it is perfectly normal
that the contents 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.  For "git apply", we check only when the
postimage is a symbolic link regardless of the preimage, and we only
care about preimage when applying in reverse.  Similarly, "git diff"
would warn only when the postimage is a symbolic link, or the
preimage when running "git diff -R".

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/apply.c b/apply.c
index c9fb45247d8cd24ed5b9fb8cf183d887169e7368..f01204d15b4d13ef318c08e0c6b44b9ff79664e4 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -1725,6 +1725,26 @@ static int parse_fragment(struct apply_state *state,
        unsigned long oldlines, newlines;
        unsigned long leading, trailing;
 
+       /* do not complain a symbolic link being an incomplete line */
+       if (patch->ws_rule & WS_INCOMPLETE_LINE) {
+               /*
+                * We want to figure out if the postimage is a
+                * symbolic link when applying the patch normally, or
+                * if the preimage is a symbolic link when applying
+                * the patch in reverse.  A normal patch only has
+                * old_mode without new_mode.  If it changes the
+                * filemode, new_mode has value, which is different
+                * from old_mode.
+                */
+               unsigned mode = (state->apply_in_reverse
+                                ? patch->old_mode
+                                : patch->new_mode
+                                ? patch->new_mode
+                                : patch->old_mode);
+               if (mode && S_ISLNK(mode))
+                       patch->ws_rule &= ~WS_INCOMPLETE_LINE;
+       }
+
        offset = parse_fragment_header(line, len, fragment);
        if (offset < 0)
                return -1;

diff --git a/diff.c b/diff.c
index 7b7cd50dc24351312ff186b10b5fbe635fab252e..9e4b92ed69f05d7f19449a3544ee1608a33b21f6 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1834,6 +1834,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;
 
@@ -1856,9 +1857,15 @@ 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);
+
+       /* symlink being an incomplete line is not a news */
+       if (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;
@@ -3762,6 +3769,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) {
@@ -3773,6 +3781,12 @@ 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);
+
+               /* symlink being an incomplete line is not a news */
+               if (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);
@@ -3784,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;
@@ -3991,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);
 
+       /* symlink being an incomplete line is not a news */
+       if (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 3c8eb02e4f3e640fd4ce0f226dfe5be2e25feacb..b691d2947943c8877bc7b495aad73b4a14bf74de 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -90,6 +90,32 @@ 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" &&
+
+       # a regular file with an incomplete line
+       printf "%s" one >mylink &&
+       git add mylink &&
+
+       # a symbolic link
+       rm mylink &&
+       ln -s two mylink &&
+
+       git -c diff.color=always -c core.whitespace=incomplete \
+               diff mylink >forward.raw &&
+       test_decode_color >forward <forward.raw &&
+       test_grep ! "<BRED>\\\\ No newline at end of file<RESET>" forward &&
+
+       git -c diff.color=always -c core.whitespace=incomplete \
+               diff -R mylink >reverse.raw &&
+       test_decode_color >reverse <reverse.raw &&
+       test_grep "<BRED>\\\\ No newline at end of file<RESET>" reverse &&
+
+       git -c core.whitespace=incomplete diff --check mylink &&
+       test_must_fail git -c core.whitespace=incomplete diff --check -R 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 115a0f857906d4d0495fbe03ca7d315661176d37..29ea7d4268eca4acca404e0e5173d2ef5f097e09 100755 (executable)
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -743,4 +743,90 @@ 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) &&
+
+       oneshort=$(git rev-parse --short $oneblob) &&
+       twoshort=$(git rev-parse --short $twoblob) &&
+
+       cat >patch0.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
+
+       # the index has the preimage symlink
+       git update-index --add --cacheinfo "120000,$oneblob,mylink" &&
+
+       # check the patch going forward and reverse
+       git -c core.whitespace=incomplete apply --cached --check \
+               --whitespace=error patch0.txt &&
+
+       git update-index --add --cacheinfo "120000,$twoblob,mylink" &&
+       git -c core.whitespace=incomplete apply --cached --check \
+               --whitespace=error -R patch0.txt &&
+
+       # the patch turns it into the postimage symlink
+       git update-index --add --cacheinfo "120000,$oneblob,mylink" &&
+       git -c core.whitespace=incomplete apply --cached --whitespace=error \
+               patch0.txt &&
+
+       # and then back.
+       git -c core.whitespace=incomplete apply --cached -R --whitespace=error \
+               patch0.txt &&
+
+       # a text file turns into a symlink
+       cat >patch1.txt <<-EOF &&
+       diff --git a/mylink b/mylink
+       deleted file mode 100644
+       index $oneshort..0000000
+       --- a/mylink
+       +++ /dev/null
+       @@ -1 +0,0 @@
+       -one
+       \ No newline at end of file
+       diff --git a/mylink b/mylink
+       new file mode 120000
+       index 0000000..$twoshort
+       --- /dev/null
+       +++ b/mylink
+       @@ -0,0 +1 @@
+       +two
+       \ No newline at end of file
+       EOF
+
+       # the index has the preimage text
+       git update-index --cacheinfo "100644,$oneblob,mylink" &&
+
+       # check
+       git -c core.whitespace=incomplete apply --cached \
+               --check --whitespace=error patch1.txt &&
+
+       # reverse, leaving an incomplete text file, should error
+       git update-index --cacheinfo "120000,$twoblob,mylink" &&
+       test_must_fail git -c core.whitespace=incomplete \
+               apply --cached --check --whitespace=error -R patch1.txt &&
+
+       # apply to create a symbolic link
+       git update-index --cacheinfo "100644,$oneblob,mylink" &&
+       git -c core.whitespace=incomplete apply --cached --whitespace=error \
+               patch1.txt &&
+
+       # turning it back into an incomplete text file is an error
+       test_must_fail git -c core.whitespace=incomplete \
+               apply --cached --whitespace=error -R patch1.txt
+
+
+
+'
+
 test_done