Re: [PATCH 1/7] odb/source: discern missing and corrupt objects

On Tue, Aug 18, 2026 at 11:00:40AM -0700, Junio C Hamano wrote:

Patrick Steinhardt [off-list ref] writes:

quoted

The `read_object_info()` callback of `struct odb_source` is documented
to return a negative error code in case reading the object has failed,
and zero otherwise. This is overly broad though, as there are two very
different kinds of failures:

  - The object may not exist in the source at all.

  - The object exists, but reading it has failed, for example because
    its on-disk state is corrupt.

This distinction matters to callers: when an object is corrupt in one
source we may still find a good copy of it in another source, so we may
still be able to proceed with a given operation.

The "packed" source already distinguishes these cases by returning a
positive value for missing objects and a negative value in case reading
the object has failed. But all the other sources conflate them into a
single negative return value.

In other words, "packed" did not honor the documented contract with
the callers and nobody noticed?  It gives us a usable escape hatch ;-)

Yes, kind of. It didn't matter much though, as the "files" backend
knew to translate the positive value into a negative one.

Do we need to support many other "it is an error but we treat as non
error in some context" values, like the "does not exist"?  If so, it
does make sense to say 0 is absolute success, positive values are
such half-errors, and negative values are absolute failures.  If
not, it would have been much nicer if "you asked me about this
information but there is no such object" were still signalled as an
error (i.e., negative return value) that is distinct from other
kinds of errors like I/O error (which also should be signalled by a
negative return value), instead of a positive value whose meanings
were not defined, though.

I cannot think of any other classes of errors where we'd want to fail
gracefully from the top of my head. The only one that's potentially
worth thinking about is in case an object disappears right while we are
looking at it. But that's basically just another edge case of a missing
object.

In any case, I think I'm aligned with the proposal to turn this into a
proper enum and then use negative values exclusively. Thanks!

Patrick