Re: [PATCH] config: don't BUG when both kvi and source are set

[Junio C Hamano <gitster@pobox.com>]

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Therefore, fix the bug by removing the BUG() check. We're reverting to
> an older, less safe state, but that's generally okay since
> key_value_info is always preferentially read, so we'd always read the
> correct values when we iterate a config set in the middle of a config
> parse (like we are here).

I wonder if the source being pushed and config_kvi value at this
point have some particular relationship (like "if kvi exists, the
source must match kvi's source" or something) that we can cheaply
use to avoid "reverting to an older less safe state"?

I would agree that, as long as we know by the end of this summer a
real fix would come to rescue us ;-), it is sensible not to add too
much code to work it around for the short-term.

> The reverse would be wrong, but extremely
> unlikely to happen since very few callers parse config without going
> through a config set.

Sorry, but I do not quite get this comment.

>     Here's a quick fix for the bug reported at [1]. As noted in the commit
>     message and that thread, I think the real fix to take [2], which
>     simplifies the config.c state and makes this a non-issue, so this is
>     just a band-aid while we wait for that.

Thanks for a quick fix.  Will queue.

> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> index f519d2a87a7..8759fc28533 100755
> --- a/t/t5616-partial-clone.sh
> +++ b/t/t5616-partial-clone.sh
> @@ -257,8 +257,8 @@ test_expect_success 'partial clone with transfer.fsckobjects=1 works with submod
>  	test_commit -C submodule mycommit &&
>  
>  	test_create_repo src_with_sub &&
> -	test_config -C src_with_sub uploadpack.allowfilter 1 &&
> -	test_config -C src_with_sub uploadpack.allowanysha1inwant 1 &&
> +	git -C src_with_sub config uploadpack.allowfilter 1 &&
> +	git -C src_with_sub config uploadpack.allowanysha1inwant 1 &&

We only tentatively configured uploadpack in src_with_sub in the
original because this single test piece was the only place where
src_with_sub repository was used, but now we use a more permanent
configuration because ...

> @@ -270,6 +270,12 @@ test_expect_success 'partial clone with transfer.fsckobjects=1 works with submod
>  	test_when_finished rm -rf dst
>  '
>  
> +test_expect_success 'lazily fetched .gitmodules works' '
> +	git clone --filter="blob:none" --no-checkout "file://$(pwd)/src_with_sub" dst &&
> +	git -C dst fetch &&
> +	test_when_finished rm -rf dst
> +'

... we run another "git clone" from the repository now.

OK.

Thanks.