Re: [PATCH v9 2/3] introduce submodule.hasSuperproject record

From: Glen Choo <chooglen@google.com>
Date: Mar 10, 2022, 21:54 UTC
Message-ID: <kl6l7d91sccb.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20220310004423.2627181-3-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 17 quoted lines

> diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt
> index ee454f8126..99d5260b8e 100644
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index c5d3fc3817..eda9ed550e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1839,6 +1839,11 @@ static int clone_submodule(struct module_clone_data *clone_data)
>  		git_config_set_in_file(p, "submodule.alternateErrorStrategy",
>  				       error_strategy);
>  
> +	/*
> +	 * Teach the submodule that it's a submodule.
> +	 */
> +	git_config_set_in_file(p, "submodule.hasSuperproject", "true");
> +
>  	free(sm_alternate);
>  	free(error_strategy);

This git_config_set_* is superfluous - it sets the config in newly cloned submodules..

Show 14 quoted lines

>  
> @@ -2617,6 +2622,12 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)
>  
>  	free(prefixed_path);
>  
> +	/*
> +	 * This entry point is always called from a submodule, so this is a
> +	 * good place to set a hint that this repo is a submodule.
> +	 */
> +	git_config_set("submodule.hasSuperproject", "true");
> +
>  	if (!oideq(&update_data.oid, &update_data.suboid) || update_data.force)
>  		return do_run_update_procedure(&update_data);
>

but this is called over *all* submodules, so we're guaranteed to always set the config if "git submodule update" isn't interrupted halfway.

I don't think we guarantee correctness if it is interrupted halfway e.g. core.worktree can be unset if it is interrupted halfway (because ensure-core-worktree is called adjacent to run-update-procedure, not inside of update-clone).

So I think it's better to just drop the previous hunk - it will disappear anyway in gc/submodule-update-part2.