Message ID | 6bda69541b12e93cfcf7b841b8691296dc82eeba.1592951611.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Allow overriding the default name of the default branch | expand |
Hi Dscho, > Le 23 juin 2020 à 18:33, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> a écrit : > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > When `remote.<name>.branch` is not configured, `git submodule update` > currently falls back to using the branch name `master`. A much better > idea, however, is to use `HEAD`: on all Git servers running reasonably > recent Git versions, the symref `HEAD` points to the main branch. To be pedantic, it is up to the maintainer/developers to make sure that HEAD on the canonic repository indeed points to the branch that is considered the 'main' branch of the project; the Git version does not really matter IMO... I've seen plenty of repos at work that have HEAD pointing to `master` but `master` is not used, or not anymore... > Note: t7419 demonstrates that there _might_ be use cases out there that > _expect_ `git submodule update --remote` to update submodules to the > remote `master` branch even if the remote `HEAD` points to another > branch. Arguably, this patch makes the behavior more intuitive, but > there is a slight possibility that this might cause regressions in > obscure setups. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> I think that's an excellent idea. However I'd be more explicit in the commit message title: submodule: fall back to remote's HEAD for missing remote.<name>.branch meta comment: I never know where is the best place to make suggestions about changing the commit message title, since I can't quote it in my replies! > --- > Documentation/git-submodule.txt | 4 ++-- > builtin/submodule--helper.c | 2 +- > t/t7406-submodule-update.sh | 16 ++++++++++++++++ > t/t7419-submodule-set-branch.sh | 7 +++++-- > 4 files changed, 24 insertions(+), 5 deletions(-) I think Documentation/gitmodules.txt (`submodule.<name>.branch` header) should also be changed to reflect the behaviour change: diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 67275fd187..539b4e1997 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -49,9 +49,9 @@ 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 + If the option is not specified, it defaults to the remote 'HEAD'. + 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. > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index c9ed2bf3d5..b20f85e622 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -284,7 +284,7 @@ OPTIONS > `.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. If the > - option is not specified, it defaults to 'master'. > + option is not specified, it defaults to 'HEAD'. Just to be extra clear (it's easy to get confused with submodules!) I'd say explicitly: If the option is not specified, it defaults to the remote 'HEAD'. > > -f:: > --force:: > @@ -322,7 +322,7 @@ OPTIONS > the superproject's recorded SHA-1 to update the submodule, use the > status of the submodule's remote-tracking branch. The remote used > is branch's remote (`branch.<name>.remote`), defaulting to `origin`. > - The remote branch used defaults to `master`, but the branch name may > + The remote branch used defaults to `HEAD`, but the branch name may Same thing here: The remote branch used defaults to the remote `HEAD`. > be overridden by setting the `submodule.<name>.branch` option in > either `.gitmodules` or `.git/config` (with `.git/config` taking > precedence). Also, you seem to have missed the `master` reference in the description of the 'set-branch' subcommand. Something like this would do, I think: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index c9ed2bf3d5..8cf5831a72 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -183,7 +183,7 @@ set-branch (-d|--default) [--] <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'. + key, which causes the tracking branch to default to the remote 'HEAD'. set-url [--] <path> <newurl>:: Sets the URL of the specified submodule to <newurl>. Then, it will The rest of the patch looks good. Cheers, Philippe.
Hi Philippe, On Tue, 23 Jun 2020, Philippe Blain wrote: > > Le 23 juin 2020 à 18:33, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> a écrit : > > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > When `remote.<name>.branch` is not configured, `git submodule update` > > currently falls back to using the branch name `master`. A much better > > idea, however, is to use `HEAD`: on all Git servers running reasonably > > recent Git versions, the symref `HEAD` points to the main branch. > > To be pedantic, it is up to the maintainer/developers to make sure that > HEAD on the canonic repository indeed points to the branch that is > considered the 'main' branch of the project; the Git version does not > really matter IMO... I've seen plenty of repos at work that have HEAD > pointing to `master` but `master` is not used, or not anymore... Oh, but in this case, my main concern was servers that are so old that `HEAD` is not a symref, in which case it can be a bit tricky to figure out the default branch. In any case, I feel this is such a minor point that I don't really want to spend a lot of time on this paragraph. > > Note: t7419 demonstrates that there _might_ be use cases out there that > > _expect_ `git submodule update --remote` to update submodules to the > > remote `master` branch even if the remote `HEAD` points to another > > branch. Arguably, this patch makes the behavior more intuitive, but > > there is a slight possibility that this might cause regressions in > > obscure setups. > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > I think that's an excellent idea. However I'd be more explicit in the > commit message title: > > submodule: fall back to remote's HEAD for missing remote.<name>.branch Great! > meta comment: I never know where is the best place to make suggestions > about changing the commit message title, since I can't quote it in my > replies! True. You could copy it, but that's not the same. > > --- > > Documentation/git-submodule.txt | 4 ++-- > > builtin/submodule--helper.c | 2 +- > > t/t7406-submodule-update.sh | 16 ++++++++++++++++ > > t/t7419-submodule-set-branch.sh | 7 +++++-- > > 4 files changed, 24 insertions(+), 5 deletions(-) > > I think Documentation/gitmodules.txt (`submodule.<name>.branch` header) > should also be changed to reflect the behaviour change: > > diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt > index 67275fd187..539b4e1997 100644 > --- a/Documentation/gitmodules.txt > +++ b/Documentation/gitmodules.txt > @@ -49,9 +49,9 @@ 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 > + If the option is not specified, it defaults to the remote 'HEAD'. > + 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. Excellent, I used that verbatim. Thank you! > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > > index c9ed2bf3d5..b20f85e622 100644 > > --- a/Documentation/git-submodule.txt > > +++ b/Documentation/git-submodule.txt > > @@ -284,7 +284,7 @@ OPTIONS > > `.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. If the > > - option is not specified, it defaults to 'master'. > > + option is not specified, it defaults to 'HEAD'. > > Just to be extra clear (it's easy to get confused with submodules!) I'd > say explicitly: > > If the option is not specified, it defaults to the remote 'HEAD'. Good idea. I incorporated that, too. > > -f:: > > --force:: > > @@ -322,7 +322,7 @@ OPTIONS > > the superproject's recorded SHA-1 to update the submodule, use the > > status of the submodule's remote-tracking branch. The remote used > > is branch's remote (`branch.<name>.remote`), defaulting to `origin`. > > - The remote branch used defaults to `master`, but the branch name may > > + The remote branch used defaults to `HEAD`, but the branch name may > > Same thing here: > > The remote branch used defaults to the remote `HEAD`. And that. > > be overridden by setting the `submodule.<name>.branch` option in > > either `.gitmodules` or `.git/config` (with `.git/config` taking > > precedence). > > Also, you seem to have missed the `master` reference in the description of > the 'set-branch' subcommand. Something like this would do, I think: > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index c9ed2bf3d5..8cf5831a72 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -183,7 +183,7 @@ set-branch (-d|--default) [--] <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'. > + key, which causes the tracking branch to default to the remote 'HEAD'. > > set-url [--] <path> <newurl>:: > Sets the URL of the specified submodule to <newurl>. Then, it will And that. > The rest of the patch looks good. Awesome. Thank you for helping me improve this patch series! Ciao, Dscho
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index c9ed2bf3d5..b20f85e622 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -284,7 +284,7 @@ OPTIONS `.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. If the - option is not specified, it defaults to 'master'. + option is not specified, it defaults to 'HEAD'. -f:: --force:: @@ -322,7 +322,7 @@ OPTIONS the superproject's recorded SHA-1 to update the submodule, use the status of the submodule's remote-tracking branch. The remote used is branch's remote (`branch.<name>.remote`), defaulting to `origin`. - The remote branch used defaults to `master`, but the branch name may + The remote branch used defaults to `HEAD`, but the branch name may be overridden by setting the `submodule.<name>.branch` option in either `.gitmodules` or `.git/config` (with `.git/config` taking precedence). diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 46c03d2a12..f55f7b7704 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1981,7 +1981,7 @@ static const char *remote_submodule_branch(const char *path) free(key); if (!branch) - return "master"; + return "HEAD"; if (!strcmp(branch, ".")) { const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL); diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 4fb447a143..aa19ff3a2e 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -70,6 +70,22 @@ test_expect_success 'setup a submodule tree' ' ) ' +test_expect_success 'update --remote falls back to using HEAD' ' + test_create_repo main-branch-submodule && + test_commit -C main-branch-submodule initial && + + test_create_repo main-branch && + git -C main-branch submodule add ../main-branch-submodule && + git -C main-branch commit -m add-submodule && + + git -C main-branch-submodule switch -c hello && + test_commit -C main-branch-submodule world && + + git clone --recursive main-branch main-branch-clone && + git -C main-branch-clone submodule update --remote main-branch-submodule && + test_path_exists main-branch-clone/main-branch-submodule/world.t +' + test_expect_success 'submodule update detaching the HEAD ' ' (cd super/submodule && git reset --hard HEAD~1 diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh index fd25f786a3..3b925c302f 100755 --- a/t/t7419-submodule-set-branch.sh +++ b/t/t7419-submodule-set-branch.sh @@ -52,12 +52,13 @@ test_expect_success 'test submodule set-branch --branch' ' ' test_expect_success 'test submodule set-branch --default' ' + test_commit -C submodule c && (cd super && git submodule set-branch --default submodule && ! grep branch .gitmodules && git submodule update --remote && cat <<-\EOF >expect && - a + c EOF git -C submodule show -s --pretty=%s >actual && test_cmp expect actual @@ -65,6 +66,7 @@ test_expect_success 'test submodule set-branch --default' ' ' test_expect_success 'test submodule set-branch -b' ' + test_commit -C submodule b && (cd super && git submodule set-branch -b topic submodule && grep "branch = topic" .gitmodules && @@ -78,12 +80,13 @@ test_expect_success 'test submodule set-branch -b' ' ' test_expect_success 'test submodule set-branch -d' ' + test_commit -C submodule d && (cd super && git submodule set-branch -d submodule && ! grep branch .gitmodules && git submodule update --remote && cat <<-\EOF >expect && - a + d EOF git -C submodule show -s --pretty=%s >actual && test_cmp expect actual