Subject: Re: [PATCH 07/14] update submodules: introduce is_interesting_submodule

From: Brandon Williams <[email-masked]>
Date: Wed, 15 Feb 2017 09:04:41 -0800

On 02/14, Stefan Beller wrote:
> In later patches we introduce the --recurse-submodule flag for commands
> that modify the working directory, e.g. git-checkout.
> 
> It is potentially expensive to check if a submodule needs an update,
> because a common theme to interact with submodules is to spawn a child
> process for each interaction.
> 
> So let's introduce a function that pre checks if a submodule needs
> to be checked for an update.
> 
> Signed-off-by: Stefan Beller <[email-masked]>
> ---
>  submodule.c | 26 ++++++++++++++++++++++++++
>  submodule.h |  8 ++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/submodule.c b/submodule.c
> index c0060c29f2..4c33374ae8 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -551,6 +551,32 @@ void set_config_update_recurse_submodules(int value)
>  	config_update_recurse_submodules = value;
>  }
>  
> +int submodules_interesting_for_update(void)
> +{
> +	/*
> +	 * Update can't be "none", "merge" or "rebase",
> +	 * treat any value as OFF, except an explicit ON.
> +	 */
> +	return config_update_recurse_submodules == RECURSE_SUBMODULES_ON;
> +}
> +
> +int is_interesting_submodule(const struct cache_entry *ce)

Is there perhaps a more descriptive function name we could use instead
of "is_interesting"?  The problem is that its difficult to know why its
interesting or for what purpose it is interesting.

> +{
> +	const struct submodule *sub;
> +
> +	if (!S_ISGITLINK(ce->ce_mode))
> +		return 0;
> +
> +	if (!submodules_interesting_for_update())
> +		return 0;
> +
> +	sub = submodule_from_path(null_sha1, ce->name);
> +	if (!sub)
> +		return 0;
> +
> +	return sub->update_strategy.type != SM_UPDATE_NONE;
> +}
> +
>  static int has_remote(const char *refname, const struct object_id *oid,
>  		      int flags, void *cb_data)
>  {
> diff --git a/submodule.h b/submodule.h
> index c4e1ac828e..84b67a7c4a 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -59,6 +59,14 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
>  		const struct diff_options *opt);
>  extern void set_config_fetch_recurse_submodules(int value);
>  extern void set_config_update_recurse_submodules(int value);
> +
> +/*
> + * Traditionally Git ignored changes made for submodules.
> + * This function checks if we are interested in the given submodule
> + * for any kind of operation.
> + */
> +extern int submodules_interesting_for_update(void);
> +extern int is_interesting_submodule(const struct cache_entry *ce);
>  extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
>  extern int fetch_populated_submodules(const struct argv_array *options,
>  			       const char *prefix, int command_line_option,
> -- 
> 2.12.0.rc0.16.gd1691994b4.dirty
> 

-- 
Brandon Williams