[PATCH 0/3] Teach submodule set-branch subcommand

[PATCH 0/3] Teach submodule set-branch subcommand

[Denton Liu]

Currently, there is no way to set the branch of a submodule without
manually manipulating the .gitmodules file. This patchset introduces a
porcelain command that enables this.


Denton Liu (3):
  git-submodule.txt: document default behavior without --branch
  submodule--helper: teach config subcommand --unset
  submodule: teach set-branch subcommand

 Documentation/git-submodule.txt        | 10 ++-
 builtin/submodule--helper.c            | 15 +++--
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7411-submodule-config.sh            |  9 +++
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 6 files changed, 197 insertions(+), 10 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

-- 
2.20.1.522.g5f42c252e9

[PATCH 1/3] git-submodule.txt: document default behavior without --branch

[Denton Liu]

This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-submodule.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..9951c68744 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -261,7 +261,8 @@ OPTIONS
 	The name of the branch is recorded as `submodule.<name>.branch` in
 	`.gitmodules` for `update --remote`.  A special value of `.` is used to
 	indicate that the name of the branch in the submodule should be the
-	same name as the current branch in the current repository.
+	same name as the current branch in the current repository.  If the
+	option is not specified, it defaults to 'master'.
 
 -f::
 --force::
-- 
2.20.1.522.g5f42c252e9

[PATCH 2/3] submodule--helper: teach config subcommand --unset

[Denton Liu]

This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/submodule--helper.c | 15 +++++++++++----
 t/t7411-submodule-config.sh |  9 +++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0e140f176c..336e4429e6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2149,15 +2149,21 @@ static int module_config(int argc, const char **argv, const char *prefix)
 	enum {
 		CHECK_WRITEABLE = 1
 	} command = 0;
+	enum {
+		DO_UNSET = 1
+	} unset = 0;
 
 	struct option module_config_options[] = {
 		OPT_CMDMODE(0, "check-writeable", &command,
 			    N_("check if it is safe to write to the .gitmodules file"),
 			    CHECK_WRITEABLE),
+		OPT_CMDMODE(0, "unset", &unset,
+			    N_("unset the config in the .gitmodules file"),
+			    DO_UNSET),
 		OPT_END()
 	};
 	const char *const git_submodule_helper_usage[] = {
-		N_("git submodule--helper config name [value]"),
+		N_("git submodule--helper config name [--unset] [value]"),
 		N_("git submodule--helper config --check-writeable"),
 		NULL
 	};
@@ -2169,15 +2175,16 @@ static int module_config(int argc, const char **argv, const char *prefix)
 		return is_writing_gitmodules_ok() ? 0 : -1;
 
 	/* Equivalent to ACTION_GET in builtin/config.c */
-	if (argc == 2)
+	if (argc == 2 && unset != DO_UNSET)
 		return print_config_from_gitmodules(the_repository, argv[1]);
 
 	/* Equivalent to ACTION_SET in builtin/config.c */
-	if (argc == 3) {
+	if (argc == 3 || (argc == 2 && unset == DO_UNSET)) {
 		if (!is_writing_gitmodules_ok())
 			die(_("please make sure that the .gitmodules file is in the working tree"));
 
-		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+		const char *value = (argc == 3) ? argv[2] : NULL;
+		return config_set_in_gitmodules_file_gently(argv[1], value);
 	}
 
 	usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
 	)
 '
 
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+	(cd super &&
+		git submodule--helper config --unset submodule.submodule.url &&
+		git submodule--helper config submodule.submodule.url >actual &&
+		test_must_be_empty actual
+	)
+'
+
+
 test_expect_success 'writing submodules config with "submodule--helper config"' '
 	(cd super &&
 		echo "new_url" >expect &&
-- 
2.20.1.522.g5f42c252e9

[PATCH 3/3] submodule: teach set-branch subcommand

[Denton Liu]

This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
 Documentation/git-submodule.txt        |  7 ++
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 5 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 9951c68744..cb838ee556 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
 'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
 'git submodule' [--quiet] summary [<options>] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 --
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+	Sets the default remote tracking branch for the submodule. The
+	`--branch` option allows the remote branch to be specified. The
+	`--default` option removes the submodule.<name>.branch configuration
+	key, which causes the tracking branch to default to 'master'.
+--
 summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..8b3b5a9d34 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="add status init deinit update summary foreach sync"
+	local subcommands="add status init deinit update set-branch summary foreach sync"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
 			--force --rebase --merge --reference --depth --recursive --jobs
 		"
 		;;
+	set-branch,--*)
+		__gitcomp "--default --branch"
+		;;
 	summary,--*)
 		__gitcomp "--cached --files --summary-limit"
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..43ec756602 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
 	}
 }
 
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+	unset_branch=false
+	branch=
+
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			# we don't do anything with this but we need to accept it
+			;;
+		-d|--default)
+			unset_branch=true
+			;;
+		-b|--branch)
+			case "$2" in '') usage ;; esac
+			branch=$2
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# -ne 1
+	then
+		usage
+	fi
+
+	# we can't use `git submodule--helper name` here because internally, it
+	# hashes the path so a trailing slash could lead to an unintentional no match
+	name="$(git submodule--helper list "$1" | cut -f2)"
+	if test -z "$name"
+	then
+		exit 1
+	fi
+
+	test -n "$branch"; has_branch=$?
+	test "$unset_branch" = true; has_unset_branch=$?
+
+	if test $((!$has_branch != !$has_unset_branch)) -eq 0
+	then
+		usage
+	fi
+
+	if test $has_branch -eq 0
+	then
+		git submodule--helper config submodule."$name".branch "$branch"
+	else
+		git submodule--helper config --unset submodule."$name".branch
+	fi
+}
+
 #
 # Show commit summary for submodules in index or working tree
 #
@@ -980,7 +1047,7 @@ cmd_absorbgitdirs()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+	add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
@@ -1021,8 +1088,8 @@ then
     fi
 fi
 
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
 then
 	usage
 fi
