Re: [PATCH] setup.c: Improve 'Not a git repository' messages

[Jonathan Nieder <jrnieder@gmail.com>]

Hi Ralf,

Ralf Ebert wrote:

> +++ b/setup.c
> @@ -431,7 +431,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  				*nongit_ok = 1;
>  				return NULL;
>  			}
> -			die("Not a git repository (or any of the parent directories): %s", DEFAULT_GIT_DIR_ENVIRONMENT);
> +			die("No %s repository in %s or its parent directories",
> +				DEFAULT_GIT_DIR_ENVIRONMENT, cwd);

So, before:

 fatal: Not a git repository (or any of the parent directories): .git

and after:

 fatal: No .git repository in /home/jrn/src/some/deeply/nested/path/that/goes/on/for/a/while or its parent directories

The idea being to give a hint to the confused user.  That sort of
makes sense, but I would (warning! nitpicks coming!) prefer something
along the lines of

 fatal: not a git repository: /home/jrn/src/some/deeply/nested/pa
 hint: This means every candidate metadata (.git) directory considered
 hint: was invalid in some way.
 hint: See "git help repository-layout" for more information.

In other words:

 - first, a straightforward error message;
 - cutting off user-specified data at some reasonable length
   (e.g. 50 chars);
 - avoiding overly specific (false) advice and deferring to
   documentation for the details.

> @@ -445,9 +446,14 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  					*nongit_ok = 1;
>  					return NULL;
>  				}
> -				cwd[offset] = '\0';
> -				die("Not a git repository (or any parent up to mount parent %s)\n"
> -				"Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).", cwd);
> +				const char *home = getenv("HOME");
> +				if (home == NULL || strncmp(home, cwd, offset) != 0) {
> +					warning("Stopped searching for %s at %.*s "
> +						"as GIT_DISCOVERY_ACROSS_FILESYSTEM is not set",
> +						DEFAULT_GIT_DIR_ENVIRONMENT, offset, cwd);
> +				}
> +				die("No %s repository in %s or its parent directories",
> +					DEFAULT_GIT_DIR_ENVIRONMENT, cwd);

I don’t understand what’s special about $HOME here.

Would something like the following be okay?

 fatal: not a git repository: /home/jrn/src/some/deeply/nested/pa
 info: search stopped at mount parent /home
 hint: This means every candidate metadata (.git) directory considered
 hint: was invalid in some way.
 hint: See "git help repository-layout" for more information.

I actually suspect overriding with $GIT_DISCOVERY_ACROSS_FILESYSTEM
would be kind of rare, so it might be okay to let the documentation
take care of explaining that (provided we include a clear pointer).

Just my two cents.  Feel free to ignore. :)