Re: [RFC] fetch: support hideRefs to speed up connectivity checks

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2023, 19:24 UTC
Message-ID: <xmqqv8k8dmz0.fsf@gitster.g>
In-Reply-To: <20230211075333.M933000@dcvr>

Eric Wong <e@80x24.org> writes:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> wrote:
>> I however notice error handling in the codepath that deals with
>> "--exclude-hidden" is  a bit sloppy.
>> 
>> refs.c::parse_hide_refs_config() is nice enough to diagnose a
>> malformed transfer.hiderefs configuration as an error by returning
>> -1, and revision.c::hide_refs_config() propagates such an error up,
>> but revision.c::exclude_hidden_refs() ignores the error from
>> git_config(), and revision.c::handle_revision_pseudo_opt() ignores
>> any error from exclude_hidden_refs() anyway.
>
> Not sure I follow.  exclude_hidden_refs() either dies or calls
> git_config().  git_config() calls repo_config(), then
> configset_iter().  configset_iter() will git_die_config_linenr()
> if `fn' (hide_refs_config() in this case) returns < 0.

Somehow I had this wishful thinking that the return value from git_config() can be checked and the caller can handle the error more gracefully, but its return type is void. We'll die when we see a bad configuration but only when we see "--exclude-hidden", which is when we need a valid value from there. That is how it should work, so I am now happier.

Thanks.