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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2017, 19:18 UTC
Message-ID: <xmqqvarbis4j.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170313214341.172676-7-bmwill@google.com>

Brandon Williams <bmwill@google.com> 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?

Show 16 quoted lines

> 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.