From: Junio C Hamano <gitster@pobox•com>
To: "Torsten Bögershausen" <tboegi@web•de>
Cc: git@vger•kernel.org, johannes.schindelin@gmx•de, kasal@ucw•cz
Subject: Re: [PATCH v4] blame: CRLF in the working tree and LF in the repo
Date: Fri, 01 May 2015 10:13:56 -0700 [thread overview]
Message-ID: <xmqqbni4xlt7.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <55436852.3030002@web.de> ("Torsten Bögershausen"'s message of "Fri, 01 May 2015 13:49:38 +0200")
Torsten Bögershausen <tboegi@web•de> writes:
> A typical setup under Windows:
> core.eol is CRLF and a file is marked as "text" in .gitattributes,
> or core.autocrlf is true
A full sentence (a proper prose) is easier to read. The above is
unclear if that is what you are unilaterally declaring, a statement
of fact, or something else (see what I queued based on the previous
round on 'pu').
> After 4d4813a5 "git blame" no longer works as expected,
> every line is annotated as "Not Committed Yet",
> even though the working directory is clean.
>
> commit 4d4813a5 removed the conversion in blame.c for all files,
> with or without CRLF in the repo.
>
> Having files with CRLF in the repo and core.autocrlf=input is a temporary
> situation, the files should be normalized in the repo.
> Blaming them with "Not Committed Yet" is OK.
>
> The solution is to revert commit 4d4813a5.
Do these two (or three) patches separately:
- One patch to revert it and do nothing else other than justify the
revert in the log message; and
- Another patch that demonstrates failures Kasal found so that we
won't repeat the mistake.
- Optionally, yet another patch that demonstrates the current
breakage that exists with or without the reversion.
The former two may be combined into one, but the last one should be
a separate patch, as it does not have anything to do with this
reversion.
In other words, there shouldn't be a new test-expect-failure in
a revert of an existing commit (unless that commit removed that
test-expect-failure in the first place, claiming that it fixed
something, that is). If you are documenting a failure that has
not been tested in our test suite, do so in a separate patch.
Thanks.
> Reported-By: Stepan Kasal <kasal@ucw•cz>
> Signed-off-by: Torsten Bögershausen <tboegi@web•de>
> ---
> Changes since V3:
> more test cases
> Sorry for the spam
>
> builtin/blame.c | 1 +
> t/t8003-blame-corner-cases.sh | 48 +++++++++++++++++++++++++++++++++++++------
> 2 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 06484c2..8d70623 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2348,6 +2348,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
> if (strbuf_read(&buf, 0, 0) < 0)
> die_errno("failed to read from stdin");
> }
> + convert_to_git(path, buf.buf, buf.len, &buf, 0);
> origin->file.ptr = buf.buf;
> origin->file.size = buf.len;
> pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index 32895e5..8af36a6 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -191,13 +191,49 @@ test_expect_success 'indent of line numbers, ten lines' '
> test $(grep -c " " actual) = 9
> '
>
> -test_expect_success 'blaming files with CRLF newlines' '
> - git config core.autocrlf false &&
> +test_expect_success 'setup crlf files' '
> printf "testcase\r\n" >crlffile &&
> - git add crlffile &&
> - git commit -m testcase &&
> - git -c core.autocrlf=input blame crlffile >actual &&
> - grep "A U Thor" actual
> + printf "testcase\n" >lffile &&
> + git -c core.autocrlf=false add lffile crlffile &&
> + git commit -m "add files" &&
> + git -c core.autocrlf=false blame HEAD -- crlffile >crlfclean.txt
> + printf "testcase\r\n" >crlffile &&
> + git -c core.autocrlf=false blame HEAD -- lffile >lfclean.txt
> + printf "testcase\r\n" >lffile
> + #Keep lffile with CRLF in worktree
> +'
> +
> +test_expect_success 'blame file with CRLF in repo core.autocrlf=false' '
> + git -c core.autocrlf=false blame crlffile >crlf_repo_false &&
> + test_cmp crlfclean.txt crlf_repo_false
> +'
> +
> +#has_cr_in_index() should suppress the normalization, see convert.c
> +#but read_blob_data_from_cache() returns NULL
> +test_expect_failure 'blame file with CRLF in repo core.autocrlf=true' '
> + git -c core.autocrlf=true blame crlffile >crlf_repo_true &&
> + test_cmp crlfclean.txt crlf_repo_true
> +'
> +
> +test_expect_success 'blame file with CRLF in WS core.autocrlf=true' '
> + git -c core.autocrlf=true blame lffile >lf_repo_true &&
> + test_cmp lfclean.txt lf_repo_true
> +'
> +
> +test_expect_success 'blame file with CRLF in WS core.autocrlf=input' '
> + git -c core.autocrlf=input blame lffile >lf_repo_input &&
> + test_cmp lfclean.txt lf_repo_input
> +'
> +
> +test_expect_success 'blame file with CRLF in WS core.autocrlf=false' '
> + git -c core.autocrlf=false blame lffile >lf_repo_false &&
> + grep "Not Committed Yet" lf_repo_false
> +'
> +
> +test_expect_success 'blame file with CRLF in WS core.autocrlf=false attributes' '
> + printf "lffile text\r\n" >.gitattributes &&
> + git -c core.autocrlf=false blame lffile >lf_repo_attr_text &&
> + test_cmp lfclean.txt lf_repo_attr_text
> '
>
> test_done
next prev parent reply other threads:[~2015-05-01 17:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-01 11:49 [PATCH v4] blame: CRLF in the working tree and LF in the repo Torsten Bögershausen
2015-05-01 17:13 ` Junio C Hamano [this message]
2015-05-01 18:57 ` Torsten Bögershausen
2015-05-03 2:02 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbni4xlt7.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=johannes.schindelin@gmx$(echo .)de \
--cc=kasal@ucw$(echo .)cz \
--cc=tboegi@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox