Re: [PATCH v3 5/5] setup: report when GIT_OBJECT_DIRECTORY is what we could not use

[Junio C Hamano <gitster@pobox.com>]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> Improve this error reporting UX by explaining the real cause wherever
> we are about to die. Because the check does not depend on the candidate
> that is given to is_git_directory(), it is enough to ask once, at the
> point of failure, whether the configured object directory is to blame.
> With this, the user would see the following:
>
>   ...
>   $ git status
>   fatal: not a git repository (or any parent up to mount point /)
>   Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
>   cannot access object directory '/tmp/git-objects' set via
>     $GIT_OBJECT_DIRECTORY: No such file or directory

Hmph, this is only a half of the real story isn't it?

    $ git status ;# to show I am in a healthy repository
    on branch master
    nothing to commit (use -u to show untracked files)

In such a healthy repository, if I have a bogus DB_ENVIRONMENT,
discovery does not have to fail.

    $ GIT_OBJECT_DIRECTORY=/ git status
    fatal: bad object HEAD

On the other hand, if I am truly in a bogus non-repository,
it does not matter where DB_ENVIRONMENT points at.

    $ (cd / && GIT_OBJECT_DIRECTORY=. git status)
    fatal: not a git repository (or any of the parent directories): .git

Even when DB_ENVIRONMENT points at a valid object directory

    $ (cd / && GIT_OBJECT_DIRECTORY=$HOME/w/git.git/.git/objects git status)
    fatal: not a git repository (or any of the parent directories): .git

the command fails because it cannot find a valid repository.

> +static int describe_unusable_object_dir(struct strbuf *out)
> +{
> +	const char *objdir = getenv(DB_ENVIRONMENT);
> +	int saved_errno;
> +
> +	if (!objdir || !access(objdir, X_OK))
> +		return 0;
> +	saved_errno = errno;
> +
> +	/*
> +	 * TRANSLATORS: The first %s is a path, the second %s is the
> +	 * name of the environment variable that set it, and the third
> +	 * %s is the system error message.
> +	 */
> +	strbuf_addf(out, _("cannot access object directory '%s' set via $%s: %s"),
> +		    objdir, DB_ENVIRONMENT, strerror(saved_errno));
> +	return 1;
> +}

As this is called ONLY when we cannot figure out a usable GIT_DIR,
we do not have to worry about a case where $GIT_OBJECT_DIRECTORY is
not set and $GIT_DIR/objects/ is not readable.  So I do not think
this additional message hurts much, but I am not sure how much it
would help.  It would make the message more confusing in the "no
valid repository, DB_ENVIRONMENT pointing at a healthy object
directory" situation in the above example.