Re: [PATCH 2/2] read_info_alternates: warn on non-trivial errors

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sep 19, 2017, 02:46 UTC
Message-ID: <20170919024623.GD175206@aiede.mtv.corp.google.com>
In-Reply-To: <20170918155519.llmetd2ylebzokrl@sigill.intra.peff.net>

Jeff King wrote:

Show 10 quoted lines

> When we fail to open $GIT_DIR/info/alternates, we silently
> assume there are no alternates. This is the right thing to
> do for ENOENT, but not for other errors.
>
> A hard error is probably overkill here. If we fail to read
> an alternates file then either we'll complete our operation
> anyway, or we'll fail to find some needed object. Either
> way, a warning is good idea. And we already have a helper
> function to handle this pattern; let's just call
> warn_on_fopen_error().

I think I prefer a hard error. What kind of cases are you imagining where it would be better to warn?

E.g. for EIO, erroring out so that the user can try again seems better than hoping that the application will be able to cope with the more subtle error that comes from discovering some objects are missing.

For EACCES, I can see how it makes sense to warn and move on, but no other errors like that are occuring to me.

Thoughts?

Thanks, Jonathan

Show 5 quoted lines

> Note that technically the errno from strbuf_read_file()
> might be from a read() error, not open(). But since read()
> would never return ENOENT or ENOTDIR, and since it produces
> a generic "unable to access" error, it's suitable for
> handling errors from either.