[PATCH] add: allow configurations to be overriden by command line

[PATCH] add: allow configurations to be overriden by command line

[Stephen Boyd]

Don't call git_config after parsing the command line options, otherwise
the config settings will override any settings made by the command line.

This can be seen by setting add.ignore_errors and then specifying
--no-ignore-errors when using git-add.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-add.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index c1b229a..56e5221 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -298,6 +298,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int add_new_files;
 	int require_pathspec;
 
+	git_config(add_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, builtin_add_options,
 			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
 	if (patch_interactive)
@@ -305,8 +307,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	if (add_interactive)
 		exit(interactive_add(argc - 1, argv + 1, prefix));
 
-	git_config(add_config, NULL);
-
 	if (edit_interactive)
 		return(edit_patch(argc, argv, prefix));
 	argc--;
-- 
1.6.3.2.306.g4f4fa

Re: [PATCH] add: allow configurations to be overriden by command line

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> Don't call git_config after parsing the command line options, otherwise
> the config settings will override any settings made by the command line.

Amusing.

This dates back to the original 0d78153 (Do "git add" as a builtin,
2006-05-17).  It had two git_config() calls (one before command line
parsing, and then another).

Then 11be42a (Make git-mv a builtin, 2006-07-26) noticed this duplicate
calls, and removed the wrong one.

The calling sequence has been the same ever since; parse-opt-ification
5c46f75 (Port builtin-add.c to use the new option parser., 2007-10-03) 
did not affect it either.

Perhaps this deserves a test.  It is an ancient breakage nobody noticed so
far.

Thanks

Re: [PATCH] add: allow configurations to be overriden by command line

[Stephen Boyd]

Junio C Hamano wrote:
>
> Perhaps this deserves a test.  It is an ancient breakage nobody noticed so
> far

I didn't think it would be very useful, which is why I left it out. But
hey, at least it stops it from happening again. Squash it in if you want.

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 6ce8256..91c1f7a 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -242,4 +242,13 @@ test_expect_success 'git add to resolve conflicts on otherwise ignored path' '
 	git add track-this
 '
 
+test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
+	git config add.ignore-errors 1 &&
+	git reset --hard &&
+	date >foo1 &&
+	test_must_fail git add --verbose --no-ignore-errors . &&
+	! ( git ls-files foo1 | grep foo1 ) &&
+	git config add.ignore-errors 0
+'
+
 test_done

Re: [PATCH] add: allow configurations to be overriden by command line

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> I didn't think it would be very useful, which is why I left it out. But
> hey, at least it stops it from happening again. Squash it in if you want.
>
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 6ce8256..91c1f7a 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -242,4 +242,13 @@ test_expect_success 'git add to resolve conflicts on otherwise ignored path' '
>  	git add track-this
>  '
>  
> +test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
> +	git config add.ignore-errors 1 &&
> +	git reset --hard &&
> +	date >foo1 &&
> +	test_must_fail git add --verbose --no-ignore-errors . &&
> +	! ( git ls-files foo1 | grep foo1 ) &&
> +	git config add.ignore-errors 0
> +'
> +
>  test_done

Why do you need POSIXPERM for this?

Re: [PATCH] add: allow configurations to be overriden by command line

[Stephen Boyd]

Junio C Hamano wrote:
> Why do you need POSIXPERM for this?
>   

I copied a previous test and it was doing chmod 0 foo2. This test is
actually depending on that earlier test which is probably bad style,
sorry. Here's a more insulated version.

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 6ce8256..66bdc24 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -221,6 +221,19 @@ test_expect_success POSIXPERM 'git add (add.ignore-errors = false)' '
        test_must_fail git add --verbose . &&
        ! ( git ls-files foo1 | grep foo1 )
 '
+rm -f foo2
+
+test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
+       git config add.ignore-errors 1 &&
+       git reset --hard &&
+       date >foo1 &&
+       date >foo2 &&
+       chmod 0 foo2 &&
+       test_must_fail git add --verbose --no-ignore-errors . &&
+       ! ( git ls-files foo1 | grep foo1 ) &&
+       git config add.ignore-errors 0
+'
+rm -f foo2
 
 test_expect_success BSLASHPSPEC "git add 'fo\\[ou\\]bar' ignores foobar" '
        git reset --hard &&

[PATCH] t3700-add: add a POSIXPERM prerequisite to a new test

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

The new test does a 'chmod 0', which does not have the intended
effect on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Stephen Boyd schrieb:
> Junio C Hamano wrote:
>> Why do you need POSIXPERM for this?
> 
> I copied a previous test and it was doing chmod 0 foo2.
...
> +test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
> +       git config add.ignore-errors 1 &&
> +       git reset --hard &&
> +       date >foo1 &&
> +       date >foo2 &&
> +       chmod 0 foo2 &&

I can only guess that you missed this 'chmod 0' despite Stephen's
explanation and dropped POSIXPERM when you applied the patch.

-- Hannes

 t/t3700-add.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 6ae5a2c..85eb0fb 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -223,7 +223,7 @@ test_expect_success POSIXPERM 'git add
 '
 rm -f foo2

-test_expect_success '--no-ignore-errors overrides config' '
+test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
        git config add.ignore-errors 1 &&
        git reset --hard &&
        date >foo1 &&
-- 
1.6.3.2.1450.g146e1

Re: [PATCH] t3700-add: add a POSIXPERM prerequisite to a new test

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> From: Johannes Sixt <j6t@kdbg.org>
>
> The new test does a 'chmod 0', which does not have the intended
> effect on Windows.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Stephen Boyd schrieb:
>> Junio C Hamano wrote:
>>> Why do you need POSIXPERM for this?
>> 
>> I copied a previous test and it was doing chmod 0 foo2.
> ...
>> +test_expect_success POSIXPERM '--no-ignore-errors overrides config' '
>> +       git config add.ignore-errors 1 &&
>> +       git reset --hard &&
>> +       date >foo1 &&
>> +       date >foo2 &&
>> +       chmod 0 foo2 &&
>
> I can only guess that you missed this 'chmod 0' despite Stephen's
> explanation and dropped POSIXPERM when you applied the patch.

Actually, I wanted to add the test to 'maint' and 'maint-$v' for values of
$v that are lower than 1.6.3, so I did want to drop POSIXPERM on the
branch the patch initially applied; I forgot to make an evil merge when I
merged the result up, crossing 1.6.2/1.6.3 boundary..

Will queue on 'maint'.  Thanks.