@@ -1033,4 +1100,4 @@ then
 	usage
 fi
 
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+	mkdir submodule &&
+	(cd submodule &&
+		git init &&
+		echo a >a &&
+		git add . &&
+		git commit -ma &&
+		git checkout -b topic &&
+		echo b >a &&
+		git add . &&
+		git commit -mb
+	) &&
+	mkdir super &&
+	(cd super &&
+		git init &&
+		git submodule add ../submodule &&
+		git commit -m "add submodule"
+	)
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+	(cd super &&
+		test_must_fail grep branch .gitmodules
+	)
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+	(cd super &&
+		git submodule set-branch --branch topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch --default' '
+	(cd super &&
+		git submodule set-branch --default submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -b' '
+	(cd super &&
+		git submodule set-branch -b topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -d' '
+	(cd super &&
+		git submodule set-branch -d submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.20.1.522.g5f42c252e9

Re: [PATCH 1/3] git-submodule.txt: document default behavior without --branch

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> This behavior is mentioned in gitmodules.txt but not in
> git-submodule.txt so we copy the information over so that it is not
> missed.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  Documentation/git-submodule.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index ba3c4df550..9951c68744 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -261,7 +261,8 @@ OPTIONS
>  	The name of the branch is recorded as `submodule.<name>.branch` in
>  	`.gitmodules` for `update --remote`.  A special value of `.` is used to
>  	indicate that the name of the branch in the submodule should be the
> -	same name as the current branch in the current repository.
> +	same name as the current branch in the current repository.  If the
> +	option is not specified, it defaults to 'master'.

OK.  Thanks.

I read the whole page and there are a few other things I noticed,
which I'll write down here so that somebody else can think about
them (and that somebody else does not have to be you) as a tangent.

1. The description of "--branch <branch>" option here is missing the
   "<branch>", a mandatory argument to the option.  Options in the
   OPTIONS list like "--jobs <n>", "--repository <repo>" do show the
   argument, and the description for this option should be updated
   to match.

2. Some options are mentioned in the description of each subcommand
   it can appear (e.g. "sync --recursive"), while many others are
   not and shown only in the OPTIONS section (e.g. "--remote" which
   can be used only with "update" subcommand).

These may want to be cleaned up.  Actually it may not be a bad idea
to do #1 as part of this change (i.e. retitle it to "git-submodule:
"--branch <branch>" option defaults to 'master'" or something).

Re: [PATCH 2/3] submodule--helper: teach config subcommand --unset

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> This teaches submodule--helper config the --unset option, which removes
> the specified configuration key from the .gitmodule file.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/submodule--helper.c | 15 +++++++++++----
>  t/t7411-submodule-config.sh |  9 +++++++++
>  2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 0e140f176c..336e4429e6 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2149,15 +2149,21 @@ static int module_config(int argc, const char **argv, const char *prefix)
>  	enum {
>  		CHECK_WRITEABLE = 1
>  	} command = 0;
> +	enum {
> +		DO_UNSET = 1
> +	} unset = 0;
>  
>  	struct option module_config_options[] = {
>  		OPT_CMDMODE(0, "check-writeable", &command,
>  			    N_("check if it is safe to write to the .gitmodules file"),
>  			    CHECK_WRITEABLE),
> +		OPT_CMDMODE(0, "unset", &unset,
> +			    N_("unset the config in the .gitmodules file"),
> +			    DO_UNSET),
>  		OPT_END()
>  	};

The way this patch uses OPT_CMDMODE() is wrong.

The situation in which CMDMODE is meant to be used is that there are
multiple (that's two or more) choices the end user can make, and the
end user can choose only one of them at a time, iow, giving more
than one is an error.  

In such a case, the programmer would

 - prepare a single variable to store the single choice the end user
   makes the choice in and initialize it to zero.

 - have one OPT_CMDMODE() element for each valid choice, all
   pointing at the same variable, but with different value that is
   not zero.

The parse_options() call would then make sure that the variable is
set to non-zero value only once, to detect conflicting command modes
given from the command line.  The program then can read from the
single variable to see if the end user made any choice (or left it
0).

An example of typical and good use of OPT_CMDMODE() mechanism can be
seen in builtin/tag.c; the 'l(ist)', 'd(elete)' and 'v(erify)' are
the distinct operating modes of the "git tag" command (i.e. you do
not delete a tag while listing them), so the &cmdmode variable is
used to ensure only one of them (or none of them) is given.

The existing use of OPT_CMDMODE we see in this function anticipates
that there would be new operating modes added other than the
check-writable mode, so if you are making a new command mode that
cannot be used with the existing check-writable, then the right way
to use the OPT_CMDMODE() is to add to the command enum a new
non-zero value distinct from CHECK_WRITABLE, without introducing a
new "unset" variable.

If this --unset thing is truly independent from --check-writable,
i.e. if all four possible combinations of having and not having
these two options are valid, then you would not want to muck with
the &command variable, but in that case, wouldn't it make more sense
for the new --unset thing to simply be using OPT_BOOL()?  After all,
it is not like you are planning to add new oprating modes in the
"unset" family that is not "--unset" in the future, no?

[PATCH v2 0/3] Teach submodule set-branch subcommand

[Denton Liu]

Currently, there is no way to set the branch of a submodule without
manually manipulating the .gitmodules file. This patchset introduces a
porcelain command that enables this.

Changes since v1:
	* Fixed incorrect usage of OPT_CMDMODE


Denton Liu (3):
  git-submodule.txt: document default behavior without --branch
  submodule--helper: teach config subcommand --unset
  submodule: teach set-branch subcommand

 Documentation/git-submodule.txt        | 10 ++-
 builtin/submodule--helper.c            | 18 +++--
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7411-submodule-config.sh            |  9 +++
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 6 files changed, 198 insertions(+), 12 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

-- 
2.20.1.522.g5f42c252e9

[PATCH v2 1/3] git-submodule.txt: document default behavior without --branch

[Denton Liu]

This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-submodule.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..9951c68744 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -261,7 +261,8 @@ OPTIONS
 	The name of the branch is recorded as `submodule.<name>.branch` in
 	`.gitmodules` for `update --remote`.  A special value of `.` is used to
 	indicate that the name of the branch in the submodule should be the
-	same name as the current branch in the current repository.
+	same name as the current branch in the current repository.  If the
+	option is not specified, it defaults to 'master'.
 
 -f::
 --force::
-- 
2.20.1.522.g5f42c252e9

[PATCH v2 2/3] submodule--helper: teach config subcommand --unset

[Denton Liu]

This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/submodule--helper.c | 18 ++++++++++++------
 t/t7411-submodule-config.sh |  9 +++++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0e140f176c..5a4a7b1993 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2147,17 +2147,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
 static int module_config(int argc, const char **argv, const char *prefix)
 {
 	enum {
-		CHECK_WRITEABLE = 1
-	} command = 0;
+		NONE,
+		CHECK_WRITEABLE,
+		DO_UNSET
+	} command = NONE;
 
 	struct option module_config_options[] = {
 		OPT_CMDMODE(0, "check-writeable", &command,
 			    N_("check if it is safe to write to the .gitmodules file"),
 			    CHECK_WRITEABLE),
+		OPT_CMDMODE(0, "unset", &command,
+			    N_("unset the config in the .gitmodules file"),
+			    DO_UNSET),
 		OPT_END()
 	};
 	const char *const git_submodule_helper_usage[] = {
-		N_("git submodule--helper config name [value]"),
+		N_("git submodule--helper config name [--unset] [value]"),
 		N_("git submodule--helper config --check-writeable"),
 		NULL
 	};
@@ -2169,15 +2174,16 @@ static int module_config(int argc, const char **argv, const char *prefix)
 		return is_writing_gitmodules_ok() ? 0 : -1;
 
 	/* Equivalent to ACTION_GET in builtin/config.c */
-	if (argc == 2)
+	if (argc == 2 && command != DO_UNSET)
 		return print_config_from_gitmodules(the_repository, argv[1]);
 
 	/* Equivalent to ACTION_SET in builtin/config.c */
-	if (argc == 3) {
+	if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
 		if (!is_writing_gitmodules_ok())
 			die(_("please make sure that the .gitmodules file is in the working tree"));
 
-		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+		const char *value = (argc == 3) ? argv[2] : NULL;
+		return config_set_in_gitmodules_file_gently(argv[1], value);
 	}
 
 	usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
 	)
 '
 
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+	(cd super &&
+		git submodule--helper config --unset submodule.submodule.url &&
+		git submodule--helper config submodule.submodule.url >actual &&
+		test_must_be_empty actual
+	)
+'
+
+
 test_expect_success 'writing submodules config with "submodule--helper config"' '
 	(cd super &&
 		echo "new_url" >expect &&
-- 
2.20.1.522.g5f42c252e9

[PATCH v2 3/3] submodule: teach set-branch subcommand

[Denton Liu]

This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
 Documentation/git-submodule.txt        |  7 ++
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 5 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 9951c68744..cb838ee556 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
 'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
 'git submodule' [--quiet] summary [<options>] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 --
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+	Sets the default remote tracking branch for the submodule. The
+	`--branch` option allows the remote branch to be specified. The
+	`--default` option removes the submodule.<name>.branch configuration
+	key, which causes the tracking branch to default to 'master'.
+--
 summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..8b3b5a9d34 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="add status init deinit update summary foreach sync"
+	local subcommands="add status init deinit update set-branch summary foreach sync"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
 			--force --rebase --merge --reference --depth --recursive --jobs
 		"
 		;;
+	set-branch,--*)
+		__gitcomp "--default --branch"
+		;;
 	summary,--*)
 		__gitcomp "--cached --files --summary-limit"
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..43ec756602 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
 	}
 }
 
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+	unset_branch=false
+	branch=
+
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			# we don't do anything with this but we need to accept it
+			;;
+		-d|--default)
+			unset_branch=true
+			;;
+		-b|--branch)
+			case "$2" in '') usage ;; esac
+			branch=$2
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# -ne 1
+	then
+		usage
+	fi
+
+	# we can't use `git submodule--helper name` here because internally, it
+	# hashes the path so a trailing slash could lead to an unintentional no match
+	name="$(git submodule--helper list "$1" | cut -f2)"
+	if test -z "$name"
+	then
+		exit 1
+	fi
+
+	test -n "$branch"; has_branch=$?
+	test "$unset_branch" = true; has_unset_branch=$?
+
+	if test $((!$has_branch != !$has_unset_branch)) -eq 0
+	then
+		usage
+	fi
+
+	if test $has_branch -eq 0
+	then
+		git submodule--helper config submodule."$name".branch "$branch"
+	else
+		git submodule--helper config --unset submodule."$name".branch
+	fi
+}
+
 #
 # Show commit summary for submodules in index or working tree
 #
