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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:

> 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@xxxxxxxxx>
> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>

> 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?

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]