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

[Emily Shaffer <emilyshaffer@google.com>]

On Thu, Mar 10, 2022 at 01:54:12PM -0800, Glen Choo wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > 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..
> 
> >  
> > @@ -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.

Ah, this makes sense. Sure, will do.

 - Emily