Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 16, 2018, 18:04 UTC
Message-ID: <xmqqwotvt5uz.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180713193958.GB12162@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>    site's support). And the broken .gitmodules may be too
>    far back in history for rewriting to be feasible (again,
>    this is an issue for cgit).

"again" but this is the first mention that hints cgit has some problem (but not exactly what problem). Is that the "cgit has a file called .gitmodules that predates the submodule support on our side?" thing?

Show 17 quoted lines

> So we're being unnecessarily restrictive without actually
> improving the security in a meaningful way. It would be more
> convenient to downgrade this check to "info", which means
> we'd still comment on it, but not reject a push. Site admins
> can already do this via config, but we should ship sensible
> defaults.
> ...
> Considering both sets of arguments, it makes sense to loosen
> this check for now.
>
> Note that we have to tweak the test in t7415 since fsck will
> no longer consider this a fatal error. But we still check
> that it reports the warning, and that we don't get the
> spurious error from the config code.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Thanks.

Show 37 quoted lines

>  fsck.c                     | 2 +-
>  t/t7415-submodule-names.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index 4129935d86..69ea8d5321 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -62,7 +62,6 @@ static struct oidset gitmodules_done = OIDSET_INIT;
>  	FUNC(ZERO_PADDED_DATE, ERROR) \
>  	FUNC(GITMODULES_MISSING, ERROR) \
>  	FUNC(GITMODULES_BLOB, ERROR) \
> -	FUNC(GITMODULES_PARSE, ERROR) \
>  	FUNC(GITMODULES_LARGE, ERROR) \
>  	FUNC(GITMODULES_NAME, ERROR) \
>  	FUNC(GITMODULES_SYMLINK, ERROR) \
> @@ -77,6 +76,7 @@ static struct oidset gitmodules_done = OIDSET_INIT;
>  	FUNC(ZERO_PADDED_FILEMODE, WARN) \
>  	FUNC(NUL_IN_COMMIT, WARN) \
>  	/* infos (reported as warnings, but ignored by default) */ \
> +	FUNC(GITMODULES_PARSE, INFO) \
>  	FUNC(BAD_TAG_NAME, INFO) \
>  	FUNC(MISSING_TAGGER_ENTRY, INFO)
>  
> diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
> index ba8af785a5..293e2e1963 100755
> --- a/t/t7415-submodule-names.sh
> +++ b/t/t7415-submodule-names.sh
> @@ -185,7 +185,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '
>  		git add .gitmodules &&
>  		git commit -m "broken gitmodules" &&
>  
> -		test_must_fail git fsck 2>output &&
> +		git fsck 2>output &&
>  		grep gitmodulesParse output &&
>  		test_i18ngrep ! "bad config" output
>  	)