@@ -980,7 +1047,7 @@ cmd_absorbgitdirs()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+	add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
@@ -1021,8 +1088,8 @@ then
     fi
 fi
 
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
 then
 	usage
 fi
@@ -1033,4 +1100,4 @@ then
 	usage
 fi
 
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+	mkdir submodule &&
+	(cd submodule &&
+		git init &&
+		echo a >a &&
+		git add . &&
+		git commit -ma &&
+		git checkout -b topic &&
+		echo b >a &&
+		git add . &&
+		git commit -mb
+	) &&
+	mkdir super &&
+	(cd super &&
+		git init &&
+		git submodule add ../submodule &&
+		git commit -m "add submodule"
+	)
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+	(cd super &&
+		test_must_fail grep branch .gitmodules
+	)
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+	(cd super &&
+		git submodule set-branch --branch topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch --default' '
+	(cd super &&
+		git submodule set-branch --default submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -b' '
+	(cd super &&
+		git submodule set-branch -b topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -d' '
+	(cd super &&
+		git submodule set-branch -d submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.20.1.522.g5f42c252e9

[PATCH v3 0/3] Teach submodule set-branch subcommand

[Denton Liu]

I rebased the changes onto the latest 'next' because if this branch gets
merged into 'next', there'll be merge conflicts from
'dl/complete-submodule-absorbgitdirs'.

Currently, there is no way to set the branch of a submodule without
manually manipulating the .gitmodules file. This patchset introduces a
porcelain command that enables this.

Changes since v1:
	* Fixed incorrect usage of OPT_CMDMODE

Changes since v2:
	* Corrected missing argument for -b/--branch in git-submodule.txt
	* Rebased onto latest next


Denton Liu (3):
  git-submodule.txt: "--branch <branch>" option defaults to 'master'
  submodule--helper: teach config subcommand --unset
  submodule: teach set-branch subcommand

 Documentation/git-submodule.txt        | 14 +++-
 builtin/submodule--helper.c            | 18 +++--
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7411-submodule-config.sh            |  9 +++
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 6 files changed, 200 insertions(+), 14 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

-- 
2.20.1.522.g5f42c252e9

[PATCH v3 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master'

[Denton Liu]

This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.

Also, add the missed argument to the -b/--branch option.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-submodule.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..4150148fa3 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -255,13 +255,14 @@ OPTIONS
 	This option is only valid for the deinit command. Unregister all
 	submodules in the working tree.
 
--b::
---branch::
+-b <branch>::
+--branch <branch>::
 	Branch of repository to add as submodule.
 	The name of the branch is recorded as `submodule.<name>.branch` in
 	`.gitmodules` for `update --remote`.  A special value of `.` is used to
 	indicate that the name of the branch in the submodule should be the
-	same name as the current branch in the current repository.
+	same name as the current branch in the current repository.  If the
+	option is not specified, it defaults to 'master'.
 
 -f::
 --force::
-- 
2.20.1.522.g5f42c252e9

[PATCH v3 2/3] submodule--helper: teach config subcommand --unset

[Denton Liu]

This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/submodule--helper.c | 18 ++++++++++++------
 t/t7411-submodule-config.sh |  9 +++++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b80fc4ba3d..a86eacf167 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2148,17 +2148,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
 static int module_config(int argc, const char **argv, const char *prefix)
 {
 	enum {
-		CHECK_WRITEABLE = 1
-	} command = 0;
+		NONE,
+		CHECK_WRITEABLE,
+		DO_UNSET
+	} command = NONE;
 
 	struct option module_config_options[] = {
 		OPT_CMDMODE(0, "check-writeable", &command,
 			    N_("check if it is safe to write to the .gitmodules file"),
 			    CHECK_WRITEABLE),
+		OPT_CMDMODE(0, "unset", &command,
+			    N_("unset the config in the .gitmodules file"),
+			    DO_UNSET),
 		OPT_END()
 	};
 	const char *const git_submodule_helper_usage[] = {
-		N_("git submodule--helper config name [value]"),
+		N_("git submodule--helper config name [--unset] [value]"),
 		N_("git submodule--helper config --check-writeable"),
 		NULL
 	};
@@ -2170,15 +2175,16 @@ static int module_config(int argc, const char **argv, const char *prefix)
 		return is_writing_gitmodules_ok() ? 0 : -1;
 
 	/* Equivalent to ACTION_GET in builtin/config.c */
-	if (argc == 2)
+	if (argc == 2 && command != DO_UNSET)
 		return print_config_from_gitmodules(the_repository, argv[1]);
 
 	/* Equivalent to ACTION_SET in builtin/config.c */
-	if (argc == 3) {
+	if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
 		if (!is_writing_gitmodules_ok())
 			die(_("please make sure that the .gitmodules file is in the working tree"));
 
-		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+		const char *value = (argc == 3) ? argv[2] : NULL;
+		return config_set_in_gitmodules_file_gently(argv[1], value);
 	}
 
 	usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
 	)
 '
 
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+	(cd super &&
+		git submodule--helper config --unset submodule.submodule.url &&
+		git submodule--helper config submodule.submodule.url >actual &&
+		test_must_be_empty actual
+	)
+'
+
+
 test_expect_success 'writing submodules config with "submodule--helper config"' '
 	(cd super &&
 		echo "new_url" >expect &&
-- 
2.20.1.522.g5f42c252e9

[PATCH v3 3/3] submodule: teach set-branch subcommand

[Denton Liu]

This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
 Documentation/git-submodule.txt        |  7 ++
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 5 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4150148fa3..6c608b3b8c 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
 'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
 'git submodule' [--quiet] summary [<options>] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 --
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+	Sets the default remote tracking branch for the submodule. The
+	`--branch` option allows the remote branch to be specified. The
+	`--default` option removes the submodule.<name>.branch configuration
+	key, which causes the tracking branch to default to 'master'.
+--
 summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index de56879960..0fccadfc97 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="add status init deinit update summary foreach sync absorbgitdirs"
+	local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
 			--force --rebase --merge --reference --depth --recursive --jobs
 		"
 		;;
+	set-branch,--*)
+		__gitcomp "--default --branch"
+		;;
 	summary,--*)
 		__gitcomp "--cached --files --summary-limit"
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index b5f2beee60..470f681573 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
 	}
 }
 
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+	unset_branch=false
+	branch=
+
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			# we don't do anything with this but we need to accept it
+			;;
+		-d|--default)
+			unset_branch=true
+			;;
+		-b|--branch)
+			case "$2" in '') usage ;; esac
+			branch=$2
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# -ne 1
+	then
+		usage
+	fi
+
+	# we can't use `git submodule--helper name` here because internally, it
+	# hashes the path so a trailing slash could lead to an unintentional no match
+	name="$(git submodule--helper list "$1" | cut -f2)"
+	if test -z "$name"
+	then
+		exit 1
+	fi
+
+	test -n "$branch"; has_branch=$?
+	test "$unset_branch" = true; has_unset_branch=$?
+
+	if test $((!$has_branch != !$has_unset_branch)) -eq 0
+	then
+		usage
+	fi
+
+	if test $has_branch -eq 0
+	then
+		git submodule--helper config submodule."$name".branch "$branch"
+	else
+		git submodule--helper config --unset submodule."$name".branch
+	fi
+}
+
 #
 # Show commit summary for submodules in index or working tree
 #
