Re: [PATCH v9 3/3] rev-parse: short-circuit superproject worktree when config unset

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 10, 2022, 01:47 UTC
Message-ID: <xmqq35jqino7.fsf@gitster.g>
In-Reply-To: <20220310004423.2627181-4-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 6 quoted lines

> +	/*
> +	 * Because get_superproject_working_tree() is older than
> +	 * submodule.hasSuperproject, don't rely on the default "unset = false"
> +	 * - instead, only rely on if submodule.hasSuperproject was explicitly
> +	 * set to false.
> +	 */

That's a round-about way to say that submodule.hassuperproject defaults to true, isn't it ;-)?

Show 8 quoted lines

> +	if (! git_config_get_bool("submodule.hassuperproject", &has_superproject_cfg)
> +	    && !has_superproject_cfg) {
> +		/*
> +		 * If we don't have a superproject, then we're probably not a
> +		 * submodule. If this is failing and shouldn't be, investigate
> +		 * why the config was set to false.
> +		 */
> +		error(_("Asked to find a superproject, but submodule.hasSuperproject == false"));

s/Asked to/asked to/, probably.

Show 27 quoted lines

> +		return 0;
> +	}
> +
>  	if (!strbuf_realpath(&one_up, "../", 0))
>  		return 0;
>  
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 1c2df08333..dd35036bd6 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -244,7 +244,15 @@ test_expect_success 'showing the superproject correctly' '
>  	test_must_fail git -C super merge branch1 &&
>  
>  	git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> -	test_cmp expect out
> +	test_cmp expect out &&
> +
> +	# When submodule.hasSuperproject=false, --show-superproject-working-tree
> +	# should fail instead of checking the filesystem.
> +	test_config -C super/dir/sub submodule.hasSuperproject false &&
> +	git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> +	# --show-superproject-working-tree should print an error about the
> +	# broken config
> +	! grep "error:.*hasSuperproject" out
>  '
>  
>  # at least one external project depends on this behavior: