# [PATCH] add: allow configurations to be overriden by command line 7 messages from 2009-06-18 to 2009-06-22. Participants: Stephen Boyd, Junio C Hamano, Johannes Sixt. Thread: https://gitlist.dev/t/19851 ## Stephen Boyd, 2009-06-18 09:17 Subject: [PATCH] add: allow configurations to be overriden by command line Message-ID: <1245316674-32377-1-git-send-email-bebarino@gmail.com> URL: https://gitlist.dev/e/1245316674-32377-1-git-send-email-bebarino%40gmail.com ``` 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 --- 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 ``` ## Junio C Hamano, 2009-06-18 16:40 Subject: Re: [PATCH] add: allow configurations to be overriden by command line Message-ID: <7vfxdxxzps.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxdxxzps.fsf%40alter.siamese.dyndns.org In-Reply-To: <1245316674-32377-1-git-send-email-bebarino@gmail.com> ``` Stephen Boyd 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 ``` ## Stephen Boyd, 2009-06-18 18:14 Subject: Re: [PATCH] add: allow configurations to be overriden by command line Message-ID: <4A3A8412.7070705@gmail.com> URL: https://gitlist.dev/e/4A3A8412.7070705%40gmail.com In-Reply-To: <7vfxdxxzps.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Junio C Hamano, 2009-06-18 18:34 Subject: Re: [PATCH] add: allow configurations to be overriden by command line Message-ID: <7veitho0gu.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7veitho0gu.fsf%40alter.siamese.dyndns.org In-Reply-To: <4A3A8412.7070705@gmail.com> ``` Stephen Boyd 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? ``` ## Stephen Boyd, 2009-06-18 18:58 Subject: Re: [PATCH] add: allow configurations to be overriden by command line Message-ID: <4A3A8E73.1010807@gmail.com> URL: https://gitlist.dev/e/4A3A8E73.1010807%40gmail.com In-Reply-To: <7veitho0gu.fsf@alter.siamese.dyndns.org> ``` 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 && ``` ## Johannes Sixt, 2009-06-22 07:30 Subject: [PATCH] t3700-add: add a POSIXPERM prerequisite to a new test Message-ID: <4A3F331E.5080301@viscovery.net> URL: https://gitlist.dev/e/4A3F331E.5080301%40viscovery.net In-Reply-To: <4A3A8E73.1010807@gmail.com> ``` From: Johannes Sixt The new test does a 'chmod 0', which does not have the intended effect on Windows. Signed-off-by: Johannes Sixt --- 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 ``` ## Junio C Hamano, 2009-06-22 07:42 Subject: Re: [PATCH] t3700-add: add a POSIXPERM prerequisite to a new test Message-ID: <7vab40g1dv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vab40g1dv.fsf%40alter.siamese.dyndns.org In-Reply-To: <4A3F331E.5080301@viscovery.net> ``` Johannes Sixt writes: > From: Johannes Sixt > > The new test does a 'chmod 0', which does not have the intended > effect on Windows. > > Signed-off-by: Johannes Sixt > --- > 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. ```