threads / patch / 19133

patchgit config: error when editing a repo config and not being in one

Subject: [PATCH] git config: error when editing a repo config and not being in one

## tl;dr

8 messages between Apr 29, 2009 and Apr 30, 2009. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Felipe Contreras· Apr 29, 2009, 22:25 UTC · lore

Let's throw an error on this specific case. If the user specifies the config file, he must know what he is doing.

Teemu Likonen pointed this out.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin-config.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to builtin-config.c +2 −0
diff --git a/builtin-config.c b/builtin-config.c
index d8da72c..6e936e1 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -390,6 +390,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 	}
 	else if (actions == ACTION_EDIT) {
 		check_argc(argc, 0, 0);
+		if (!config_exclusive_filename && !is_inside_git_dir())
+			die("not in a git directory");
 		git_config(git_default_config, NULL);
 		launch_editor(config_exclusive_filename ?
 			      config_exclusive_filename : git_path("config"),
-- 
1.6.3.rc3.12.gb7937.dirty
Johannes Schindelin· Apr 29, 2009, 22:44 UTC · re: Felipe Contreras · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

Hi,
On Thu, 30 Apr 2009, Felipe Contreras wrote:
Show 10 quoted lines
> diff --git a/builtin-config.c b/builtin-config.c
> index d8da72c..6e936e1 100644
> --- a/builtin-config.c
> +++ b/builtin-config.c
> @@ -390,6 +390,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
>  	}
>  	else if (actions == ACTION_EDIT) {
>  		check_argc(argc, 0, 0);
> +		if (!config_exclusive_filename && !is_inside_git_dir())
> +			die("not in a git directory");
Why not use the nongit variable?

Ciao, Dscho

Felipe Contreras· Apr 29, 2009, 22:49 UTC · re: Johannes Schindelin · lore

Let's throw an error on this specific case. If the user specifies the config file, he must know what he is doing.

Teemu Likonen pointed this out.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin-config.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to builtin-config.c +2 −0
diff --git a/builtin-config.c b/builtin-config.c
index d8da72c..a81bc8b 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -390,6 +390,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 	}
 	else if (actions == ACTION_EDIT) {
 		check_argc(argc, 0, 0);
+		if (!config_exclusive_filename && nongit)
+			die("not in a git directory");
 		git_config(git_default_config, NULL);
 		launch_editor(config_exclusive_filename ?
 			      config_exclusive_filename : git_path("config"),
-- 
1.6.3.rc3.13.g195b.dirty
Johannes Schindelin· Apr 30, 2009, 08:37 UTC · re: Felipe Contreras · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

Hi,
On Thu, 30 Apr 2009, Felipe Contreras wrote:
Show 6 quoted lines
> Let's throw an error on this specific case. If the user specifies the
> config file, he must know what he is doing.
> 
> Teemu Likonen pointed this out.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
I would have Acked it, but Junio already applied it ;-)

Ciao, Dscho

Felipe Contreras· Apr 30, 2009, 09:11 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

On Thu, Apr 30, 2009 at 11:37 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 12 quoted lines
> Hi,
>
> On Thu, 30 Apr 2009, Felipe Contreras wrote:
>
>> Let's throw an error on this specific case. If the user specifies the
>> config file, he must know what he is doing.
>>
>> Teemu Likonen pointed this out.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> I would have Acked it, but Junio already applied it ;-)
:)

Just for the record, where is people supposed to learn about 'nongit'? Apparently it's not mentioned in the documentation.

-- 
Felipe Contreras
Johannes Schindelin· Apr 30, 2009, 09:21 UTC · re: Felipe Contreras · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

Hi,
On Thu, 30 Apr 2009, Felipe Contreras wrote:
Show 18 quoted lines
> On Thu, Apr 30, 2009 at 11:37 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Thu, 30 Apr 2009, Felipe Contreras wrote:
> >
> >> Let's throw an error on this specific case. If the user specifies the 
> >> config file, he must know what he is doing.
> >>
> >> Teemu Likonen pointed this out.
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > I would have Acked it, but Junio already applied it ;-)
> 
> :)
> 
> Just for the record, where is people supposed to learn about 'nongit'? 
> Apparently it's not mentioned in the documentation.

It's not about 'nongit'. It is about setting up the git directory "gently", i.e you can ask for a setup _if possible_ using setup_git_directory_gently(), which will not fail if we're not in any repository.

To require a repository, call setup_git_directory(). This will die() if there is no repository.

As we already ask via _gently() in cmd_config(), it appeared natural to me to reuse that information rather than redoing the discovery.

Ciao, Dscho

Junio C Hamano· Apr 29, 2009, 23:22 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 16 quoted lines
> Hi,
>
> On Thu, 30 Apr 2009, Felipe Contreras wrote:
>
>> diff --git a/builtin-config.c b/builtin-config.c
>> index d8da72c..6e936e1 100644
>> --- a/builtin-config.c
>> +++ b/builtin-config.c
>> @@ -390,6 +390,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
>>  	}
>>  	else if (actions == ACTION_EDIT) {
>>  		check_argc(argc, 0, 0);
>> +		if (!config_exclusive_filename && !is_inside_git_dir())
>> +			die("not in a git directory");
>
> Why not use the nongit variable?
Good; makes a lot more sense.
Junio C Hamano· Apr 29, 2009, 23:01 UTC · re: Felipe Contreras · lore

Re: [PATCH] git config: error when editing a repo config and not being in one

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 23 quoted lines
> Let's throw an error on this specific case. If the user specifies the
> config file, he must know what he is doing.
>
> Teemu Likonen pointed this out.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin-config.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-config.c b/builtin-config.c
> index d8da72c..6e936e1 100644
> --- a/builtin-config.c
> +++ b/builtin-config.c
> @@ -390,6 +390,8 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
>  	}
>  	else if (actions == ACTION_EDIT) {
>  		check_argc(argc, 0, 0);
> +		if (!config_exclusive_filename && !is_inside_git_dir())
> +			die("not in a git directory");
>  		git_config(git_default_config, NULL);
>  		launch_editor(config_exclusive_filename ?
>  			      config_exclusive_filename : git_path("config"),
How could this be correct?

When you are inside a work tree controlled by a git repository, and you want to edit the configuration file that belongs to the repository, I do not think is_inside_git_dir() is true.

What you are trying to catch is if $GIT_DIR exists (not in the sense that "is it a non-empty string?", but in the sense that "does it point at an existing directory?"), I think.

So perhaps you want to say something like this instead?
No, I did not test it.
 builtin-config.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Show changes to builtin-config.c +6 −0
diff --git a/builtin-config.c b/builtin-config.c
index d8da72c..a4bd516 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -390,6 +390,12 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 	}
 	else if (actions == ACTION_EDIT) {
 		check_argc(argc, 0, 0);
+		if (!config_exclusive_filename) {
+			const char *git = get_git_dir();
+			struct stat sb;
+			if (!git || stat(git, &sb) || !S_ISDIR(sb.st_mode))
+				die("not in a git directory");
+		}
 		git_config(git_default_config, NULL);
 		launch_editor(config_exclusive_filename ?
 			      config_exclusive_filename : git_path("config"),

← back to recent threads