threads / discuss / 42998

[PATCHv4 6/7] submodule--helper: add remote-branch helper

Subject: [PATCHv4 6/7] submodule--helper: add remote-branch helper

## tl;dr

6 messages between Aug 3, 2016 and Aug 3, 2016.

replies: 5people: 2as markdown or json

Stefan Beller· Aug 3, 2016, 20:44 UTC · lore

[PATCHv4 {6,7}/7] submodule update: allow '.' for branch value

This replaces the last two commits of  sb/submodule-update-dot-branch.

Thanks Jeff for pointing out the possible segfault. In the last commit I removed commented code in the test.

Thanks, Stefan

diff to sb/submodule-update-dot-branch:
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ae88eff..f1acc4d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -906,6 +906,9 @@ static const char *remote_submodule_branch(const char *path)
        git_config(submodule_config, NULL);
 
        sub = submodule_from_path(null_sha1, path);
+       if (!sub)
+               return NULL;
+
        if (!sub->branch)
                return "master";
 
@@ -933,11 +936,16 @@ static const char *remote_submodule_branch(const char *path)
 static int resolve_remote_submodule_branch(int argc, const char **argv,
                const char *prefix)
 {
+       const char *ret;
        struct strbuf sb = STRBUF_INIT;
        if (argc != 2)
                die("submodule--helper remote-branch takes exactly one arguments, got %d", argc);
 
-       printf("%s", remote_submodule_branch(argv[1]));
+       ret = remote_submodule_branch(argv[1]);
+       if (!ret)
+               die("submodule %s doesn't exist", argv[1]);
+
+       printf("%s", ret);
        strbuf_release(&sb);
        return 0;
 }
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1c4c1f2..d7983cf 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -238,7 +238,6 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit
                test_cmp expect actual &&
                git checkout master &&
                git branch -d test-branch &&
-               #~ git -C ../submodule branch -d test-branch &&
                git reset --hard HEAD^
        )
 '

Stefan Beller (2):
  submodule--helper: add remote-branch helper
  submodule update: allow '.' for branch value

 builtin/submodule--helper.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh            |  2 +-
 t/t7406-submodule-update.sh | 35 ++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 3 deletions(-)

-- 
2.9.2.524.gdbd1860
Stefan Beller· Aug 3, 2016, 20:44 UTC · re: Stefan Beller · lore

In a later patch we want to enhance the logic for the branch selection. Rewrite the current logic to be in C, so we can directly use C when we enhance the logic.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c | 36 +++++++++++++++++++++++++++++++++++-
 git-submodule.sh            |  2 +-
 2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index fb90c64..9be2c75 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -899,6 +899,39 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix
	return 0;
 }
 
