From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 20 Jan 2016 21:23:16 GMT
Subject: Re: [PATCH 2/4] submodule-config: keep groups around
Message-ID: <xmqqwpr43pyz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1453260880-628-3-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> We need the submodule groups in a later patch.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  submodule-config.c | 13 +++++++++++++
>  submodule-config.h |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index a32259e..b5453d0 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -60,6 +60,8 @@ static void free_one_config(struct submodule_entry *entry)
>  {
>  	free((void *) entry->config->path);
>  	free((void *) entry->config->name);
> +	if (entry->config->groups)
> +		string_list_clear(entry->config->groups, 0);

You are allocating entry->config->groups itself in the in the hunk
around ll.327, so that also need to be freed, I would think.

>  	free(entry->config);
>  }
>  
> @@ -184,6 +186,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>  	submodule->update = NULL;
>  	submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>  	submodule->ignore = NULL;
> +	submodule->groups = NULL;
>  
>  	hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
>  
> @@ -324,6 +327,16 @@ static int parse_specific_submodule_config(const char *subsection, int subsectio
>  			free((void *) submodule->update);
>  			submodule->update = xstrdup(value);
>  		}
> +	} else if (!strcmp(key, "group")) {
> +		if (!value)
> +			ret = config_error_nonbool(var);
> +		else {
> +			if (!submodule->groups) {
> +				submodule->groups = xmalloc(sizeof(*submodule->groups));
> +				string_list_init(submodule->groups, 1);
> +			}
> +			string_list_insert(submodule->groups, value);
> +		}
>  	}
>  
>  	return ret;
> diff --git a/submodule-config.h b/submodule-config.h
> index d9bbf9a..332f5be 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -17,6 +17,7 @@ struct submodule {
>  	const char *update;
>  	/* the sha1 blob id of the responsible .gitmodules file */
>  	unsigned char gitmodules_sha1[20];
> +	struct string_list *groups;
>  };

Is there a case where you need to enumerate and show the groups a
submodule belongs to to the end users?  Using string_list_insert()
to manage this list would mean you will lose the original ordering
you saw the list of groups in their .gitmodules files, which may or
may not matter.  I'd assume that the ordering should not matter, but
that is something the user may want to see documented.

>  
>  int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);