Re: [PATCH v2 6/7] correct blame for files commited with CRLF
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 1, 2016, 22:29 UTC
- Message-ID
- <xmqq60w1gdyr.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1459526911-19985-1-git-send-email-tboegi@web.de>
tboegi@web.de writes:
> Whenever a CLRF conversion is needed (or any filter us set), load the > index temporally, before calling convert_to_git()
I am not sure if this is needed. We should just do read_cache() unconditionally at the beginning of fake_working_tree_commit(), without even discarding it. Most Git commands that work on the working tree files follow that pattern, and I do not think the "optimization" this patch does over that simple pattern, while it may be nice, is an unnecessary complication.
By the way, thanks for pushing back in the [v1 6/7] discussion thread to show me where convert_to_git() was failing.
Show 81 quoted lines
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> builtin/blame.c | 6 +++++-
> convert.c | 10 ++++++++++
> convert.h | 1 +
> t/t8003-blame-corner-cases.sh | 14 ++++++++++++++
> 4 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index e982fb8..a219068 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2376,7 +2376,11 @@ 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);
> + if (convert_needs_conversion(path)) {
> + read_cache();
> + convert_to_git(path, buf.buf, buf.len, &buf, 0);
> + discard_cache();
> + }
> 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/convert.c b/convert.c
> index 4ed5d89..02c50da 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -903,6 +903,16 @@ const char *get_convert_attr_ascii(const char *path)
> return "";
> }
>
> +int convert_needs_conversion(const char *path)
> +{
> + struct conv_attrs ca;
> +
> + convert_attrs(&ca, path);
> + if (ca.drv || ca.ident || ca.crlf_action != CRLF_BINARY)
> + return 1;
> + return 0;
> +}
> +
> int convert_to_git(const char *path, const char *src, size_t len,
> struct strbuf *dst, enum safe_crlf checksafe)
> {
> diff --git a/convert.h b/convert.h
> index ccf436b..ffd9c32 100644
> --- a/convert.h
> +++ b/convert.h
> @@ -35,6 +35,7 @@ extern enum eol core_eol;
> extern const char *get_cached_convert_stats_ascii(const char *path);
> extern const char *get_wt_convert_stats_ascii(const char *path);
> extern const char *get_convert_attr_ascii(const char *path);
> +int convert_needs_conversion(const char *path);
>
> /* returns 1 if *dst was used */
> extern int convert_to_git(const char *path, const char *src, size_t len,
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index 6568429..a9b266f 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -212,4 +212,18 @@ test_expect_success 'blame file with CRLF attributes text' '
> grep "A U Thor" actual
> '
>
> +test_expect_success 'blame file with CRLF core.autocrlf=true' '
> + git config core.autocrlf false &&
> + printf "testcase\r\n" >crlfinrepo &&
> + >.gitattributes &&
> + git add crlfinrepo &&
> + git commit -m "add crlfinrepo" &&
> + git config core.autocrlf true &&
> + mv crlfinrepo tmp &&
> + git checkout crlfinrepo &&
> + rm tmp &&
> + git blame crlfinrepo >actual &&
> + grep "A U Thor" actual
> +'
> +
> test_done