Re: [PATCH v3 1/2] diff --no-index: optionally follow symlinks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 19, 2017, 22:14 UTC
- Message-ID
- <xmqqk27kzzfm.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170318210038.22638-2-dennis@kaarsemaker.net>
Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
Show 20 quoted lines
> diff --git a/diff.c b/diff.c
> index be11e4ef2b..2afecfb939 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
> s->size = xsize_t(st.st_size);
> if (!s->size)
> goto empty;
> - if (S_ISLNK(st.st_mode)) {
> + if (S_ISLNK(s->mode)) {
> struct strbuf sb = STRBUF_INIT;
>
> if (strbuf_readlink(&sb, s->path, s->size))
> @@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
> s->should_free = 1;
> return 0;
> }
> + if (S_ISLNK(st.st_mode)) {
> + stat(s->path, &st);
> + s->size = xsize_t(st.st_size);Doesn't this affect --no-index mode? We never need to do a wasteful stat() after lstat() and we are penalizing the normal codepath with this change, no?
Show 11 quoted lines
> @@ -3884,7 +3888,11 @@ int diff_opt_parse(struct diff_options *options,
> else if (!strcmp(arg, "--no-follow")) {
> DIFF_OPT_CLR(options, FOLLOW_RENAMES);
> DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
> - } else if (!strcmp(arg, "--color"))
> + } else if (!strcmp(arg, "--dereference"))
> + DIFF_OPT_SET(options, DEREFERENCE);
> + else if (!strcmp(arg, "--no-dereference"))
> + DIFF_OPT_CLR(options, DEREFERENCE);
> + else if (!strcmp(arg, "--color"))
> options->use_color = 1;Also shouldn't be some code to detect --[no-]dereference options given when --no-index is not in effect and error out? As the patch title says, this change should be a no-op for normal codepath and only affect the no-index hack.