Subject: Re: [PATCH v3 06/10] submodule update: add `--init-active` switch

From: Junio C Hamano <[email-masked]>
Date: Tue, 14 Mar 2017 12:18:52 -0700

Brandon Williams <[email-masked]> writes:

> +--init-active::
> +	This option is only valid for the update command.
> +	Initialize all submodules configured in "`submodule.active`"
> +	that have not been updated before.

You mean s/not been updated/not been initialized/ here?

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index f38e332c5..a3acc9e4c 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -270,6 +270,34 @@ static int module_list_compute(int argc, const char **argv,
>  	return result;
>  }
>  
> +static void module_list_active(struct module_list *list)
> +{
> +	int i;
> +
> +	if (read_cache() < 0)
> +		die(_("index file corrupt"));
> +
> +	gitmodules_config();

To this codepath, it probably would not make any difference, but in
general, calling gitmodules_config() after you did git_config()
would be a bug, right?  Otherwise, gitmodules_config() would
overwrite submodule.* configuration you read with git_config().

I have a feeling that we need to make this function hard to misuse;
it is not a fault of _this_ patch, but still any and all additions
to any of the submodule*.c files needs careful vetting.