Re: [PATCH v2 2/5] odb/source: introduce error status when reading objects

On Thu, Aug 20, 2026 at 08:41:10AM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <[email protected]> writes:
> 
> > 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.
> >
> 
> But isn't that the same for an object not existing in a source? If it
> doesn't exist in one source, we may find a good copy of it in another?

Yeah, that paragraph is a bit odd indeed. What I really wanted to say is
that the failure mode is different depending on whether the object is
found at all: if it's not then we'd fail gracefully, if it is but it's
corrupt then we die.

> > diff --git a/odb.c b/odb.c
> > index caf1d0f542..1b37b26376 100644
> > --- a/odb.c
> > +++ b/odb.c
> > @@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,
> >  	return ret;
> >  }
> >
> 
> Here and elsewhere. Shouldn't we explicitly return ODB_READ_OK or
> ODB_READ_ERROR instead of relying on implicit conversion?

I didn't want to go through the complete callchain to make sure that we
explicitly return those values. I think it'd be mostly pointless: the
return code convention is established enough, and all callers already
return the expected values anyway, even though they're not using the
enum now.

Patrick