[PATCH] git-completion.bash: prevent 'git help' from searching for git repository

[PATCH] git-completion.bash: prevent 'git help' from searching for git repository

[Gerrit Pape]

On 'git <TAB><TAB>' the bash completion runs 'git help -a'.  Since
'git help' actually doesn't need to be run inside a git repository,
this commit uses the option --git-dir=/nonexistent to prevent it
from searching a git directory.  Unnecessary searching for a git
directory can be annoying in auto-mount environments.

The annoying behavior and suggested fix has been reported by Vincent
Danjean through
 http://bugs.debian.org/539273

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf688e1..d51854a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -500,7 +500,7 @@ __git_all_commands ()
 		return
 	fi
 	local i IFS=" "$'\n'
-	for i in $(git help -a|egrep '^ ')
+	for i in $(git --git-dir=/nonexistent help -a|egrep '^ ')
 	do
 		case $i in
 		*--*)             : helper pattern;;
-- 
1.6.0.3

Re: [PATCH] git-completion.bash: prevent 'git help' from searching for git repository

[Sverre Rabbelier]

Heya,

On Wed, Sep 2, 2009 at 11:58, Gerrit Pape<pape@smarden.org> wrote:
> +       for i in $(git --git-dir=/nonexistent help -a|egrep '^ ')

Wouldn't implementing "git --no-git-dir" be more appropriate?

-- 
Cheers,

Sverre Rabbelier

[PATCH 1/2] git: add new option --no-git-dir

[Gerrit Pape]

This commit adds the --no-git-dir option to the git program.  Setting
this option prevents the git program from searching for a path to a git
repository, which can be useful for commands that do not require one.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 Documentation/git.txt |    6 +++++-
 git.c                 |    6 +++++-
 setup.c               |    2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index ad44cac..6327203 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path]
     [-p|--paginate|--no-pager]
