Re: includeIf breaks calling dashed externals
On Sat, Apr 15, 2017 at 06:49:01PM +0700, Duy Nguyen wrote: > > 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? I know that we only kick in discover_git_directory() in certain cases when we're reading config (for the "early config"). Would it makes sense to respect the same rules here? I.e., if we have a program that wants to look at config but explicitly _doesn't_ setup the git repository, is it right to say that we are inside that repository? So instead of lazily doing the discovery here: > 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; ..should we record the discovered git dir in read_early_config(), and use it only if available? And if not, then we know the program is explicitly trying not to be in a repo (or we know we tried to discover one already and it didn't exist). -Peff
Re: includeIf breaks calling dashed externals
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
Re: includeIf breaks calling dashed externals
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. -Peff
includeIf breaks calling dashed externals
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 Best, Bert
