Jeff King <peff@xxxxxxxx> writes:
> When we try to open a loose object file, we first attempt to
> open in the local object database, and then try any
> alternates. This means that the errno value when we return
> will be from the last place we looked (and due to the way
> the code is structured, simply ENOENT if we do not have have
> any alternates).
>
> This can cause confusing error messages, as read_sha1_file
> checks for ENOENT when reporting a missing object. If errno
> is something else, we report that. If it is ENOENT, but
> has_loose_object reports that we have it, then we claim the
> object is corrupted. For example:
>
> $ chmod 0 .git/objects/??/*
> $ git rev-list --all
> fatal: loose object b2d6fab18b92d49eac46dc3c5a0bcafabda20131 (stored in .git/objects/b2/d6fab18b92d49eac46dc3c5a0bcafabda20131) is corrupt
Hmmmmmmmm. So we keep track of a more interesting errno we get from
some other place than what we get for this local loose object, and
we no longer give this message pointing at the local loose
object---is that the idea?
What I am wondering is that this report we give in the new code
> $ git rev-list --all
> fatal: failed to read object b2d6fab18b92d49eac46dc3c5a0bcafabda20131: Permission denied
may want to say which of the various possible places we saw this
most interesting errno, which I think was the original motivation
came from e8b15e61 (sha1_file: Show the the type and path to corrupt
objects, 2010-06-10) that added "(stored in ...)".
But that may involve a larger surgery, and I definitely do not want
to add unnecessary logic in the common-case codepath to keep track
of pieces of information that are only used in the error codepath,
so it smells like that this is the best fix to the issue the commit
message describes.
Thanks.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> sha1_file.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 3e9f55f..34d527f 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1437,19 +1437,23 @@ static int open_sha1_file(const unsigned char *sha1)
> {
> int fd;
> struct alternate_object_database *alt;
> + int most_interesting_errno;
>
> fd = git_open_noatime(sha1_file_name(sha1));
> if (fd >= 0)
> return fd;
> + most_interesting_errno = errno;
>
> prepare_alt_odb();
> - errno = ENOENT;
> for (alt = alt_odb_list; alt; alt = alt->next) {
> fill_sha1_path(alt->name, sha1);
> fd = git_open_noatime(alt->base);
> if (fd >= 0)
> return fd;
> + if (most_interesting_errno == ENOENT)
> + most_interesting_errno = errno;
> }
> + errno = most_interesting_errno;
> return -1;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html