-    [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
+    [--bare] [--git-dir=GIT_DIR|--no-git-dir] [--work-tree=GIT_WORK_TREE]
     [--help] COMMAND [ARGS]
 
 DESCRIPTION
@@ -212,6 +212,10 @@ help ...`.
 	setting the GIT_DIR environment variable. It can be an absolute
 	path or relative path to current working directory.
 
+--no-git-dir::
+	Do not set a path to a repository, and do not try to find one.
+	Setting this option is equivalent to setting --git-dir="".
+
 --work-tree=<path>::
 	Set the path to the working tree.  The value will not be
 	used in combination with repositories found automatically in
diff --git a/git.c b/git.c
index 0b22595..8e060b9 100644
--- a/git.c
+++ b/git.c
@@ -5,7 +5,7 @@
 #include "run-command.h"
 
 const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR|--no-git-dir] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
 const char git_more_info_string[] =
 	"See 'git help COMMAND' for more information on a specific command.";
@@ -99,6 +99,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--no-git-dir")) {
+			setenv(GIT_DIR_ENVIRONMENT, "", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else if (!strcmp(cmd, "--work-tree")) {
 			if (*argc < 2) {
 				fprintf(stderr, "No directory given for --work-tree.\n" );
diff --git a/setup.c b/setup.c
index e3781b6..ee9be6e 100644
--- a/setup.c
+++ b/setup.c
@@ -335,6 +335,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			*nongit_ok = 1;
 			return NULL;
 		}
+		if (!*gitdirenv)
+			die("This command requires a git repository");
 		die("Not a git repository: '%s'", gitdirenv);
 	}
 
-- 
1.6.0.3

[PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Gerrit Pape]

On 'git <TAB><TAB>' the bash completion runs 'git help -a'.  Since 'git
help' actually doesn't need to be run inside a git repository, this
commit uses the --no-git-dir option to prevent it from searching a git
directory.  Unnecessary searching for a git directory can be annoying in
auto-mount environments.

The annoying behavior and suggested fix has been reported by Vincent
Danjean through
 http://bugs.debian.org/539273

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf688e1..a55e3cd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -500,7 +500,7 @@ __git_all_commands ()
 		return
 	fi
 	local i IFS=" "$'\n'
-	for i in $(git help -a|egrep '^ ')
+	for i in $(git --no-git-dir help -a|egrep '^ ')
 	do
 		case $i in
 		*--*)             : helper pattern;;
-- 
1.6.0.3

Re: [PATCH] git-completion.bash: prevent 'git help' from searching for git repository

[Rogan Dawes]

Sverre Rabbelier wrote:
> Heya,
> 
> On Wed, Sep 2, 2009 at 11:58, Gerrit Pape<pape@smarden.org> wrote:
>> +       for i in $(git --git-dir=/nonexistent help -a|egrep '^ ')
> 
> Wouldn't implementing "git --no-git-dir" be more appropriate?
> 

Or documenting which git commands do/don't require a git dir at all?

I assume that documenting those that don't would be better than
documenting those that do . . .

And by documenting, I mean in the code, so that the code can DTRT.

Otherwise, having this switch lets people shoot themselves in the foot,
I'd think.

Rogan

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Junio C Hamano]

Gerrit Pape <pape@smarden.org> writes:

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index bf688e1..a55e3cd 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -500,7 +500,7 @@ __git_all_commands ()
>  		return
>  	fi
>  	local i IFS=" "$'\n'
> -	for i in $(git help -a|egrep '^ ')
> +	for i in $(git --no-git-dir help -a|egrep '^ ')

Thanks.

What the --no-git-dir option actually does is "pretend that cwd is the git
directory but do not worry if it is not", which is different from "there
is no git directory, so do not barf as long as you do not need to access
git-dir".  The latter is what the name implies, and also additionally it
implies "but please do barf if you ever need to access something from git
directory."  I do not know if that holds true with your Patch 1/2, and I
am a bit too tired to check.

Besides, "git --git-dir=." is shorter to type, and it is equally magical
that the user has to know about it anyway.  Hopefully, most of the time
the end user would not have to use it directly, and the only demonstrated
use case is here in this completion script.

Would it be an option to chuck the Patch 1/2 at least for now and instead
say "git --git-dir=. help -a" here in this patch?

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Johannes Schindelin]

Hi,

On Fri, 4 Sep 2009, Gerrit Pape wrote:

> On 'git <TAB><TAB>' the bash completion runs 'git help -a'.

Correct me if I am wrong, but does "git help -a" not list aliases?  If it 
does, "git help" must search for the Git repository.

If it does not, then "git help" needs fixing, not the completions.  I.e. 
something like this:

-- snipsnap --
[PATCH] git help -a: do not look for a repository

<all the acknowledgements go here>

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 builtin-help.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-help.c b/builtin-help.c
index e1eba77..719aa23 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -416,9 +416,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	const char *alias;
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	setup_git_directory_gently(&nongit);
-	git_config(git_help_config, NULL);
-
 	argc = parse_options(argc, argv, prefix, builtin_help_options,
 			builtin_help_usage, 0);
 
@@ -429,6 +426,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		return 0;
 	}
 
+	setup_git_directory_gently(&nongit);
+	git_config(git_help_config, NULL);
+
 	if (!argv[0]) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();

Re: [PATCH] git-completion.bash: prevent 'git help' from searching for git repository

[Johannes Schindelin]

Hi,

On Fri, 4 Sep 2009, Rogan Dawes wrote:

> Sverre Rabbelier wrote:
> 
> > On Wed, Sep 2, 2009 at 11:58, Gerrit Pape<pape@smarden.org> wrote:
> >> +       for i in $(git --git-dir=/nonexistent help -a|egrep '^ ')
> > 
> > Wouldn't implementing "git --no-git-dir" be more appropriate?
> 
> Or documenting which git commands do/don't require a git dir at all?

This patch is not about documentation, but about preventing the 
auto-completion from trying to discover a Git repository (to prevent 
auto-mounting; although I wonder why you would run Git there if you do not 
want to auto-mount).

> I assume that documenting those that don't would be better than 
> documenting those that do . . .

It's not as easy as that: some commands, such as "ls-remote" do _not_ 
require one, but they take it into account (think "git ls-remote origin").  
Other commands, such as "archive", have modes in which they _need_ a 
repository, and other modes where they do not even look for one.

"git help -a" seems to be similar to the latter modes of "archive".

> And by documenting, I mean in the code, so that the code can DTRT.
> 
> Otherwise, having this switch lets people shoot themselves in the foot, 
> I'd think.

Git offers plenty of opportunity to shoot yourself in the foot (and it 
does not help that we are introducing user-unfriendly constructs like the 
current form of the foreign VCS helpers with more such opportunities, 
either), but for the love of God, I cannot find how "this switch" lets 
people shoot themselves in the foot here.

Ciao,
Dscho

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Gerrit Pape]

On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> -- snipsnap --
> [PATCH] git help -a: do not look for a repository

Perfect, thanks.

Acked-by: Gerrit Pape <pape@smarden.org>

> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> ---
> 
>  builtin-help.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin-help.c b/builtin-help.c
> index e1eba77..719aa23 100644
> --- a/builtin-help.c
> +++ b/builtin-help.c
> @@ -416,9 +416,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  	const char *alias;
>  	load_command_list("git-", &main_cmds, &other_cmds);
>  
> -	setup_git_directory_gently(&nongit);
> -	git_config(git_help_config, NULL);
> -
>  	argc = parse_options(argc, argv, prefix, builtin_help_options,
>  			builtin_help_usage, 0);
>  
> @@ -429,6 +426,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  		return 0;
>  	}
>  
> +	setup_git_directory_gently(&nongit);
> +	git_config(git_help_config, NULL);
> +
>  	if (!argv[0]) {
>  		printf("usage: %s\n\n", git_usage_string);
>  		list_common_cmds_help();
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Johannes Schindelin]

Hi,

On Fri, 4 Sep 2009, Gerrit Pape wrote:

> On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > -- snipsnap --
> > [PATCH] git help -a: do not look for a repository
> 
> Perfect, thanks.
> 
> Acked-by: Gerrit Pape <pape@smarden.org>

Hmm... I haven't checked if 'git help -a' wants to discover the (possibly 
repository-specific) aliases.  Have you?

Ciao,
Dscho

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Gerrit Pape]

On Fri, Sep 04, 2009 at 02:35:00PM +0200, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 4 Sep 2009, Gerrit Pape wrote:
> 
> > On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > > -- snipsnap --
> > > [PATCH] git help -a: do not look for a repository
> > 
> > Perfect, thanks.
> > 
> > Acked-by: Gerrit Pape <pape@smarden.org>
> 
> Hmm... I haven't checked if 'git help -a' wants to discover the (possibly 
> repository-specific) aliases.  Have you?

Yes, it doesn't.  Regards, Gerrit.

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Johannes Schindelin]

Hi,

On Fri, 4 Sep 2009, Gerrit Pape wrote:

> On Fri, Sep 04, 2009 at 02:35:00PM +0200, Johannes Schindelin wrote:
> > Hi,
> > 
> > On Fri, 4 Sep 2009, Gerrit Pape wrote:
> > 
> > > On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > > > -- snipsnap --
> > > > [PATCH] git help -a: do not look for a repository
> > > 
> > > Perfect, thanks.
> > > 
> > > Acked-by: Gerrit Pape <pape@smarden.org>
> > 
> > Hmm... I haven't checked if 'git help -a' wants to discover the 
> > (possibly repository-specific) aliases.  Have you?
> 
> Yes, it doesn't.  Regards, Gerrit.

Thanks.

Junio, I am serious about this patch (i.e. it is meant for inclusion now).  
Want me to resend?

Ciao,
Dscho

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Thanks.
>
> Junio, I am serious about this patch (i.e. it is meant for inclusion now).  
> Want me to resend?

Thanks.

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Gerrit Pape]

Hi Junio, I suggest to apply this patch from Johannes to master.

Thanks, Gerrit.


On Fri, Sep 04, 2009 at 11:09:36AM +0000, Gerrit Pape wrote:
> On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > -- snipsnap --
> > [PATCH] git help -a: do not look for a repository
> 
> Perfect, thanks.
> 
> Acked-by: Gerrit Pape <pape@smarden.org>
> 
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > 
> > ---
> > 
> >  builtin-help.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/builtin-help.c b/builtin-help.c
> > index e1eba77..719aa23 100644
> > --- a/builtin-help.c
> > +++ b/builtin-help.c
> > @@ -416,9 +416,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
> >  	const char *alias;
> >  	load_command_list("git-", &main_cmds, &other_cmds);
> >  
> > -	setup_git_directory_gently(&nongit);
> > -	git_config(git_help_config, NULL);
> > -
> >  	argc = parse_options(argc, argv, prefix, builtin_help_options,
> >  			builtin_help_usage, 0);
> >  
> > @@ -429,6 +426,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
> >  		return 0;
> >  	}
> >  
> > +	setup_git_directory_gently(&nongit);
> > +	git_config(git_help_config, NULL);
> > +
> >  	if (!argv[0]) {
> >  		printf("usage: %s\n\n", git_usage_string);
> >  		list_common_cmds_help();
> > --
> > To unsubscribe from this list: send the line "unsubscribe git" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository

[Junio C Hamano]

Gerrit Pape <pape@smarden.org> writes:

> Hi Junio, I suggest to apply this patch from Johannes to master.
>
> Thanks, Gerrit.

Could you help by coming up with a suitable log message?

It's a bit too much to ask me to hunt for ancient discussion to correct
the <<all the ack go here>> myself to describe what the issue was,
especially when I wasn't heavily involved in the review of the patch
itself.  My impression is that the original description of the problem 
and the solution in your first message does not apply to what Dscho and
you agreed to be the best solution.

>
> On Fri, Sep 04, 2009 at 11:09:36AM +0000, Gerrit Pape wrote:
>> On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
>> > -- snipsnap --
>> > [PATCH] git help -a: do not look for a repository
>> 
>> Perfect, thanks.
>> 
>> Acked-by: Gerrit Pape <pape@smarden.org>
>> 
>> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> > 
>> > ---
>> > 
>> >  builtin-help.c |    6 +++---
>> >  1 files changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/builtin-help.c b/builtin-help.c
>> > index e1eba77..719aa23 100644
>> > --- a/builtin-help.c
>> > +++ b/builtin-help.c
>> > @@ -416,9 +416,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>> >  	const char *alias;
>> >  	load_command_list("git-", &main_cmds, &other_cmds);
>> >  
>> > -	setup_git_directory_gently(&nongit);
>> > -	git_config(git_help_config, NULL);
>> > -
>> >  	argc = parse_options(argc, argv, prefix, builtin_help_options,
>> >  			builtin_help_usage, 0);
>> >  
>> > @@ -429,6 +426,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>> >  		return 0;
>> >  	}
>> >  
>> > +	setup_git_directory_gently(&nongit);
>> > +	git_config(git_help_config, NULL);
>> > +
>> >  	if (!argv[0]) {
>> >  		printf("usage: %s\n\n", git_usage_string);
>> >  		list_common_cmds_help();
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe git" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] help -a: do not unnecessarily look for a repository

[Gerrit Pape]

Although 'git help -a' actually doesn't need to be run inside a git
repository and uses no repository-specific information, it looks for a
git directory.  On 'git <TAB><TAB>' the bash completion runs 'git help
-a' and unnecessary searching for a git directory can be annoying in
auto-mount environments.  With this commit, 'git help' no longer
searches for a repository when run with the -a option.

The fix is from Johannes Schindelin, the annoying behavior has been
reported by Vincent Danjean through
 http://bugs.debian.org/539273

Signed-off-by: Gerrit Pape <pape@smarden.org>
---

On Tue, Oct 27, 2009 at 11:11:01PM -0700, Junio C Hamano wrote:
> Gerrit Pape <pape@smarden.org> writes:
> > Hi Junio, I suggest to apply this patch from Johannes to master.

> Could you help by coming up with a suitable log message?
>
> It's a bit too much to ask me to hunt for ancient discussion to
> correct the <<all the ack go here>> myself to describe what the
> issue was,

Sure, sorry for that.  Regards, Gerrit.


 builtin-help.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-help.c b/builtin-help.c
index e1ade8e..ca08519 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -417,9 +417,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	const char *alias;
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	setup_git_directory_gently(&nongit);
-	git_config(git_help_config, NULL);
-
 	argc = parse_options(argc, argv, prefix, builtin_help_options,
 			builtin_help_usage, 0);
 
@@ -430,6 +427,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		return 0;
 	}
 
+	setup_git_directory_gently(&nongit);
+	git_config(git_help_config, NULL);
+
 	if (!argv[0]) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
-- 
1.6.5.2

Re: [PATCH] help -a: do not unnecessarily look for a repository

[Junio C Hamano]

Thanks.