Re: [PATCH] Always check the return value of `repo_read_object_file()`

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2024, 16:37 UTC
Message-ID: <xmqqjzndk302.fsf@gitster.g>
In-Reply-To: <5fd95ae0-cd50-ddc4-5095-ee953c2640b3@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 6 quoted lines

> It does change behavior. The previous behavior looked up the note OID,
> then tried to read it, and if it was missing just pretended that there had
> not been a note.
>
> I'm not quite sure whether we should keep that behavior, as it is unclear
> in which scenarios it would be desirable to paper over missing objects.

Yeah, an object that does not have any notes attached to is a norm so the calling application must be prepared for it, but this codepath is different. The notes tree says it has notes, we try to read it and it is not there---at least we noticed an inconsistent notes tree (and object store), and if we were to run "git fsck" at that point, we would certainly complain about a missing blob object (can a tree object at an intermediate level be missing and would we notice, by the way, I wonder). It is only prudent to report it, instead of pretending that the notes are not there.

So I think this tightening falls into the "bugfix" category.

Thanks.