Re: includeIf breaks calling dashed externals

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Apr 14, 2017 at 01:43:37PM -0400, Jeff King wrote:
> On Fri, Apr 14, 2017 at 07:04:23PM +0200, Bert Wesarg wrote:
> 
> > Dear Duy,
> > 
> > heaving an includeIf in a git config file breaks calling external git
> > commands, most prominently git-gui.
> > 
> > $ git --version
> > git version 2.12.2.599.gcf11a6797
> > $ git rev-parse --is-inside-work-tree
> > true
> > $ git echo
> > git: 'echo' is not a git command. See 'git --help'.
> > 
> > Did you mean this?
> >     fetch
> > $ echo '[includeIf "gitdir:does-not-exists"]path = does-not-exists'
> > >>.git/config
> > $ git rev-parse --is-inside-work-tree
> > true
> > $ git echo
> > fatal: BUG: setup_git_env called without repository
> 
> Probably this fixes it:
> 
> diff --git a/config.c b/config.c
> index b6e4a57b9..8d66bdf56 100644
> --- a/config.c
> +++ b/config.c
> @@ -213,6 +213,9 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
>  	struct strbuf pattern = STRBUF_INIT;
>  	int ret = 0, prefix;
>  
> +	if (!have_git_dir())
> +		return 0;
> +
>  	strbuf_add_absolute_path(&text, get_git_dir());
>  	strbuf_add(&pattern, cond, cond_len);
>  	prefix = prepare_include_condition_pattern(&pattern);
> 
> But it does raise a question of reading config before/after repository
> setup, since those will give different answers. I guess they do anyway
> because of $GIT_DIR/config.

This happens in execv_dased_external() -> check_pager_config() ->
read_early_config(). We probably could use the same discover_git_directory
trick to get .git dir (because we should find it). Maybe something
like this instead?

diff --git a/config.c b/config.c
index 1a4d85537b..4f540ae578 100644
--- a/config.c
+++ b/config.c
@@ -212,8 +212,14 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
 	struct strbuf text = STRBUF_INIT;
 	struct strbuf pattern = STRBUF_INIT;
 	int ret = 0, prefix;
+	struct strbuf gitdir = STRBUF_INIT;
 
-	strbuf_add_absolute_path(&text, get_git_dir());
+	if (have_git_dir())
+		strbuf_addstr(&gitdir, get_git_dir());
+	else if (!discover_git_directory(&gitdir))
+		goto done;
+
+	strbuf_add_absolute_path(&text, gitdir.buf);
 	strbuf_add(&pattern, cond, cond_len);
 	prefix = prepare_include_condition_pattern(&pattern);
 
@@ -237,6 +243,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
 			 icase ? WM_CASEFOLD : 0, NULL);
 
 done:
+	strbuf_release(&gitdir);
 	strbuf_release(&pattern);
 	strbuf_release(&text);
 	return ret;

--
Duy