Re: [PATCH v4] blame: CRLF in the working tree and LF in the repo
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 1, 2015, 17:13 UTC
- Message-ID
- <xmqqbni4xlt7.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <55436852.3030002@web.de>
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').
Show 12 quoted lines
> 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.
Show 83 quoted lines
> 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