Re: [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2014, 18:26 UTC
Message-ID: <xmqqmw9z2zi9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1410837850-5169-2-git-send-email-davvid@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> This patch now has the t1503 test case squashed into it.
> It was previously a separate patch, but it makes more sense
> for them to go together.

Yes.

Show 22 quoted lines

> diff --git a/refs.c b/refs.c
> index 2ce5d69..447e339 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3108,7 +3108,7 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
>  	return 1;
>  }
>  
> -int read_ref_at(const char *refname, unsigned long at_time, int cnt,
> +int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time, int cnt,
>  		unsigned char *sha1, char **msg,
>  		unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
>  {
> @@ -3126,8 +3126,12 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt,
>  
>  	for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb);
>  
> -	if (!cb.reccnt)
> -		die("Log for %s is empty.", refname);
> +	if (!cb.reccnt) {
> +		if (flags & GET_SHA1_QUIETLY)
> +			exit(1);

Do we want 1 or 128 just like die()?

> +		else
> +			die("Log for %s is empty.", refname);
> +	}

Given that I see this behaviour:

    $ git rev-parse da/rev-parse-verify-quiet@{1}
    2892dfeec3f98f7e65a2746d271471d2c3c4af57
    $ git rev-parse da/rev-parse-verify-quiet@{10}
    fatal: Log for 'da/rev-parse-verify-quiet' only has 4 entries

and I do not see a change in this patch to touch "only has %d entries" (found in sha1_name.c), I have to suspect that this change is not sufficient to say "rev-parse --quiet will now be quiet".

    $ git rev-parse --quiet --verify da/rev-parse-verify-quiet@{10.years.ago}

may want to return the oldest-known value as it has always done without giving a warning.

There probably are other cases that relate to reflogs, e.g.

    $ git rev-parse --quiet --verify @{-99999}

to ask the tip of the branch you were on 99999 "git checkout"s ago. I think the last one does not have to be in the same commit as this fix for "empty" and "you do not have that many" cases, though.

Thanks.