From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 22 Mar 2016 22:28:03 GMT
Subject: Re: [RFC_PATCHv4 1/7] git submodule: teach `add` to label submodules
Message-ID: <xmqqio0ef8r0.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1458612372-10966-2-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".

Can you define the semantics of "label" with a bit more detail in
the doc?  For example, it is unclear from the description if a
submodule can have multiple labels (I can read the patch, and I am
not asking you to explain it to _me_).  It also is unclear if a user
can tweak the set of labels a submodule has locally, iow, without
modifying .gitmodules that would end up affecting other people.

I think the expected use of "label" is to have the whole project to
share which labels a given submodule should have (i.e. it is not
like branches you can freely create in your repository, but more
like tags that give all project participant shared understanding of
stable anchoring points), and I am guessing that there is no need
for "repository local labels" for that reason, but we'd need to see
that kind of design decisions spelled out in the documentation to
avoid end-user confusion.

Thanks.

> 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 13adebf..c0744eb 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>...]
> @@ -101,6 +101,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.
>  
>  status::
>  	Show the status of the submodules. This will print the SHA-1 of the
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 97a3097..def1e1c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -5,7 +5,7 @@
>  # Copyright (c) 2007 Lars Hjemli
>  
>  dashless=$(basename "$0" | sed -e 's/-/ /')
> -USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
> +USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-l|--label <label>][--] <repository> [<path>]
>     or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] init [--] [<path>...]
>     or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
> @@ -130,6 +130,7 @@ cmd_add()
>  {
>  	# parse $args after "submodule ... add".
>  	reference_path=
> +	labels=
>  	while test $# -ne 0
>  	do
>  		case "$1" in
> @@ -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 &&
>  	if test -n "$branch"
>  	then
>  		git config -f .gitmodules submodule."$sm_name".branch "$branch"
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 5991e3c..fc948fd 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -986,6 +986,7 @@ test_expect_success 'submodule with UTF-8 name' '
>  '
>  
>  test_expect_success 'submodule add clone shallow submodule' '
> +	test_when_finished "rm -rf super" &&
>  	mkdir super &&
>  	pwd=$(pwd) &&
>  	(
> @@ -999,5 +1000,36 @@ test_expect_success 'submodule add clone shallow submodule' '
>  	)
>  '
>  
> +test_expect_success 'submodule add records a label' '
> +	test_when_finished "rm -rf super" &&
> +	mkdir super &&
> +	pwd=$(pwd) &&
> +	(
> +		cd super &&
> +		git init &&
> +		git submodule add --label labelA file://"$pwd"/example2 submodule &&
> +		git config -f .gitmodules submodule."submodule".label >actual &&
> +		echo labelA >expected &&
> +		test_cmp expected actual
> +	)
> +'
> +
> +cat >expected <<-EOF
> +labelA
> +labelB
> +EOF
> +
> +test_expect_success 'submodule add records multiple labels' '
> +	test_when_finished "rm -rf super" &&
> +	mkdir super &&
> +	pwd=$(pwd) &&
> +	(
> +		cd super &&
> +		git init &&
> +		git submodule add --label=labelA -l labelB file://"$pwd"/example2 submodule &&
> +		git config --get-all -f .gitmodules submodule."submodule".label >../actual
> +	) &&
> +	test_cmp expected actual
> +'
>  
>  test_done