diff mbox series

[3/3] init: provide useful advice about init.defaultBranch

Message ID 253d6706e6ab97e71ec012f6de33c75f3e980701.1606087406.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add helpful advice about init.defaultBranch | expand

Commit Message

Johannes Schindelin Nov. 22, 2020, 11:23 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

To give ample warning in case we decide to change the fall-back for an
unconfigured `init.defaultBranch`, let's introduce some advice that is
shown upon `git init` when that value is not set.

Note: three test cases in Git's test suite want to verify that the
`stderr` output of `git init` is empty. With this patch, that is only
true if `init.defaultBranch` is configured, so let's do exactly that in
those test cases. The same reasoning applies to `test_create_repo()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 refs.c                        | 13 ++++++++++++-
 t/t0001-init.sh               |  9 ++++++++-
 t/t1510-repo-setup.sh         |  2 +-
 t/t7414-submodule-mistakes.sh |  2 +-
 t/test-lib-functions.sh       |  3 ++-
 5 files changed, 24 insertions(+), 5 deletions(-)

Comments

Junio C Hamano Nov. 22, 2020, 11:53 p.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> To give ample warning in case we decide to change the fall-back for an
> unconfigured `init.defaultBranch`, let's introduce some advice that is
> shown upon `git init` when that value is not set.

I know this means well.  

I however doubt "ample warning" before we announce an actual
decision makes user's life any easier in practice, though.  Until
they know what it will be changed to (if we decide not to change
anything, treat it as changing it to 'master'), they would not be
able to decide if the name is more suitable for their use case.

I further suspect that their choice will be influenced the most by
the choice made by the projects they most often interact with, and
not by our plan.

> +static const char default_branch_name_advice[] = N_(
> +"Using '%s' as the name for the initial branch. This name is subject\n"
> +"to change. To configure the name to use as the initial branch name in\n"
> +"new repositories, or to silence this warning, run:\n"

s/new repositories/all of your new repositories/ as that is the
whole point of using --global option below.

> +"\n"
> +"\tgit config --global init.defaultBranch <name>\n"
> +);
> +

> +test_expect_success 'advice on unconfigured init.defaultBranch' '
> +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \
> +		init unconfigured-default-branch-name 2>err &&
> +	test_decode_color <err >decoded &&
> +	test_i18ngrep "<YELLOW>hint: " decoded
> +'
> +

> diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
> index f2e7df59cf..0ed02938f9 100755
> --- a/t/t7414-submodule-mistakes.sh
> +++ b/t/t7414-submodule-mistakes.sh
> @@ -30,7 +30,7 @@ test_expect_success 'no warning when updating entry' '
>  
>  test_expect_success 'submodule add does not warn' '
>  	test_when_finished "git rm -rf submodule .gitmodules" &&
> -	git submodule add ./embed submodule 2>stderr &&
> +	git -c init.defaultBranch=x submodule add ./embed submodule 2>stderr &&
>  	test_i18ngrep ! warning stderr
>  '

It is a bit subtle that this not only tests whatever "git submodule"
test it originally wanted to do but also serves as a test that the
new advice message is squelched by the presence of the configuration.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 59bbf75e83..772152320a 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1202,7 +1202,8 @@ test_create_repo () {
>  	mkdir -p "$repo"
>  	(
>  		cd "$repo" || error "Cannot setup test environment"
> -		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
> +		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
> +			-c init.defaultBranch=master init \

I wonder if this should be more like

	-c init.defaultBranch=${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME-master}

so that tests that want a particular branch name would get what they
want?

Eventually we would need to do the s/master/X/ if/when the actual
default change happens.

>  			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
>  		error "cannot run git init -- have you built things yet?"
>  		mv .git/hooks .git/hooks-disabled
Junio C Hamano Nov. 23, 2020, 2:07 a.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

>> +static const char default_branch_name_advice[] = N_(
>> +"Using '%s' as the name for the initial branch. This name is subject\n"
>> +"to change. To configure the name to use as the initial branch name in\n"
>> +"new repositories, or to silence this warning, run:\n"
>
> s/new repositories/all of your new repositories/ as that is the
> whole point of using --global option below.
>
>> +"\n"
>> +"\tgit config --global init.defaultBranch <name>\n"
>> +);
>> +

The above may give a valuable lesson to those who want to use one
branch name across new repositories, but it does not tell those who
wanted 'trunk' (to match the project, perhaps github.com/cli/cli,
with which they intend to interact) how to recover from having
already created the 'master' branch.  We may want to add some text
to suggest "branch -M" after giving the advice for the permanent
option.

Also, it is unclear to those who do not have a good <name> in mind
(or, those who do not care to choose a <name> for themselves), what
<name> they should give to take the "or to silence this warning"
part of the advice.  It probably is a good idea to rephrase and say
either:

    ... To configure ... in all your new repositories and squelch
    this message, run:

	git config --global init.defaultBranch <name>

or

    ... To configure ... in all your new repositories, run:

	git config --global init.defaultBranch <name>

    Note that this message won't appear after doing so.

I am offhand not sure which one is better.

Thanks.
Johannes Schindelin Nov. 23, 2020, 12:26 p.m. UTC | #3
Hi Junio,

On Sun, 22 Nov 2020, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > To give ample warning in case we decide to change the fall-back for an
> > unconfigured `init.defaultBranch`, let's introduce some advice that is
> > shown upon `git init` when that value is not set.
>
> I know this means well.
>
> I however doubt "ample warning" before we announce an actual
> decision makes user's life any easier in practice, though.

I reworded the commit message to make the rationale more obvious: the
ample warning is to give users a chance to override whatever is Git's
default branch name by configuring `init.defaultBranch`. If they care at
all.

> Until they know what it will be changed to (if we decide not to change
> anything, treat it as changing it to 'master'), they would not be able
> to decide if the name is more suitable for their use case.
>
> I further suspect that their choice will be influenced the most by
> the choice made by the projects they most often interact with, and
> not by our plan.
>
> > +static const char default_branch_name_advice[] = N_(
> > +"Using '%s' as the name for the initial branch. This name is subject\n"
> > +"to change. To configure the name to use as the initial branch name in\n"
> > +"new repositories, or to silence this warning, run:\n"
>
> s/new repositories/all of your new repositories/ as that is the
> whole point of using --global option below.

Makes sense.

> > +"\n"
> > +"\tgit config --global init.defaultBranch <name>\n"
> > +);
> > +
>
> > +test_expect_success 'advice on unconfigured init.defaultBranch' '
> > +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \
> > +		init unconfigured-default-branch-name 2>err &&
> > +	test_decode_color <err >decoded &&
> > +	test_i18ngrep "<YELLOW>hint: " decoded
> > +'
> > +
>
> > diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
> > index f2e7df59cf..0ed02938f9 100755
> > --- a/t/t7414-submodule-mistakes.sh
> > +++ b/t/t7414-submodule-mistakes.sh
> > @@ -30,7 +30,7 @@ test_expect_success 'no warning when updating entry' '
> >
> >  test_expect_success 'submodule add does not warn' '
> >  	test_when_finished "git rm -rf submodule .gitmodules" &&
> > -	git submodule add ./embed submodule 2>stderr &&
> > +	git -c init.defaultBranch=x submodule add ./embed submodule 2>stderr &&
> >  	test_i18ngrep ! warning stderr
> >  '
>
> It is a bit subtle that this not only tests whatever "git submodule"
> test it originally wanted to do but also serves as a test that the
> new advice message is squelched by the presence of the configuration.

Thank you for catching this. It is a debugging left-over.

The real fix for the underlying issue is in the previous commit, which
silences the warning when `init_db()` is called via `clone`, as is the
case here.

I removed it from v2.

> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > index 59bbf75e83..772152320a 100644
> > --- a/t/test-lib-functions.sh
> > +++ b/t/test-lib-functions.sh
> > @@ -1202,7 +1202,8 @@ test_create_repo () {
> >  	mkdir -p "$repo"
> >  	(
> >  		cd "$repo" || error "Cannot setup test environment"
> > -		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
> > +		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
> > +			-c init.defaultBranch=master init \
>
> I wonder if this should be more like
>
> 	-c init.defaultBranch=${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME-master}
>
> so that tests that want a particular branch name would get what they
> want?

As is custom with our `GIT_TEST_*` variables,
`GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME` overrides `init.defaultBranch`.

Therefore, _technically_ what you suggested is not even necessary here.

> Eventually we would need to do the s/master/X/ if/when the actual
> default change happens.

However, _practically_, what you suggested is very helpful, as it will
make it easy to find when (if?) we finish the transition to a new
fall-back.

Thank you for your thorough feedback,
Dscho

> >  			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
> >  		error "cannot run git init -- have you built things yet?"
> >  		mv .git/hooks .git/hooks-disabled
>
Johannes Schindelin Nov. 23, 2020, 12:28 p.m. UTC | #4
Hi Junio,

On Sun, 22 Nov 2020, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
> >> +static const char default_branch_name_advice[] = N_(
> >> +"Using '%s' as the name for the initial branch. This name is subject\n"
> >> +"to change. To configure the name to use as the initial branch name in\n"
> >> +"new repositories, or to silence this warning, run:\n"
> >
> > s/new repositories/all of your new repositories/ as that is the
> > whole point of using --global option below.
> >
> >> +"\n"
> >> +"\tgit config --global init.defaultBranch <name>\n"
> >> +);
> >> +
>
> The above may give a valuable lesson to those who want to use one
> branch name across new repositories, but it does not tell those who
> wanted 'trunk' (to match the project, perhaps github.com/cli/cli,
> with which they intend to interact) how to recover from having
> already created the 'master' branch.  We may want to add some text
> to suggest "branch -M" after giving the advice for the permanent
> option.

Good point.

> Also, it is unclear to those who do not have a good <name> in mind
> (or, those who do not care to choose a <name> for themselves), what
> <name> they should give to take the "or to silence this warning"
> part of the advice.

Also a good point.

> It probably is a good idea to rephrase and say
> either:
>
>     ... To configure ... in all your new repositories and squelch
>     this message, run:
>
> 	git config --global init.defaultBranch <name>
>
> or
>
>     ... To configure ... in all your new repositories, run:
>
> 	git config --global init.defaultBranch <name>
>
>     Note that this message won't appear after doing so.

I came up with this, which I intend to submit with v2:

static const char default_branch_name_advice[] = N_(
"Using '%s' as the name for the initial branch. This name is subject\n"
"to change. To configure the initial branch name to use in all of your\n"
"new repositories (or to suppress this warning), run:\n"
"\n"
"\tgit config --global init.defaultBranch <name>\n"
"\n"
"Common names are 'main', 'trunk' and 'development'. The initial branch\n"
"can be renamed via this command:\n"
"\n"
"\tgit branch -m <name>\n"
);

Ciao,
Dscho
Philip Oakley Nov. 23, 2020, 12:49 p.m. UTC | #5
hi dscho,

On 22/11/2020 23:23, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> To give ample warning in case we decide to change the fall-back for an
> unconfigured `init.defaultBranch`, let's introduce some advice that is
> shown upon `git init` when that value is not set.
>
> Note: three test cases in Git's test suite want to verify that the
> `stderr` output of `git init` is empty. With this patch, that is only
> true if `init.defaultBranch` is configured, so let's do exactly that in
> those test cases. The same reasoning applies to `test_create_repo()`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  refs.c                        | 13 ++++++++++++-
>  t/t0001-init.sh               |  9 ++++++++-
>  t/t1510-repo-setup.sh         |  2 +-
>  t/t7414-submodule-mistakes.sh |  2 +-
>  t/test-lib-functions.sh       |  3 ++-
>  5 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 8df03122d6..61d712ca05 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -562,6 +562,14 @@ void expand_ref_prefix(struct strvec *prefixes, const char *prefix)
>  		strvec_pushf(prefixes, *p, len, prefix);
>  }
>  
> +static const char default_branch_name_advice[] = N_(
> +"Using '%s' as the name for the initial branch. This name is subject\n"
> +"to change. 
This sounds like Git will change the branch name within the user's repo,
rather than "The default" being subject to change.  So maybe
s/This/The default/   ?

Philip

> To configure the name to use as the initial branch name in\n"
> +"new repositories, or to silence this warning, run:\n"
> +"\n"
> +"\tgit config --global init.defaultBranch <name>\n"
> +);
> +
>  char *repo_default_branch_name(struct repository *r, int quiet)
>  {
>  	const char *config_key = "init.defaultbranch";
> @@ -574,8 +582,11 @@ char *repo_default_branch_name(struct repository *r, int quiet)
>  	else if (repo_config_get_string(r, config_key, &ret) < 0)
>  		die(_("could not retrieve `%s`"), config_display_key);
>  
> -	if (!ret)
> +	if (!ret) {
>  		ret = xstrdup("master");
> +		if (!quiet)
> +			advise(_(default_branch_name_advice), ret);
> +	}
>  
>  	full_ref = xstrfmt("refs/heads/%s", ret);
>  	if (check_refname_format(full_ref, 0))
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 69a320489f..754dab3bab 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -163,7 +163,7 @@ test_expect_success 'reinit' '
>  	(
>  		mkdir again &&
>  		cd again &&
> -		git init >out1 2>err1 &&
> +		git -c init.defaultBranch=initial init >out1 2>err1 &&
>  		git init >out2 2>err2
>  	) &&
>  	test_i18ngrep "Initialized empty" again/out1 &&
> @@ -558,6 +558,13 @@ test_expect_success 'overridden default initial branch name (config)' '
>  	grep nmb actual
>  '
>  
> +test_expect_success 'advice on unconfigured init.defaultBranch' '
> +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \
> +		init unconfigured-default-branch-name 2>err &&
> +	test_decode_color <err >decoded &&
> +	test_i18ngrep "<YELLOW>hint: " decoded
> +'
> +
>  test_expect_success 'overridden default main branch name (env)' '
>  	test_config_global init.defaultBranch nmb &&
>  	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=env git init main-branch-env &&
> diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
> index 9974457f56..5189a520a2 100755
> --- a/t/t1510-repo-setup.sh
> +++ b/t/t1510-repo-setup.sh
> @@ -79,7 +79,7 @@ setup_repo () {
>  	name=$1 worktreecfg=$2 gitfile=$3 barecfg=$4 &&
>  	sane_unset GIT_DIR GIT_WORK_TREE &&
>  
> -	git init "$name" &&
> +	git -c init.defaultBranch=repo init "$name" &&
>  	maybe_config "$name/.git/config" core.worktree "$worktreecfg" &&
>  	maybe_config "$name/.git/config" core.bare "$barecfg" &&
>  	mkdir -p "$name/sub/sub" &&
> diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
> index f2e7df59cf..0ed02938f9 100755
> --- a/t/t7414-submodule-mistakes.sh
> +++ b/t/t7414-submodule-mistakes.sh
> @@ -30,7 +30,7 @@ test_expect_success 'no warning when updating entry' '
>  
>  test_expect_success 'submodule add does not warn' '
>  	test_when_finished "git rm -rf submodule .gitmodules" &&
> -	git submodule add ./embed submodule 2>stderr &&
> +	git -c init.defaultBranch=x submodule add ./embed submodule 2>stderr &&
>  	test_i18ngrep ! warning stderr
>  '
>  
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 59bbf75e83..772152320a 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1202,7 +1202,8 @@ test_create_repo () {
>  	mkdir -p "$repo"
>  	(
>  		cd "$repo" || error "Cannot setup test environment"
> -		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
> +		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
> +			-c init.defaultBranch=master init \
>  			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
>  		error "cannot run git init -- have you built things yet?"
>  		mv .git/hooks .git/hooks-disabled
Junio C Hamano Nov. 23, 2020, 6:40 p.m. UTC | #6
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> The above may give a valuable lesson to those who want to use one
>> branch name across new repositories, but it does not tell those who
>> wanted 'trunk' (to match the project, perhaps github.com/cli/cli,
>> with which they intend to interact) how to recover from having
>> already created the 'master' branch.  We may want to add some text
>> to suggest "branch -M" after giving the advice for the permanent
>> option.
>
> Good point.
>
>> Also, it is unclear to those who do not have a good <name> in mind
>> (or, those who do not care to choose a <name> for themselves), what
>> <name> they should give to take the "or to silence this warning"
>> part of the advice.
>
> Also a good point.

> I came up with this, which I intend to submit with v2:
>
> static const char default_branch_name_advice[] = N_(
> "Using '%s' as the name for the initial branch. This name is subject\n"
> "to change. To configure the initial branch name to use in all of your\n"
> "new repositories (or to suppress this warning), run:\n"

The same issue around "to suppress" is here, though.

> "\n"
> "\tgit config --global init.defaultBranch <name>\n"
> "\n"
> "Common names are 'main', 'trunk' and 'development'. The initial branch\n"
> "can be renamed via this command:\n"
> "\n"
> "\tgit branch -m <name>\n"

It is very likely that the users are on an unborn branch when they
see this message and "git branch -m/-M <name>" does not work.  We'd
probably want to update "git branch" to allow renaming the current
branch that is unborn.

In the meantime, you could do "git checkout --orphan <name>" here,
but once <name> exists as a branch that would not work, so...


> );
>
> Ciao,
> Dscho
Johannes Schindelin Nov. 23, 2020, 8:46 p.m. UTC | #7
Hi Junio,

On Mon, 23 Nov 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> The above may give a valuable lesson to those who want to use one
> >> branch name across new repositories, but it does not tell those who
> >> wanted 'trunk' (to match the project, perhaps github.com/cli/cli,
> >> with which they intend to interact) how to recover from having
> >> already created the 'master' branch.  We may want to add some text
> >> to suggest "branch -M" after giving the advice for the permanent
> >> option.
> >
> > Good point.
> >
> >> Also, it is unclear to those who do not have a good <name> in mind
> >> (or, those who do not care to choose a <name> for themselves), what
> >> <name> they should give to take the "or to silence this warning"
> >> part of the advice.
> >
> > Also a good point.
>
> > I came up with this, which I intend to submit with v2:
> >
> > static const char default_branch_name_advice[] = N_(
> > "Using '%s' as the name for the initial branch. This name is subject\n"
> > "to change. To configure the initial branch name to use in all of your\n"
> > "new repositories (or to suppress this warning), run:\n"
>
> The same issue around "to suppress" is here, though.

Hmm. I would like to believe that readers understand that setting it to
_any_ name would suppress this warning.

> > "\n"
> > "\tgit config --global init.defaultBranch <name>\n"
> > "\n"
> > "Common names are 'main', 'trunk' and 'development'. The initial branch\n"
> > "can be renamed via this command:\n"
> > "\n"
> > "\tgit branch -m <name>\n"
>
> It is very likely that the users are on an unborn branch when they
> see this message and "git branch -m/-M <name>" does not work.  We'd
> probably want to update "git branch" to allow renaming the current
> branch that is unborn.

Ouch, good point, I had not even realized that that does not work. In v2,
I will include a patch that lets it work.

> In the meantime, you could do "git checkout --orphan <name>" here,
> but once <name> exists as a branch that would not work, so...

Careful. We should start thinking about phasing out either `checkout` or
`switch`. The latter was intended to supersede the former with respect to
working on branches, so I would think of phasing out `checkout` and favor
`switch` instead.

That issue aside, `git switch --orphan <name>` is not necessarily a good
UI here. Its documentation talks about creating a new branch, deleting all
tracked files. While it is technically still correct that this command,
when run on an unborn branch, does the same as renaming said branch, it is
highly unintuitive to think about it that way. ("I don't want to create a
new branch, I want to rename the current one! And whoa, delete tracked
files? I don't want to delete _any_ files! What do you mean: there aren't
any tracked files, so none are deleted? Why do you talk about deleting
files, then?")

So in the interest of _reducing_ confusion, I would really, really,
_really_ like to avoid mentioning that command in this advice.

It really is much better to make `git branch -m <name>` work in this
scenario.

Ciao,
Dscho

>
>
> > );
> >
> > Ciao,
> > Dscho
>
Johannes Schindelin Nov. 23, 2020, 8:47 p.m. UTC | #8
Hi Philip,

On Mon, 23 Nov 2020, Philip Oakley wrote:

> On 22/11/2020 23:23, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > To give ample warning in case we decide to change the fall-back for an
> > unconfigured `init.defaultBranch`, let's introduce some advice that is
> > shown upon `git init` when that value is not set.
> >
> > Note: three test cases in Git's test suite want to verify that the
> > `stderr` output of `git init` is empty. With this patch, that is only
> > true if `init.defaultBranch` is configured, so let's do exactly that in
> > those test cases. The same reasoning applies to `test_create_repo()`.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  refs.c                        | 13 ++++++++++++-
> >  t/t0001-init.sh               |  9 ++++++++-
> >  t/t1510-repo-setup.sh         |  2 +-
> >  t/t7414-submodule-mistakes.sh |  2 +-
> >  t/test-lib-functions.sh       |  3 ++-
> >  5 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/refs.c b/refs.c
> > index 8df03122d6..61d712ca05 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -562,6 +562,14 @@ void expand_ref_prefix(struct strvec *prefixes, const char *prefix)
> >  		strvec_pushf(prefixes, *p, len, prefix);
> >  }
> >
> > +static const char default_branch_name_advice[] = N_(
> > +"Using '%s' as the name for the initial branch. This name is subject\n"
> > +"to change.
> This sounds like Git will change the branch name within the user's repo,
> rather than "The default" being subject to change.  So maybe
> s/This/The default/   ?

Yes, that sounds clearer to me. Thank you,
Dscho

>
> Philip
>
> > To configure the name to use as the initial branch name in\n"
> > +"new repositories, or to silence this warning, run:\n"
> > +"\n"
> > +"\tgit config --global init.defaultBranch <name>\n"
> > +);
> > +
> >  char *repo_default_branch_name(struct repository *r, int quiet)
> >  {
> >  	const char *config_key = "init.defaultbranch";
> > @@ -574,8 +582,11 @@ char *repo_default_branch_name(struct repository *r, int quiet)
> >  	else if (repo_config_get_string(r, config_key, &ret) < 0)
> >  		die(_("could not retrieve `%s`"), config_display_key);
> >
> > -	if (!ret)
> > +	if (!ret) {
> >  		ret = xstrdup("master");
> > +		if (!quiet)
> > +			advise(_(default_branch_name_advice), ret);
> > +	}
> >
> >  	full_ref = xstrfmt("refs/heads/%s", ret);
> >  	if (check_refname_format(full_ref, 0))
> > diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> > index 69a320489f..754dab3bab 100755
> > --- a/t/t0001-init.sh
> > +++ b/t/t0001-init.sh
> > @@ -163,7 +163,7 @@ test_expect_success 'reinit' '
> >  	(
> >  		mkdir again &&
> >  		cd again &&
> > -		git init >out1 2>err1 &&
> > +		git -c init.defaultBranch=initial init >out1 2>err1 &&
> >  		git init >out2 2>err2
> >  	) &&
> >  	test_i18ngrep "Initialized empty" again/out1 &&
> > @@ -558,6 +558,13 @@ test_expect_success 'overridden default initial branch name (config)' '
> >  	grep nmb actual
> >  '
> >
> > +test_expect_success 'advice on unconfigured init.defaultBranch' '
> > +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \
> > +		init unconfigured-default-branch-name 2>err &&
> > +	test_decode_color <err >decoded &&
> > +	test_i18ngrep "<YELLOW>hint: " decoded
> > +'
> > +
> >  test_expect_success 'overridden default main branch name (env)' '
> >  	test_config_global init.defaultBranch nmb &&
> >  	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=env git init main-branch-env &&
> > diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
> > index 9974457f56..5189a520a2 100755
> > --- a/t/t1510-repo-setup.sh
> > +++ b/t/t1510-repo-setup.sh
> > @@ -79,7 +79,7 @@ setup_repo () {
> >  	name=$1 worktreecfg=$2 gitfile=$3 barecfg=$4 &&
> >  	sane_unset GIT_DIR GIT_WORK_TREE &&
> >
> > -	git init "$name" &&
> > +	git -c init.defaultBranch=repo init "$name" &&
> >  	maybe_config "$name/.git/config" core.worktree "$worktreecfg" &&
> >  	maybe_config "$name/.git/config" core.bare "$barecfg" &&
> >  	mkdir -p "$name/sub/sub" &&
> > diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
> > index f2e7df59cf..0ed02938f9 100755
> > --- a/t/t7414-submodule-mistakes.sh
> > +++ b/t/t7414-submodule-mistakes.sh
> > @@ -30,7 +30,7 @@ test_expect_success 'no warning when updating entry' '
> >
> >  test_expect_success 'submodule add does not warn' '
> >  	test_when_finished "git rm -rf submodule .gitmodules" &&
> > -	git submodule add ./embed submodule 2>stderr &&
> > +	git -c init.defaultBranch=x submodule add ./embed submodule 2>stderr &&
> >  	test_i18ngrep ! warning stderr
> >  '
> >
> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > index 59bbf75e83..772152320a 100644
> > --- a/t/test-lib-functions.sh
> > +++ b/t/test-lib-functions.sh
> > @@ -1202,7 +1202,8 @@ test_create_repo () {
> >  	mkdir -p "$repo"
> >  	(
> >  		cd "$repo" || error "Cannot setup test environment"
> > -		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
> > +		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
> > +			-c init.defaultBranch=master init \
> >  			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
> >  		error "cannot run git init -- have you built things yet?"
> >  		mv .git/hooks .git/hooks-disabled
>
>
Junio C Hamano Nov. 23, 2020, 9:28 p.m. UTC | #9
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hmm. I would like to believe that readers understand that setting it to
> _any_ name would suppress this warning.

Yes, and the point raised in an earlier messages was that it is not
clear that there is no recourse for those who do NOT want to choose
but still want to suppress.

> It really is much better to make `git branch -m <name>` work in this
> scenario.

Yes, if we can make it work, that is much more preferred.
diff mbox series

Patch

diff --git a/refs.c b/refs.c
index 8df03122d6..61d712ca05 100644
--- a/refs.c
+++ b/refs.c
@@ -562,6 +562,14 @@  void expand_ref_prefix(struct strvec *prefixes, const char *prefix)
 		strvec_pushf(prefixes, *p, len, prefix);
 }
 
+static const char default_branch_name_advice[] = N_(
+"Using '%s' as the name for the initial branch. This name is subject\n"
+"to change. To configure the name to use as the initial branch name in\n"
+"new repositories, or to silence this warning, run:\n"
+"\n"
+"\tgit config --global init.defaultBranch <name>\n"
+);
+
 char *repo_default_branch_name(struct repository *r, int quiet)
 {
 	const char *config_key = "init.defaultbranch";
@@ -574,8 +582,11 @@  char *repo_default_branch_name(struct repository *r, int quiet)
 	else if (repo_config_get_string(r, config_key, &ret) < 0)
 		die(_("could not retrieve `%s`"), config_display_key);
 
-	if (!ret)
+	if (!ret) {
 		ret = xstrdup("master");
+		if (!quiet)
+			advise(_(default_branch_name_advice), ret);
+	}
 
 	full_ref = xstrfmt("refs/heads/%s", ret);
 	if (check_refname_format(full_ref, 0))
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 69a320489f..754dab3bab 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -163,7 +163,7 @@  test_expect_success 'reinit' '
 	(
 		mkdir again &&
 		cd again &&
-		git init >out1 2>err1 &&
+		git -c init.defaultBranch=initial init >out1 2>err1 &&
 		git init >out2 2>err2
 	) &&
 	test_i18ngrep "Initialized empty" again/out1 &&
@@ -558,6 +558,13 @@  test_expect_success 'overridden default initial branch name (config)' '
 	grep nmb actual
 '
 
+test_expect_success 'advice on unconfigured init.defaultBranch' '
+	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \
+		init unconfigured-default-branch-name 2>err &&
+	test_decode_color <err >decoded &&
+	test_i18ngrep "<YELLOW>hint: " decoded
+'
+
 test_expect_success 'overridden default main branch name (env)' '
 	test_config_global init.defaultBranch nmb &&
 	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=env git init main-branch-env &&
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index 9974457f56..5189a520a2 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -79,7 +79,7 @@  setup_repo () {
 	name=$1 worktreecfg=$2 gitfile=$3 barecfg=$4 &&
 	sane_unset GIT_DIR GIT_WORK_TREE &&
 
-	git init "$name" &&
+	git -c init.defaultBranch=repo init "$name" &&
 	maybe_config "$name/.git/config" core.worktree "$worktreecfg" &&
 	maybe_config "$name/.git/config" core.bare "$barecfg" &&
 	mkdir -p "$name/sub/sub" &&
diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
index f2e7df59cf..0ed02938f9 100755
--- a/t/t7414-submodule-mistakes.sh
+++ b/t/t7414-submodule-mistakes.sh
@@ -30,7 +30,7 @@  test_expect_success 'no warning when updating entry' '
 
 test_expect_success 'submodule add does not warn' '
 	test_when_finished "git rm -rf submodule .gitmodules" &&
-	git submodule add ./embed submodule 2>stderr &&
+	git -c init.defaultBranch=x submodule add ./embed submodule 2>stderr &&
 	test_i18ngrep ! warning stderr
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 59bbf75e83..772152320a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1202,7 +1202,8 @@  test_create_repo () {
 	mkdir -p "$repo"
 	(
 		cd "$repo" || error "Cannot setup test environment"
-		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
+		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
+			-c init.defaultBranch=master init \
 			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
 		error "cannot run git init -- have you built things yet?"
 		mv .git/hooks .git/hooks-disabled