+static const char *remote_submodule_branch(const char *path)
+{
+	const struct submodule *sub;
+	gitmodules_config();
+	git_config(submodule_config, NULL);
+
+	sub = submodule_from_path(null_sha1, path);
+	if (!sub)
+		return NULL;
+
+	if (!sub->branch)
+		return "master";
+
+	return sub->branch;
+}
+
+static int resolve_remote_submodule_branch(int argc, const char **argv,
+		const char *prefix)
+{
+	const char *ret;
+	struct strbuf sb = STRBUF_INIT;
+	if (argc != 2)
+		die("submodule--helper remote-branch takes exactly one arguments, got %d", argc);
+
+	ret = remote_submodule_branch(argv[1]);
+	if (!ret)
+		die("submodule %s doesn't exist", argv[1]);
+
+	printf("%s", ret);
+	strbuf_release(&sb);
+	return 0;
+}
+
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
@@ -912,7 +945,8 @@ static struct cmd_struct commands[] = {
 	{"relative-path", resolve_relative_path},
 	{"resolve-relative-url", resolve_relative_url},
 	{"resolve-relative-url-test", resolve_relative_url_test},
-	{"init", module_init}
+	{"init", module_init},
+	{"remote-branch", resolve_remote_submodule_branch}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index 41aff65..8c5e898 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -614,7 +614,7 @@ cmd_update()
 
 		if test -n "$remote"
 		then
-			branch=$(get_submodule_config "$name" branch master)
+			branch=$(git submodule--helper remote-branch "$sm_path")
 			if test -z "$nofetch"
 			then
 				# Fetch remote before determining tracking $sha1

-- 
2.9.2.524.gdbd1860
Junio C Hamano· Aug 3, 2016, 23:13 UTC · re: Stefan Beller · lore

Re: [PATCHv4 6/7] submodule--helper: add remote-branch helper

Stefan Beller <sbeller@google.com> writes:
Show 6 quoted lines
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index fb90c64..9be2c75 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -899,6 +899,39 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix
> 	return 0;

I wonder who lost the leading SP before the tab here. Will manually fix, so this is not a reason to force resending, but you may want to make sure there is no systemic cause to corrupt your future patches.

Thanks.
Stefan Beller· Aug 3, 2016, 23:22 UTC · re: Junio C Hamano · lore

Re: [PATCHv4 6/7] submodule--helper: add remote-branch helper

On Wed, Aug 3, 2016 at 4:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index fb90c64..9be2c75 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -899,6 +899,39 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix
>>       return 0;
>
> I wonder who lost the leading SP before the tab here.  Will manually
> fix, so this is not a reason to force resending, but you may want to
> make sure there is no systemic cause to corrupt your future patches.

It's not a systematic issue, it was user error. (I started writing out a story, but it got long)

Thanks, Stefan

>
> Thanks.
Stefan Beller· Aug 3, 2016, 20:44 UTC · re: Stefan Beller · lore

[PATCHv4 6/7] submodule update: allow '.' for branch value

Gerrit has a "superproject subscription" feature[1], that triggers a commit in a superproject that is subscribed to its submodules. Conceptually this Gerrit feature can be done on the client side with Git via (except for raciness, error handling etc):

  while [ true ]; do
    git -C <superproject> submodule update --remote --force
    git -C <superproject> commit -a -m "Update submodules"
    git -C <superproject> push
  done

for each branch in the superproject. To ease the configuration in Gerrit a special value of "." has been introduced for the submodule.<name>.branch to mean the same branch as the superproject[2], such that you can create a new branch on both superproject and the submodule and this feature continues to work on that new branch.

Now we find projects in the wild with such a .gitmodules file. The .gitmodules used in these Gerrit projects do not conform to Gits understanding of how .gitmodules should look like. This teaches Git to deal gracefully with this syntax as well.

The redefinition of "." does no harm to existing projects unaware of this change, as "." is an invalid branch name in Git, so we do not expect such projects to exist.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c | 18 ++++++++++++++++++
 t/t7406-submodule-update.sh | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 9be2c75..f1acc4d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -912,6 +912,24 @@ static const char *remote_submodule_branch(const char *path)
 	if (!sub->branch)
 		return "master";
 
+	if (!strcmp(sub->branch, ".")) {
+		unsigned char sha1[20];
+		const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
+
+		if (!refname)
+			die(_("No such ref: %s"), "HEAD");
+
+		/* detached HEAD */
+		if (!strcmp(refname, "HEAD"))
+			die(_("Submodule (%s) branch configured to inherit "
+			      "branch from superproject, but the superproject "
+			      "is not on any branch"), sub->name);
+
+		if (!skip_prefix(refname, "refs/heads/", &refname))
+			die(_("Expecting a full ref name, got %s"), refname);
+		return refname;
+	}
+
 	return sub->branch;
 }
 
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1bb1f43..d7983cf 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -209,9 +209,42 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '
 	)
 '
 
+test_expect_success 'submodule update --remote should fetch upstream changes with .' '
+	(
+		cd super &&
+		git config -f .gitmodules submodule."submodule".branch "." &&
+		git add .gitmodules &&
+		git commit -m "submodules: update from the respective superproject branch"
+	) &&
+	(
+		cd submodule &&
+		echo line4a >> file &&
+		git add file &&
+		test_tick &&
+		git commit -m "upstream line4a" &&
+		git checkout -b test-branch &&
+		test_commit on-test-branch
+	) &&
+	(
+		cd super &&
+		git submodule update --remote --force submodule &&
+		git -C submodule log -1 --oneline >actual
+		git -C ../submodule log -1 --oneline master >expect
+		test_cmp expect actual &&
+		git checkout -b test-branch &&
+		git submodule update --remote --force submodule &&
+		git -C submodule log -1 --oneline >actual
+		git -C ../submodule log -1 --oneline test-branch >expect
+		test_cmp expect actual &&
+		git checkout master &&
+		git branch -d test-branch &&
+		git reset --hard HEAD^
+	)
+'
+
 test_expect_success 'local config should override .gitmodules branch' '
 	(cd submodule &&
-	 git checkout -b test-branch &&
+	 git checkout test-branch &&
 	 echo line5 >> file &&
 	 git add file &&
 	 test_tick &&

-- 
2.9.2.524.gdbd1860
Junio C Hamano· Aug 3, 2016, 23:14 UTC · re: Stefan Beller · lore

Re: [PATCHv4 6/7] submodule update: allow '.' for branch value

Stefan Beller <sbeller@google.com> writes:
> Gerrit has a "superproject subscription" feature[1], that triggers a
> commit in a superproject that is subscribed to its submodules.
> Conceptually this Gerrit feature can be done on the client side with
> Git via (except for raciness, error handling etc):
This would replace 7/7 not 6/7 obviously ;-)
Thanks.

← back to recent threads