Re: [PATCH] git-config: Parse config files leniently

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 14, 2009, 19:52 UTC
Message-ID: <7veirejhqq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <a812f567b4541ce55e9c60037a047488a0893c36.1250262273.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 6 quoted lines

> Currently, git config dies as soon as there is a parsing error. This is
> especially unfortunate in case a user tries to correct config mistakes
> using git config -e.
>
> Instead, issue a warning only and treat the rest of the line as a
> comment (ignore it). This benefits not only git config -e users.

... a broken sentence in the middle? I would have expected the "not only" followed by "but also"; the question is "but also what?"

Hopefully the benefit is not that it now allows all the other commands to cause unspecified types of damage to the repository by following iffy settings obtained from a broken configuration file.

> Reported-by: David Reitter <david.reitter@gmail.com>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>

> Test had to be adjusted as well.

The change to the test demonstrates the issue rather well. The check() shell function does not check the exit value from "git config --get", but in a real script that cares to check and stop on error, this change will now let the script go on, leaving the breakage unnoticed. I suspect command implemented in C, that call git_config(), will also have the same issue, and I cannot convince myself this is a good change in general, outside the scope of helping "git config -e".

But I may be being overly cautious.

By the way, why did you have to change s/echo/printf/? Can't you give two lines in a single argument without "\n" escape?

Show 22 quoted lines

> diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
> index 080117c..be850c5 100755
> --- a/t/t1303-wacky-config.sh
> +++ b/t/t1303-wacky-config.sh
> @@ -9,7 +9,7 @@ setup() {
>  }
>  
>  check() {
> -	echo "$2" >expected
> +	printf "$2\n" >expected
>  	git config --get "$1" >actual 2>&1
>  	test_cmp actual expected
>  }
> @@ -44,7 +44,7 @@ LONG_VALUE=$(printf "x%01021dx a" 7)
>  test_expect_success 'do not crash on special long config line' '
>  	setup &&
>  	git config section.key "$LONG_VALUE" &&
> -	check section.key "fatal: bad config file line 2 in .git/config"
> +	check section.key "warning: bad config file line 2 in .git/config\nwarning: bad config file line 2 in .git/config"
>  '
>  
>  test_done