@@ -983,7 +1050,7 @@ cmd_absorbgitdirs()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+	add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
@@ -1024,8 +1091,8 @@ then
     fi
 fi
 
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
 then
 	usage
 fi
@@ -1036,4 +1103,4 @@ then
 	usage
 fi
 
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+	mkdir submodule &&
+	(cd submodule &&
+		git init &&
+		echo a >a &&
+		git add . &&
+		git commit -ma &&
+		git checkout -b topic &&
+		echo b >a &&
+		git add . &&
+		git commit -mb
+	) &&
+	mkdir super &&
+	(cd super &&
+		git init &&
+		git submodule add ../submodule &&
+		git commit -m "add submodule"
+	)
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+	(cd super &&
+		test_must_fail grep branch .gitmodules
+	)
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+	(cd super &&
+		git submodule set-branch --branch topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch --default' '
+	(cd super &&
+		git submodule set-branch --default submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -b' '
+	(cd super &&
+		git submodule set-branch -b topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -d' '
+	(cd super &&
+		git submodule set-branch -d submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.20.1.522.g5f42c252e9

Re: [PATCH v3 0/3] Teach submodule set-branch subcommand

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> I rebased the changes onto the latest 'next' because if this branch gets
> merged into 'next', there'll be merge conflicts from
> 'dl/complete-submodule-absorbgitdirs'.

Please don't do that.

A topic that depends on everything in 'next' cannot graduate to
'master' until everything that is cooking in 'next' does.

When

 - the "conflict" that would arise is so trivial to resolve,

 - there is no semantic crashes between the new topic and existing
    ones, and

 - the topic is usable even before other topics graduate (or even
   when they get discarded)

please make it a habit to avoid making your topic (i.e. this one)
hostage to another topic (i.e. the absorbgitdirs one), and certainly
not hostage to the whole of 'next'.  A trivial conflict resolution
in this case, if you revert the rebasing and then attempt to merge
the result to 'next', would look like this.

diff --cc contrib/completion/git-completion.bash
index 8b3b5a9d34,de56879960..0000000000
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@@ -2573,7 -2573,7 +2573,7 @@@ _git_submodule (
  {
  	__git_has_doubledash && return
  
- 	local subcommands="add status init deinit update set-branch summary foreach sync"
 -	local subcommands="add status init deinit update summary foreach sync absorbgitdirs"
++	local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
  	if [ -z "$subcommand" ]; then
  		case "$cur" in


By the way, if conflicts worry you so much, one thing you could do
is this.  Instead of maintaining the ever-growing list of
subcommands and having to worry about textual conflicts, the
completion script could use a clean-up to reduce the need of textual
conflict resolution, when it is quiescent, perhaps like this (I am
assuming that in some future, there is a quiescent period _after_
both of these two topics landed, and this illustration patch is to
be used in such a future).

Merging two topics, each of which adds a new element by inserting a
new line with the element (and the element alone) is on it, is
certainly a lot easier and simpler than having to see what word is
getting inserted by each topic on a single long string on a line.


 contrib/completion/git-completion.bash | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0fccadfc97..5d7d4ebacc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,18 @@ _git_submodule ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="add status init deinit update set-branch summary foreach sync absorbgitdirs"
+	local subcommands="
+		add
+		status
+		init
+		deinit
+		update
+		set-branch
+		summary
+		foreach
+		sync
+		absorbgitdirs
+	"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in

Re: [PATCH v3 2/3] submodule--helper: teach config subcommand --unset

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> This teaches submodule--helper config the --unset option, which removes
> the specified configuration key from the .gitmodule file.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/submodule--helper.c | 18 ++++++++++++------
>  t/t7411-submodule-config.sh |  9 +++++++++
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b80fc4ba3d..a86eacf167 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2148,17 +2148,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
>  static int module_config(int argc, const char **argv, const char *prefix)
>  {
>  	enum {
> -		CHECK_WRITEABLE = 1
> -	} command = 0;
> +		NONE,
> +		CHECK_WRITEABLE,
> +		DO_UNSET
> +	} command = NONE;

I do not think the above, except for addition of DO_UNSET, is a good
change.  The language spec may guarantee that NONE is 0, but in the
way the original is written, it is much more obvious that integer
zero is a special value and the variable is initialized to that
special value, and that is important.  The above rewrite makes it
look as if there are a bunch of symbolic constants defined by this
particular caller and one random value NONE, whose only significance
is that it is different from any other value, is picked as its
initial value---but that is a wrong impression to give to the
readers.  parse-options.c::get_value() special cases integer zero as
"unset" for any CMDMODE, so inventing a symbolic constant used by
this particular user is counter-productive.  Needless to say, it is
not such a great idea to use such an overly generic word NONE here.

The way the original did to make sure all enum values are non-zero
(by explicitly documenting that its first value is 1) and used
literal 0 as "not specified" is much better aligned to the way how
OPT_CMDMODE() is to be used, I think.

>  
>  	struct option module_config_options[] = {
>  		OPT_CMDMODE(0, "check-writeable", &command,
>  			    N_("check if it is safe to write to the .gitmodules file"),
>  			    CHECK_WRITEABLE),
> +		OPT_CMDMODE(0, "unset", &command,
> +			    N_("unset the config in the .gitmodules file"),
> +			    DO_UNSET),
>  		OPT_END()
>  	};
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper config name [value]"),
> +		N_("git submodule--helper config name [--unset] [value]"),

That gives an impression that "config name --unset value" is a valid
input; isn't it more like "you can unset, or you can set to a
value"?  The way to spell it would be "... config name [--unset | value]"
but it raises a larger question.  What if you want to set to a value
that is a string "--unset"?  Actually, allowing an option that comes
after "name" (which is an arbitrary end-user supplied thing) is one
thing, but writing it in the documentation as if we are encouraging
it is probably not a good idea.  Shouldn't it be "config --unset name"?

In any case, seeing the new test in 7411, I think the best way to
write the above usage text would be to add one new line without
mucking with the existing "show it, or set it to the given value"
and add

	git submodule--helper config --unset name

as a separate item to the list.

> diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
> index 89690b7adb..fcc0fb82d8 100755
> --- a/t/t7411-submodule-config.sh
> +++ b/t/t7411-submodule-config.sh
> @@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
>  	)
>  '
>  
> +test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
> +	(cd super &&
> +		git submodule--helper config --unset submodule.submodule.url &&
> +		git submodule--helper config submodule.submodule.url >actual &&

Re: [PATCH v3 3/3] submodule: teach set-branch subcommand

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> @@ -168,6 +169,12 @@ submodule with the `--init` option.
>  If `--recursive` is specified, this command will recurse into the
>  registered submodules, and update any nested submodules within.
>  --
> +set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
> +	Sets the default remote tracking branch for the submodule. The
> +	`--branch` option allows the remote branch to be specified. The
> +	`--default` option removes the submodule.<name>.branch configuration
> +	key, which causes the tracking branch to default to 'master'.
> +--
>  summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
>  	Show commit summary between the given commit (defaults to HEAD) and
>  	working tree/index. For a submodule in question, a series of commits

The double-dash line you added must be replaced with an empty line.
The existing double-dash is *not* a separator between the previous
entry (i.e. update) and the next entry (i.e. summary).  Rather, the
body for 'update' is enclosed in a pair of double-dash lines as it
has many paragraphs.

Re: [PATCH v3 2/3] submodule--helper: teach config subcommand --unset

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> +	if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
>  		if (!is_writing_gitmodules_ok())
>  			die(_("please make sure that the .gitmodules file is in the working tree"));
>  
> -		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
> +		const char *value = (argc == 3) ? argv[2] : NULL;

This introduces decl-after-stmt.  Move it before the "is it OK to
write?" check.

> +		return config_set_in_gitmodules_file_gently(argv[1], value);
>  	}

Re: [PATCH v3 0/3] Teach submodule set-branch subcommand

[Denton Liu]

On Thu, Feb 07, 2019 at 10:01:40AM -0800, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > I rebased the changes onto the latest 'next' because if this branch gets
> > merged into 'next', there'll be merge conflicts from
> > 'dl/complete-submodule-absorbgitdirs'.
> 
> Please don't do that.
> 
> A topic that depends on everything in 'next' cannot graduate to
> 'master' until everything that is cooking in 'next' does.
> 
> When
> 
>  - the "conflict" that would arise is so trivial to resolve,
> 
>  - there is no semantic crashes between the new topic and existing
>     ones, and
> 
>  - the topic is usable even before other topics graduate (or even
>    when they get discarded)
> 
> please make it a habit to avoid making your topic (i.e. this one)
> hostage to another topic (i.e. the absorbgitdirs one), and certainly
> not hostage to the whole of 'next'.  A trivial conflict resolution
> in this case, if you revert the rebasing and then attempt to merge
> the result to 'next', would look like this.

My mistake, even though I've made a few contributions already, I'm still
getting used to the git.git workflow.

Thanks so much for helping me out and please don't hesitate to let me
know if I'm doing anything else wrong.

By the way, just for the record, how would you like me to handle
patchsets that cause merge conflicts?

Thanks,

Denton

[PATCH v4 0/3] Teach submodule set-branch subcommand

[Denton Liu]

Thanks so much for the feedback, Junio. I've updated the patchset based
on your comments.

Currently, there is no way to set the branch of a submodule without
manually manipulating the .gitmodules file. This patchset introduces a
porcelain command that enables this.

Changes since v1:
	* Fixed incorrect usage of OPT_CMDMODE

Changes since v2:
	* Corrected missing argument for -b/--branch in git-submodule.txt
	* Rebased onto latest next

Changes since v3:
	* Rebased back onto master
	* Address Junio's comments and improve clarity of submodule--helper patch
	* Fix incorrect documentation formatting in git-submodule.txt


Denton Liu (3):
  git-submodule.txt: "--branch <branch>" option defaults to 'master'
  submodule--helper: teach config subcommand --unset
  submodule: teach set-branch subcommand

 Documentation/git-submodule.txt        | 14 +++-
 builtin/submodule--helper.c            | 17 +++--
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7411-submodule-config.sh            |  9 +++
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 6 files changed, 200 insertions(+), 13 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

-- 
2.20.1.522.g5f42c252e9

[PATCH v4 1/3] git-submodule.txt: "--branch <branch>" option defaults to 'master'

[Denton Liu]

This behavior is mentioned in gitmodules.txt but not in
git-submodule.txt so we copy the information over so that it is not
missed.

Also, add the missed argument to the -b/--branch option.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-submodule.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ba3c4df550..4150148fa3 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -255,13 +255,14 @@ OPTIONS
 	This option is only valid for the deinit command. Unregister all
 	submodules in the working tree.
 
--b::
---branch::
+-b <branch>::
+--branch <branch>::
 	Branch of repository to add as submodule.
 	The name of the branch is recorded as `submodule.<name>.branch` in
 	`.gitmodules` for `update --remote`.  A special value of `.` is used to
 	indicate that the name of the branch in the submodule should be the
-	same name as the current branch in the current repository.
+	same name as the current branch in the current repository.  If the
+	option is not specified, it defaults to 'master'.
 
 -f::
 --force::
-- 
2.20.1.522.g5f42c252e9

[PATCH v4 2/3] submodule--helper: teach config subcommand --unset

[Denton Liu]

This teaches submodule--helper config the --unset option, which removes
the specified configuration key from the .gitmodule file.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/submodule--helper.c | 17 ++++++++++++-----
 t/t7411-submodule-config.sh |  9 +++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0e140f176c..449e56f30c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2147,17 +2147,22 @@ static int check_name(int argc, const char **argv, const char *prefix)
 static int module_config(int argc, const char **argv, const char *prefix)
 {
 	enum {
-		CHECK_WRITEABLE = 1
+		CHECK_WRITEABLE = 1,
+		DO_UNSET = 2
 	} command = 0;
 
 	struct option module_config_options[] = {
 		OPT_CMDMODE(0, "check-writeable", &command,
 			    N_("check if it is safe to write to the .gitmodules file"),
 			    CHECK_WRITEABLE),
+		OPT_CMDMODE(0, "unset", &command,
+			    N_("unset the config in the .gitmodules file"),
+			    DO_UNSET),
 		OPT_END()
 	};
 	const char *const git_submodule_helper_usage[] = {
-		N_("git submodule--helper config name [value]"),
+		N_("git submodule--helper config <name> [<value>]"),
+		N_("git submodule--helper config --unset <name>"),
 		N_("git submodule--helper config --check-writeable"),
 		NULL
 	};
@@ -2169,15 +2174,17 @@ static int module_config(int argc, const char **argv, const char *prefix)
 		return is_writing_gitmodules_ok() ? 0 : -1;
 
 	/* Equivalent to ACTION_GET in builtin/config.c */
-	if (argc == 2)
+	if (argc == 2 && command != DO_UNSET)
 		return print_config_from_gitmodules(the_repository, argv[1]);
 
 	/* Equivalent to ACTION_SET in builtin/config.c */
-	if (argc == 3) {
+	if (argc == 3 || (argc == 2 && command == DO_UNSET)) {
+		const char *value = (argc == 3) ? argv[2] : NULL;
+
 		if (!is_writing_gitmodules_ok())
 			die(_("please make sure that the .gitmodules file is in the working tree"));
 
-		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+		return config_set_in_gitmodules_file_gently(argv[1], value);
 	}
 
 	usage_with_options(git_submodule_helper_usage, module_config_options);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 89690b7adb..fcc0fb82d8 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -142,6 +142,15 @@ test_expect_success 'reading submodules config from the working tree with "submo
 	)
 '
 
+test_expect_success 'unsetting submodules config from the working tree with "submodule--helper config --unset"' '
+	(cd super &&
+		git submodule--helper config --unset submodule.submodule.url &&
+		git submodule--helper config submodule.submodule.url >actual &&
+		test_must_be_empty actual
+	)
+'
+
+
 test_expect_success 'writing submodules config with "submodule--helper config"' '
 	(cd super &&
 		echo "new_url" >expect &&
-- 
2.20.1.522.g5f42c252e9

[PATCH v4 3/3] submodule: teach set-branch subcommand

[Denton Liu]

This teaches git-submodule the set-branch subcommand which allows the
branch of a submodule to be set through a porcelain command without
having to manually manipulate the .gitmodules file.
---
 Documentation/git-submodule.txt        |  7 ++
 contrib/completion/git-completion.bash |  5 +-
 git-submodule.sh                       | 75 +++++++++++++++++++--
 t/t7419-submodule-set-branch.sh        | 93 ++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 5 deletions(-)
 create mode 100755 t/t7419-submodule-set-branch.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4150148fa3..4daf144180 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
 'git submodule' [--quiet] update [<options>] [--] [<path>...]
+'git submodule' [--quiet] set-branch [<options>] [--] <path>
 'git submodule' [--quiet] summary [<options>] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
@@ -168,6 +169,12 @@ submodule with the `--init` option.
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 --
+set-branch ((-d|--default)|(-b|--branch <branch>)) [--] <path>::
+	Sets the default remote tracking branch for the submodule. The
+	`--branch` option allows the remote branch to be specified. The
+	`--default` option removes the submodule.<name>.branch configuration
+	key, which causes the tracking branch to default to 'master'.
+
 summary [--cached|--files] [(-n|--summary-limit) <n>] [commit] [--] [<path>...]::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..8b3b5a9d34 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2573,7 +2573,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="add status init deinit update summary foreach sync"
+	local subcommands="add status init deinit update set-branch summary foreach sync"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
@@ -2604,6 +2604,9 @@ _git_submodule ()
 			--force --rebase --merge --reference --depth --recursive --jobs
 		"
 		;;
+	set-branch,--*)
+		__gitcomp "--default --branch"
+		;;
 	summary,--*)
 		__gitcomp "--cached --files --summary-limit"
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..43ec756602 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -10,6 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -684,6 +685,72 @@ cmd_update()
 	}
 }
 
+#
+# Configures a submodule's default branch
+#
+# $@ = requested path
+#
+cmd_set_branch() {
+	unset_branch=false
+	branch=
+
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			# we don't do anything with this but we need to accept it
+			;;
+		-d|--default)
+			unset_branch=true
+			;;
+		-b|--branch)
+			case "$2" in '') usage ;; esac
+			branch=$2
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# -ne 1
+	then
+		usage
+	fi
+
+	# we can't use `git submodule--helper name` here because internally, it
+	# hashes the path so a trailing slash could lead to an unintentional no match
+	name="$(git submodule--helper list "$1" | cut -f2)"
+	if test -z "$name"
+	then
+		exit 1
+	fi
+
+	test -n "$branch"; has_branch=$?
+	test "$unset_branch" = true; has_unset_branch=$?
+
+	if test $((!$has_branch != !$has_unset_branch)) -eq 0
+	then
+		usage
+	fi
+
+	if test $has_branch -eq 0
+	then
+		git submodule--helper config submodule."$name".branch "$branch"
+	else
+		git submodule--helper config --unset submodule."$name".branch
+	fi
+}
+
 #
 # Show commit summary for submodules in index or working tree
 #
