Re: [PATCH v4 13/15] Reftable support for git-core

> On Thu, Jan 21, 2021 at 5:14 PM Han-Wen Nienhuys <hanwen@google.com> wrote:
>>
>> On Thu, Jan 21, 2021 at 4:55 PM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>> > I must say I do find it a bit more concerning in the case of the
>> > reftable series. I.e. the git_reftable_read_raw_ref() function seems
>> > like a faithful copy of the general logic (at least this part) of the
>> > refs_resolve_ref_unsafe() function in refs.c.
>> >
>> > That I can remove error checking/handling from this place means existing
>> > general logic was faithfully copied without checking that we have a test
>> > for it, or adding one. Or perhaps given the history of this codebase
>> > there's out-of-tree tests for it somewhere?
>>
>> One of the earlier iterations of this code didn't have the errno
>> handling, and it caused a bug that I tracked down to the errno
>> handling. I'm fairly sure of this, because I remember being perplexed
>> and flabbergasted at using errno as an out-of-band API mechanism. The
>> first iteration of reftable code was about a year ago, so if you
>> checkout the Git source code from that time, and remove the errno
>> assignments, you would be able to see which test exercised this code
>> path.
>>
>> Or you could check where errno is read-back (I believe ENOENT is the
>> case that has special meaning)
>
> In refs.c, there is behavior that depends on errno,
>
> if (refs_read_raw_ref(refs, refname,
>       oid, &sb_refname, &read_flags)) {
> ..
> /*
> * Otherwise a missing ref is OK. But the files backend
> * may show errors besides ENOENT if there are
> * similarly-named refs.
> */
> if (errno != ENOENT &&
>     errno != EISDIR &&
>     errno != ENOTDIR)
> return NULL;
>
> blame tells me it has something to do with dir/file conflicts and
> commit a1c1d8170db.