From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 07 Apr 2013 23:48:10 GMT
Subject: Re: [PATCH 1/2] clone: Fix error message for reference repository
Message-ID: <20130407234810.GG19857@elie.Belkin>
In-Reply-To: <1365376629-16054-1-git-send-email-aaron@schrab.com>

Hi Aaron,

Aaron Schrab wrote:

> Do not report an argument to clone's --reference option is not a local
> directory.  Nothing checks for the actual directory so we have no way to
> know if whether or not exists.  Telling the user that a directory doesn't
> exist when that isn't actually known may lead him or her on the wrong
> path to finding the problem.

I don't understand the above explanation.  Could you give an example?

[...]
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
>  		free(ref_git);
>  		ref_git = ref_git_git;
>  	} else if (!is_directory(mkpath("%s/objects", ref_git)))
> -		die(_("reference repository '%s' is not a local directory."),
> +		die(_("reference repository '%s' is not a local repository."),

"is_directory" calls stat and checks if its target is a directory.  Is
the problem that "/path/to/repo.git" might be a directory but
"/path/to/repo.git/objects" may not?

Would it make sense for the message to say something like the
following?

	fatal: alternate object store '/path/to/repo.git/objects' is not a local directory

Thanks and hope that helps,
Jonathan