@@ -980,7 +1047,7 @@ cmd_absorbgitdirs()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
+	add | foreach | init | deinit | update | set-branch | status | summary | sync | absorbgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
@@ -1021,8 +1088,8 @@ then
     fi
 fi
 
-# "-b branch" is accepted only by "add"
-if test -n "$branch" && test "$command" != add
+# "-b branch" is accepted only by "add" and "set-branch"
+if test -n "$branch" && (test "$command" != add || test "$command" != set-branch)
 then
 	usage
 fi
@@ -1033,4 +1100,4 @@ then
 	usage
 fi
 
-"cmd_$command" "$@"
+"cmd_$(echo $command | sed -e s/-/_/g)" "$@"
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
new file mode 100755
index 0000000000..c4b370ea85
--- /dev/null
+++ b/t/t7419-submodule-set-branch.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+#
+# Copyright (c) 2019 Denton Liu
+#
+
+test_description='Test submodules set-branch subcommand
+
+This test verifies that the set-branch subcommand of git-submodule is working
+as expected.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+test_expect_success 'submodule config cache setup' '
+	mkdir submodule &&
+	(cd submodule &&
+		git init &&
+		echo a >a &&
+		git add . &&
+		git commit -ma &&
+		git checkout -b topic &&
+		echo b >a &&
+		git add . &&
+		git commit -mb
+	) &&
+	mkdir super &&
+	(cd super &&
+		git init &&
+		git submodule add ../submodule &&
+		git commit -m "add submodule"
+	)
+'
+
+test_expect_success 'ensure submodule branch is unset' '
+	(cd super &&
+		test_must_fail grep branch .gitmodules
+	)
+'
+
+test_expect_success 'test submodule set-branch --branch' '
+	(cd super &&
+		git submodule set-branch --branch topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch --default' '
+	(cd super &&
+		git submodule set-branch --default submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -b' '
+	(cd super &&
+		git submodule set-branch -b topic submodule &&
+		grep "branch = topic" .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		b
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'test submodule set-branch -d' '
+	(cd super &&
+		git submodule set-branch -d submodule &&
+		test_must_fail grep branch .gitmodules &&
+		git submodule update --remote &&
+		cat <<-\EOF >expect &&
+		a
+		EOF
+		git -C submodule show -s --pretty=%s >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.20.1.522.g5f42c252e9

Re: [PATCH v3 0/3] Teach submodule set-branch subcommand

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> By the way, just for the record, how would you like me to handle
> patchsets that cause merge conflicts?

When it happens, I may ask you to rebase onto a specific commit.
https://public-inbox.org/git/xmqq5ztxstkh.fsf@gitster-ct.c.googlers.com/
is a recent example.

My preference (read: I'd be grateful if contributors tried to stick
to it, but it won't be the reason for patch rejection if they don't)
is:

 1. Choose the right base:

   a) For a fix to a bug that already exists in the last feature
      release (i.e. v2.20.0), build on 'maint' and make sure it
      merges cleanly to 'master', and the merge result builds and
      passes tests.  Depending on the severity of the bug, we might
      want to make sure that the fix applies maintenance track even
      older, but it would be something you would be sending patches
      to git-security mailing list---over there we'll figure out the
      right base on case-by-case basis.

   b) For a new feature, build on 'master', if you can.

   c) If you need to use something (i.e. a new helper function,
      updated type, etc.) in 'next' that is not yet in 'master',
      identify the topic(s) that introduces these things you need,
      merge them to 'master' and build on top of it.  If you did so,
      please note in the cover letter what topics you depend on.

   d) When updating a topic that is already in my tree
      (i.e. rerolling), stick to the same base as the previous
      round, if possible.  You can find out the commit the previous
      round was applied on top by fetching from me.  Your reroll may
      start using something the previous round did not use from
      'next', in which case you may end up doing c) above, which is
      OK.  Just make a note in the cover letter to let reviewers
      know.

 2. Make sure the result builds and passes tests.

 3. Make sure the result merges cleanly to 'next', and the merge
    result builds and passes tests.

 4. In any of the above steps where you are asked to "make sure it
    merges cleanly", you may see merge conflicts.

   a) If they are too much to resolve for you, for a change that is
      not a bugfix, you may have to depend on the other topic(s).
      Identify the topic(s) that touches these lines that heavily
      conflict with your changes, merge them to 'master' and base
      your topic on top of it (i.e. going back to step 1.c).

   b) If they are easy to resolve for you, do not worry too much
      about them.  It may be helpful to note in the cover letter
      "this may have minor textual conflict with these other
      topics".
   
   c) If you are left with huge conflict while working on a bugfix
      change, we need to resolve it on case-by-case basis, so send
      it out with the chosen base.  Noting that it conflicts heavily
      with 'master' or 'next' would be very much appreciated when
      this happens.

Thanks.

Re: [PATCH v4 3/3] submodule: teach set-branch subcommand

[Denton Liu]

On Fri, 8 Feb 2019 at 03:21, Denton Liu <liu.denton@gmail.com> wrote:
> This teaches git-submodule the set-branch subcommand which allows the
> branch of a submodule to be set through a porcelain command without
> having to manually manipulate the .gitmodules file.
> ---

Sorry Junio, I forgot to attach my sign-off to this patch. Would it be possible
for you to tack it on for me please?

Signed-off-by: Denton Liu <liu.denton@gmail.com>

Thanks,

Denton