[PATCH] submodule update --remote: introduce pinning

[Stefan Beller <sbeller@google.com>]

gitmodules(5) sayeth:

   submodule.<name>.branch
       A remote branch name for tracking updates in the upstream
       submodule. If the option is not specified, it defaults to master.

This doesn't allow having a "pinned" submodule that should not be updated
from upstream. We should change this to have no default --- if branch is
not specified, don't update that submodule, just like in Gerrit's
corresponding feature[1].

[1] https://gerrit-review.googlesource.com/Documentation/user-submodules.html#_defining_the_submodule_branch


However changing defaults is difficult as it may surprise the user,
Jonathan came up with a 4 step plan:
Step 0: introduce

	# current behavior:
        git submodule update --remote --remote-default-to-master

	# new behavior:
        git submodule update --remote --remote-pinned

and treat plain "git submodule update --remote" as --default-to-master.

Step 1: when neither --default-to-master nor --no-default-to-master
has been passed, warn when encountering a submodule with no branch
and treat it as "master".

Step 2: when neither --default-to-master nor --no-default-to-master
has been passed, error out when encountering a submodule with no
branch.

Step 3: when neither --default-to-master nor --no-default-to-master
has been passed, warn when encountering a submodule with no branch
and treat it as pinned.

Step 4: eliminate the warning.

This implements step 0 and 1.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

Sorry that this took so long, this is a rough patch for steps 0 & 1,
I'd still need to update docs.

Stefan


 Documentation/gitmodules.txt | 11 ++++++-----
 builtin/submodule--helper.c  | 36 +++++++++++++++++++++++++++++-------
 git-submodule.sh             | 34 +++++++++++++++++++++++++---------
 t/t7406-submodule-update.sh  | 29 +++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 4d63def206..fe42dbdb3e 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -50,11 +50,12 @@ submodule.<name>.update::
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-	If the option is not specified, it defaults to 'master'.  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.  See the `--remote` documentation in
-	linkgit:git-submodule[1] for details.
+	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.  See the `--remote` documentation
+	in linkgit:git-submodule[1] for details.
+	If not set, the default for `git submodule update --remote` is
+	to update the submodule to the superproject's recorded object id.
 
 submodule.<name>.fetchRecurseSubmodules::
 	This option can be used to control recursive fetching of this
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 40844870cf..6413f2b410 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1889,7 +1889,7 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix
 	return 0;
 }
 
-static const char *remote_submodule_branch(const char *path)
+static const char *remote_submodule_branch(const char *path, int default_pinned)
 {
 	const struct submodule *sub;
 	const char *branch = NULL;
@@ -1904,8 +1904,12 @@ static const char *remote_submodule_branch(const char *path)
 		branch = sub->branch;
 	free(key);
 
-	if (!branch)
-		return "master";
+	if (!branch) {
+		if (default_pinned)
+			return "";
+		else
+			return "master";
+	}
 
 	if (!strcmp(branch, ".")) {
 		const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
@@ -1932,12 +1936,30 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
 {
 	const char *ret;
 	struct strbuf sb = STRBUF_INIT;
-	if (argc != 2)
-		die("submodule--helper remote-branch takes exactly one arguments, got %d", argc);
+	int default_pinned = 0;
+
+	struct option remote_options[] = {
+		OPT_SET_INT(0, "default-master", &default_pinned,
+				N_("unconfigured submodules default to master branch"), 0),
+		OPT_SET_INT(0, "default-pinned", &default_pinned,
+				N_("unconfigured submodules default to superproject object id"), 1),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule--helper remote-branch [--default-{master, pinned}]  -- <path>"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, remote_options,
+			     git_submodule_helper_usage, 0);
+
+	if (argc != 1)
+		die("submodule--helper remote-branch takes exactly one path, got %d", argc);
 
-	ret = remote_submodule_branch(argv[1]);
+	ret = remote_submodule_branch(argv[0], default_pinned);
 	if (!ret)
-		die("submodule %s doesn't exist", argv[1]);
+		die("submodule %s doesn't exist", argv[0]);
 
 	printf("%s", ret);
 	strbuf_release(&sb);
diff --git a/git-submodule.sh b/git-submodule.sh
index 1b568e29b9..829b90ea97 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -431,6 +431,7 @@ fetch_in_submodule () (
 #
 cmd_update()
 {
+	remote_default_option=
 	# parse $args after "submodule ... update".
 	while test $# -ne 0
 	do
@@ -450,6 +451,12 @@ cmd_update()
 		--remote)
 			remote=1
 			;;
+		--remote-default-to-master)
+			remote_default_option=--default-master
+			;;
+		--remote-pinned)
+			remote_default_option=--default-pinned
+			;;
 		-N|--no-fetch)
 			nofetch=1
 			;;
@@ -555,17 +562,26 @@ cmd_update()
 
 		if test -n "$remote"
 		then
-			branch=$(git submodule--helper remote-branch "$sm_path")
-			if test -z "$nofetch"
+			if test -z $remote_default_option
 			then
-				# Fetch remote before determining tracking $sha1
-				fetch_in_submodule "$sm_path" $depth ||
-				die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
+				say "--remote needs clarification: --remote-pinned or --remote-default-to-master?"
+				say "assuming --remote-default-to-master for now"
+				remote_default_option=--default-master
+			fi
+			branch=$(git submodule--helper remote-branch ${remote_default_option} -- "$sm_path")
+			if test -n "$branch"
+			then
+				if test -z "$nofetch"
+				then
+					# Fetch remote before determining tracking $sha1
+					fetch_in_submodule "$sm_path" $depth ||
+					die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
+				fi
+				remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
+				sha1=$(sanitize_submodule_env; cd "$sm_path" &&
+					git rev-parse --verify "${remote_name}/${branch}") ||
+				die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
 			fi
-			remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
-			sha1=$(sanitize_submodule_env; cd "$sm_path" &&
-				git rev-parse --verify "${remote_name}/${branch}") ||
-			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
 		fi
 
 		if test "$subsha1" != "$sha1" || test -n "$force"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 10dc91620a..3019978211 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -260,6 +260,35 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit
 	)
 '
 
+test_expect_success 'submodule update --remote should not fetch upstream when no branch is set' '
+	(
+		cd super &&
+		test_might_fail git config --unset -f .gitmodules submodule.submodule.branch &&
+		git add .gitmodules &&
+		git commit --allow-empty -m "submodules: pin in superproject branch" &&
+		git -C ../submodule checkout -f master
+	) &&
+	(
+		cd submodule &&
+		echo line4b >>file &&
+		git add file &&
+		test_tick &&
+		git commit -m "upstream line4b"
+	) &&
+	(
+		cd super &&
+
+		git submodule update --remote-pinned --remote --force submodule &&
+		git status --porcelain --untracked=no --ignore-submodules=none >actual &&
+		test_must_be_empty actual &&
+
+		git submodule update --remote-default-to-master --remote --force submodule &&
+		git -C submodule log -1 --oneline >actual &&
+		git -C ../submodule log -1 --oneline >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'local config should override .gitmodules branch' '
 	(cd submodule &&
 	 git checkout test-branch &&
-- 
2.19.0