From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 26 Apr 2016 22:49:50 GMT
Subject: Re: [PATCH 03/15] submodule add: label submodules if asked to
Message-ID: <xmqq7ffkc7e9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1461703833-10350-4-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> When adding new submodules, you can specify the
> label(s) the submodule belongs to by giving one or more
> --label arguments. This will record each label in the
> .gitmodules file as a value of the key
> "submodule.$NAME.label".
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  Documentation/git-submodule.txt |  5 ++++-
>  git-submodule.sh                | 14 +++++++++++++-
>  t/t7400-submodule-basic.sh      | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8c4bbe2..349ead8 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
>  SYNOPSIS
>  --------
>  [verse]
> -'git submodule' [--quiet] add [-b <branch>] [-f|--force] [--name <name>]
> +'git submodule' [--quiet] add [-b <branch>] [-f|--force] [-l|--label <label>]
>  	      [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
> @@ -109,6 +109,9 @@ is the superproject and submodule repositories will be kept
>  together in the same relative location, and only the
>  superproject's URL needs to be provided: git-submodule will correctly
>  locate the submodule using the relative URL in .gitmodules.
> ++
> +If at least one label argument was given, all labels are recorded in the
> +.gitmodules file in the label fields.

I think this is better without "If ... given,".

I am not sure if it is sensible to make "label" namespace always
global to be shared with the project by updating .gitmodules,
though (it can cut both ways, so this is not an objection).

> @@ -165,6 +166,13 @@ cmd_add()
>  		--depth=*)
>  			depth=$1
>  			;;
> +		-l|--label)
> +			labels="${labels} $2"
> +			shift
> +			;;
> +		--label=*)
> +			labels="${labels} ${1#--label=}"
> +			;;
>  		--)
>  			shift
>  			break
> @@ -292,6 +300,10 @@ Use -f if you really want to add it." >&2
>  
>  	git config -f .gitmodules submodule."$sm_name".path "$sm_path" &&
>  	git config -f .gitmodules submodule."$sm_name".url "$repo" &&
> +	for label in $labels
> +	do
> +		git config --add -f .gitmodules submodule."$sm_name".label "${label}"
> +	done &&

Is the acceptable syntax for "label" defined and documented
somewhere?

I didn't see it in the documentation patch.  I am seeing that we do
not allow $IFS whitespaces in them, but are there other restrictions
we want to enforce?  Remember, starting with narrow and widening as
we discover the need is the right way to design things.  Once we
start allowing random strings, it is very hard to later reject some
to carve out namespace for ourselves.

The above implementation happens to allow users to say

	git submodule add -l "labelA labelB" -- $there $path

and give two labels to the module, and that will be something you
end up needing to support forever, unless you restrict early.