Allow overriding the default name of the default branch #656
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This supersedes #653 (I asked in a private conversation whether @DEGoodmanWilson is okay with me driving this forward). |
dscho
force-pushed
the
default-branch-name-option
branch
from
0e53e67 to
f680e66
Compare
|
/submit |
|
Submitted as pull.656.git.1591823971.gitgitgadget@gmail.com |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
builtin/fast-export.c
Outdated
| @@ -515,13 +515,17 @@ static const char *anonymize_refname(const char *refname) | |||
| }; | |||
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
> - * We also leave "master" as a special case, since it does not reveal
> - * anything interesting.
> + * We also leave the default branch name as a special case, since it
> + * does not reveal anything interesting.
> */
I feel this is a weird thing to do, since you're trying to anonymize the branch
name,and now the default branch is identifiable with your config file. For
example, if the default branch contains the name of my project/repo then this
sounds like a recipe for accidentally sharing it. I feel a better
alternative would
be to exclude nothing from the anonymization or the proposed default default
branch name
> - if (!strcmp(refname, "refs/heads/master"))
> + if (!default_branch_name)
> + default_branch_name = git_default_branch_name(0);
> +
> + if (!strcmp(refname, default_branch_name))
> return refname;
>
> strbuf_reset(&anon);
> --
> gitgitgadget
>
--
Matthew Rogers
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Matt Rogers <mattr94@gmail.com> writes:
>> - * We also leave "master" as a special case, since it does not reveal
>> - * anything interesting.
>> + * We also leave the default branch name as a special case, since it
>> + * does not reveal anything interesting.
>> */
> I feel this is a weird thing to do, since you're trying to anonymize the branch
> name,and now the default branch is identifiable with your config file. For
> example, if the default branch contains the name of my project/repo then this
> sounds like a recipe for accidentally sharing it. I feel a better
> alternative would
> be to exclude nothing from the anonymization or the proposed default default
> branch name
I wonder if anything bad happens if we keep *no* refs intact in this
function. "Since it does not reveal anything interesting" is an
excuse why not munging it may be OK, but it does not explain why we
prefer not munging it actively.
If there is no reason to keep _some_ refs as-is, I agree that it is
perfectly sensible not to have this special case at all.
Thanks.
>> - if (!strcmp(refname, "refs/heads/master"))
>> + if (!default_branch_name)
>> + default_branch_name = git_default_branch_name(0);
>> +
>> + if (!strcmp(refname, default_branch_name))
>> return refname;
>>
>> strbuf_reset(&anon);
>> --
>> gitgitgadget
>>
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--NHfequSh1hmJPP0s
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-06-10 at 21:54:01, Matt Rogers wrote:
> > - * We also leave "master" as a special case, since it does not =
reveal
> > - * anything interesting.
> > + * We also leave the default branch name as a special case, sin=
ce it
> > + * does not reveal anything interesting.
> > */
> I feel this is a weird thing to do, since you're trying to anonymize the =
branch
> name,and now the default branch is identifiable with your config file. F=
or
> example, if the default branch contains the name of my project/repo then =
this
> sounds like a recipe for accidentally sharing it. I feel a better
> alternative would
> be to exclude nothing from the anonymization or the proposed default defa=
ult
> branch name
I think this is fine because it only reveals the name of your particular
choice of default branch. The goal of the --anonymize option is to
allow people to maintain the structure of their repositories while
stripping private information from them, primarily for debugging
purposes (e.g., providing to us for troubleshooting).
The things people want to prevent exposing are their code, data, project
names, user names, etc.: that is, anything identifying, privileged, or
private. The default branch name isn't any of those things; we know you
have one, and for troubleshooting purposes, we aren't that interested in
what you called it. You've almost certainly picked it out of a set of
one of 20 words that people use for this purpose, none of which are
private, and all of which are shared by millions of other repositories.
In the extremely unlikely case that it does matter, invoking git with
something like "-c default.branch=3D$(git hash-object /dev/null)" would be
sufficient to anonymize all branches.
I should point out that people frequently ask for the output of "git
config -l" for troubleshooting, and most people wouldn't consider their
default branch name to be worth sanitizing there.
--=20
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
--NHfequSh1hmJPP0s
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXuFvHwAKCRB8DEliiIei
gRyiAP9nxf3ysO6zqfMFvHzl5MlLUpSh9dfsrUGzdIp0RNKh2gEAopc7fqS6LJwN
1ZYOWXWAJCaKqdPMtBzOHyfnV1dmvQI=
=ggih
-----END PGP SIGNATURE-----
--NHfequSh1hmJPP0s--
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
> I think this is fine because it only reveals the name of your particular
> choice of default branch. The goal of the --anonymize option is to
> allow people to maintain the structure of their repositories while
> stripping private information from them, primarily for debugging
> purposes (e.g., providing to us for troubleshooting).
>
> The things people want to prevent exposing are their code, data, project
> names, user names, etc.: that is, anything identifying, privileged, or
> private. The default branch name isn't any of those things; we know you
> have one, and for troubleshooting purposes, we aren't that interested in
> what you called it. You've almost certainly picked it out of a set of
> one of 20 words that people use for this purpose, none of which are
> private, and all of which are shared by millions of other repositories.
>
I think that's not very convincing. If branch names in general are identifying
enough to warrant anonymization then shouldn't the default name be too?
> In the extremely unlikely case that it does matter, invoking git with
> something like "-c default.branch=$(git hash-object /dev/null)" would be
> sufficient to anonymize all branches.
>
> I should point out that people frequently ask for the output of "git
> config -l" for troubleshooting, and most people wouldn't consider their
> default branch name to be worth sanitizing there.
I think this is a little presumptuous, most people wouldn't consider it to be
worth sanitizing because there isn't currently such a config setting. If I give
you the the output of "git config -l" then I think it's obvious that all of my
configuration settings will be included (and therefore I can choose to sanitize
accordingly), but if I'm giving an exported repository I think should be
anonymized, but my default branch, which someone could innocently base on a
project or company name, could easily be accidentally included in that output
which could lead to a frustrating experience
> --
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204
--
Matthew Rogers
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Matt Rogers <mattr94@gmail.com> writes:
> I think that's not very convincing. If branch names in general are identifying
> enough to warrant anonymization then shouldn't the default name be too?
It is a good argument. I also heard a rumor that often branch names
contain codewords given to pre-released hardware that are highly
confidential in certain circles, and heard that it is one of the
reasons why Gerrit has server side ACL that lets you hide some
branches from authenticated users that can access other branches.
Again, the original comment explains why 'master' without such a
configuration knob was not worth protecting, but what it does not
explain is why keeping it (and only that branch name) unmunged gives
a more useful result than munging everything. From the point of
view of "I want to debug the shape of the DAG, without the actual
user data", munging 'master' to 'ref47' while other branches like
'next' are munged to 'ref%d' does not make it harder to use or less
useful for the debugging than only 'master' is kept intact in the
output stream.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> A corrected code should return a hardwired constant 'main' (it
>> probably gets behind a C preprocessor macro, but the point is that
>> we do not want end-user customization) for the reason stated in that
>> message.
>
> I like `ref0` better, for two reasons:
>
> - it is more consistent to just have all anonymized branches be named
> `ref<N>`, and
>
> - using `main` both for an original `main` and an original `master` can be
> a bit confusing, as the reader might assume that this branch name (as it
> does not follow the `ref<N>` convention) was _not_ anonymized, when it
> very well might have been.
A pro for keeping a hardcoded 'master' is that it is compatible with
the current world order, and flipping it to hardcoded 'main' upon
transition is just to use the moral equivalent, so we do not need to
immediately have to change anything. The _new_ consistency across
ref<N> does feel attractive, but because it is new, there always is
a pushback not to "fix" what is not broken.
I am personally OK either way.
By the way, we'd need to devise a transition plan for switching the
default branch name (i.e. the name used for the primary branch in a
newly created repository unless the user configures it to some other
value) to 'main' (oh, I just found one reason why I will not want to
use that name in my project(s)---it is too close to 'maint').
It might roughly go like:
1. We introduce core.defaultBranchName; when it is not set, its
value defaults to 'master' in the 1st phase of the transition.
"git init" and "git clone" however issue a warning that says
"unless you configure core.defaultBranchName, we use 'master'
for now for backward compatibility but we will start using
'main' in three major releases of Git in the future". These
commands use the default branch name when creating a new
repository in the 1st phase, and set core.primaryBranchName to
that name in the resulting repository.
This is to encourage early adopters to set it to 'maint'^W'main'
(eek, see, I again made that typo), while allowing those who
have toolset that depends more heavily on the current default
branch name than other people to set it to 'master' for
stability.
In the 1st phase, a few commands that care about what the
primary branch is in a repository (i.e. fmt-merge-msg and
fast-export are the two we have identified so far) pay attention
to the core.primaryBranchName configuration, and default to
'master' if the configuration does not exist.
These commands issue a warning that says "unless you configure
core.primaryBranchName in the repository, we use 'master' for
now but we will start using 'main' in three major releases of
Git in the future".
The above two warning messages will be squelched once the user
sets respective configuration variable.
2. We flip the default for the two variables from 'master' to
'main' in three major releases of Git (i.e. 24-30 weeks from the
1st phase). The two warning messages added for the 1st phase
will be reworded for the updated default. We no longer need to
say "in three major releases" in there.
3. After long time passes, remove the warning.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Sat, 13 Jun 2020, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> A corrected code should return a hardwired constant 'main' (it
> >> probably gets behind a C preprocessor macro, but the point is that
> >> we do not want end-user customization) for the reason stated in that
> >> message.
> >
> > I like `ref0` better, for two reasons:
> >
> > - it is more consistent to just have all anonymized branches be named
> > `ref<N>`, and
> >
> > - using `main` both for an original `main` and an original `master` can be
> > a bit confusing, as the reader might assume that this branch name (as it
> > does not follow the `ref<N>` convention) was _not_ anonymized, when it
> > very well might have been.
>
> A pro for keeping a hardcoded 'master' is that it is compatible with
> the current world order, and flipping it to hardcoded 'main' upon
> transition is just to use the moral equivalent, so we do not need to
> immediately have to change anything. The _new_ consistency across
> ref<N> does feel attractive, but because it is new, there always is
> a pushback not to "fix" what is not broken.
>
> I am personally OK either way.
>
> By the way, we'd need to devise a transition plan for switching the
> default branch name (i.e. the name used for the primary branch in a
> newly created repository unless the user configures it to some other
> value) to 'main' (oh, I just found one reason why I will not want to
> use that name in my project(s)---it is too close to 'maint').
Yes, the trouble with `maint` did cross my mind, but I try not to
"overfit" to git/git. :-)
> It might roughly go like:
>
> 1. We introduce core.defaultBranchName; when it is not set, its
> value defaults to 'master' in the 1st phase of the transition.
> "git init" and "git clone" however issue a warning that says
> "unless you configure core.defaultBranchName, we use 'master'
> for now for backward compatibility but we will start using
> 'main' in three major releases of Git in the future". These
> commands use the default branch name when creating a new
> repository in the 1st phase, and set core.primaryBranchName to
> that name in the resulting repository.
>
> This is to encourage early adopters to set it to 'maint'^W'main'
> (eek, see, I again made that typo), while allowing those who
> have toolset that depends more heavily on the current default
> branch name than other people to set it to 'master' for
> stability.
>
> In the 1st phase, a few commands that care about what the
> primary branch is in a repository (i.e. fmt-merge-msg and
> fast-export are the two we have identified so far) pay attention
> to the core.primaryBranchName configuration, and default to
> 'master' if the configuration does not exist.
>
> These commands issue a warning that says "unless you configure
> core.primaryBranchName in the repository, we use 'master' for
> now but we will start using 'main' in three major releases of
> Git in the future".
>
> The above two warning messages will be squelched once the user
> sets respective configuration variable.
>
> 2. We flip the default for the two variables from 'master' to
> 'main' in three major releases of Git (i.e. 24-30 weeks from the
> 1st phase). The two warning messages added for the 1st phase
> will be reworded for the updated default. We no longer need to
> say "in three major releases" in there.
>
> 3. After long time passes, remove the warning.
Yes, that's what I had in my mind, too (modulo the concrete part about the
three major versions, which is something I would have asked about at some
stage, thank you for answering that question already!).
Thank you,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Yes, the trouble with `maint` did cross my mind, but I try not to
> "overfit" to git/git. :-)
I do not think it is overfitting; if the solution cannot even
support the originating project well, there is something wrong.
Most likely, I'd be tempted to rename it myself away from any name
that is too similar to 'maint'; perhaps to 'stable' (or 'devo', h/t
tla ;-).
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Wed, 17 Jun 2020, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Yes, the trouble with `maint` did cross my mind, but I try not to
> > "overfit" to git/git. :-)
>
> I do not think it is overfitting; if the solution cannot even
> support the originating project well, there is something wrong.
>
> Most likely, I'd be tempted to rename it myself away from any name
> that is too similar to 'maint'; perhaps to 'stable' (or 'devo', h/t
> tla ;-).
You could also use `next` instead of `master`, which would make intuitive
sense because the commits that make it into that branch are slated to be
part of the next major Git version.
And a relatively obvious name for the current `next` might be `cooking`.
I refrained from proposing this earlier, thinking that this would be too
disruptive, but since `pu` was renamed to `seen`...
:-)
Ciao,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> You could also use `next` instead of `master`, which would make intuitive
> sense because the commits that make it into that branch are slated to be
> part of the next major Git version.
>
> And a relatively obvious name for the current `next` might be `cooking`.
>
> I refrained from proposing this earlier, thinking that this would be too
> disruptive, but since `pu` was renamed to `seen`...
Renaming 'pu' away from two-letter has a positive technical and
social effect. Using 'next' for anything but what it currently
means does not have any such upside and only the downside of
confusing people.
As to 'cooking', I am not sure. Personally I consider that the
topics that are in 'next' plus those that are soon to be in 'next'
are all 'cooking'. But I do not think anybody's dying to rename
'next', so...
| @@ -1263,9 +1263,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix) | |||
| remote_head_points_at = NULL; | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> @@ -1263,9 +1263,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> remote_head_points_at = NULL;
> remote_head = NULL;
> option_no_checkout = 1;
> - if (!option_bare)
> - install_branch_config(0, "master", option_origin,
> - "refs/heads/master");
> + if (!option_bare) {
> + char *default_branch = git_default_branch_name(0);
> + const char *nick;
> +
> + if (!skip_prefix(default_branch, "refs/heads/", &nick))
> + BUG("unexpected default branch '%s'",
> + default_branch);
> + install_branch_config(0, nick, option_origin,
> + default_branch);
> + free(default_branch);
> + }
Good catch. Normal clone would follow whatever primary branch the
other side uses by pointing at it with its "HEAD" but this codepath
to deal with a clone of an empty repository needs to use a default,
so this is an appropriate change.
> write_refspec_config(src_ref_prefix, our_head_points_at,
> diff --git a/t/t5609-clone-branch.sh b/t/t5609-clone-branch.sh
> index 6e7a7be0522..66af3ac2669 100755
> --- a/t/t5609-clone-branch.sh
> +++ b/t/t5609-clone-branch.sh
> @@ -67,4 +67,13 @@ test_expect_success 'clone -b not allowed with empty repos' '
> test_must_fail git clone -b branch empty clone-branch-empty
> '
>
> +test_expect_success 'chooses correct default branch name' '
> + GIT_TEST_DEFAULT_BRANCH_NAME= \
> + git -c core.defaultBranchName=up clone empty whats-up &&
> + test_write_lines refs/heads/up refs/heads/up >expect &&
> + git -C whats-up symbolic-ref HEAD >actual &&
> + git -C whats-up config branch.up.merge >>actual &&
> + test_cmp expect actual
> +'
> +
> test_done
fmt-merge-msg.c
Outdated
| @@ -407,7 +407,7 @@ static void fmt_merge_msg_title(struct strbuf *out, | |||
| const char *current_branch) | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When formatting the commit message for merge commits, Git appends "into
> <branch-name>" unless the current branch is the default branch.
>
> Now that we can configure what the default branch name should be, we
> will want to respect that setting in that scenario rather than using the
> compiled-in default branch name.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> fmt-merge-msg.c | 6 ++++--
> t/t6200-fmt-merge-msg.sh | 8 ++++++++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
> index 72d32bd73b1..5e5c1d86f1c 100644
> --- a/fmt-merge-msg.c
> +++ b/fmt-merge-msg.c
> @@ -407,7 +407,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
> const char *current_branch)
> {
> int i = 0;
> - char *sep = "";
> + char *sep = "", *default_branch_name;
>
> strbuf_addstr(out, "Merge ");
> for (i = 0; i < srcs.nr; i++) {
> @@ -451,10 +451,12 @@ static void fmt_merge_msg_title(struct strbuf *out,
> strbuf_addf(out, " of %s", srcs.items[i].string);
> }
>
> - if (!strcmp("master", current_branch))
> + default_branch_name = git_default_branch_name(1);
> + if (!strcmp(default_branch_name, current_branch))
> strbuf_addch(out, '\n');
> else
> strbuf_addf(out, " into %s\n", current_branch);
> + free(default_branch_name);
> }
Good. J6t reminded me earlier about this one when I said "except
for 'init' and 'clone' (when it cannot tell what primary branch the
source of the clone uses), we do not need a hardcoded 'master'", and
the series covers this case. Very pleased.
> static void fmt_tag_signature(struct strbuf *tagbuf,
> diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
> index e4c2a6eca43..a23cd157ffd 100755
> --- a/t/t6200-fmt-merge-msg.sh
> +++ b/t/t6200-fmt-merge-msg.sh
> @@ -158,6 +158,14 @@ test_expect_success 'setup FETCH_HEAD' '
> git fetch . left
> '
>
> +test_expect_success 'with overridden default branch name' '
> + test_config core.defaultBranchName default &&
> + test_when_finished "git switch master" &&
> + git switch -c default &&
> + git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> + ! grep "into default" actual
> +'
> +
> test_expect_success 'merge.log=3 limits shortlog length' '
> cat >expected <<-EOF &&
> Merge branch ${apos}left${apos}
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
| @@ -258,15 +258,17 @@ static int create_default_files(const char *template_path, | |||
| die("failed to set up refs db: %s", err.buf); | |||
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--+Yg8W10oK6rlW0RR
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-06-10 at 21:19:22, Don Goodman-Wilson via GitGitGadget wrote:
> + /* prepend "refs/heads/" to the branch name */
> + prefixed =3D xstrfmt("refs/heads/%s", branch_name);
> + if (check_refname_format(prefixed, 0))
> + die(_("invalid default branch name: '%s'"), branch_name);
I'm glad to see this part and a check for it in the test below. We
wouldn't want a typo to create a broken branch name.
> +test_expect_success 'invalid custom default branch name' '
> + test_must_fail env GIT_TEST_DEFAULT_BRANCH_NAME=3D"with space" \
> + git init custom-invalid 2>err &&
> + test_i18ngrep "invalid default branch name" err
> +'
> +
--=20
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
--+Yg8W10oK6rlW0RR
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXuFrRQAKCRB8DEliiIei
gWh3AQC72M5zaVgqd44QK6VjjLfyxiVHSfnOOSyV3a/g+9EozgD+M7TTfdk7K5r8
OLAKBjiij42iFmjcqHG06qijbKVKpg4=
=KA2m
-----END PGP SIGNATURE-----
--+Yg8W10oK6rlW0RR--
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): |
| @@ -258,15 +258,17 @@ static int create_default_files(const char *template_path, | |||
| die("failed to set up refs db: %s", err.buf); | |||
There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Wed, Jun 10, 2020 at 5:19 PM Don Goodman-Wilson via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> [...]
> To make this process much less cumbersome, let's introduce support for
> `core.defaultBranchName`. That way, users won't need to keep their
> copied template files up to date, and won't interfere with default hooks
> installed by their administrators.
> [...]
> Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com>
> ---
> diff --git a/refs.c b/refs.c
> @@ -560,6 +560,40 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
> + die(_("Could not retrieve `core.defaultBranchName`"));
Nit: here the error message is capitalized...
> + if (from_config)
> + branch_name = from_config;
> + else
> + branch_name = "master";
Non-actionable nit: could be written:
branch_name = from_config ? from_config : "master";
> + }
> +
> + if (short_name)
> + return from_config ? from_config : xstrdup(branch_name);
The logic overall is a bit difficult to follow when trying to
understand when and when not to duplicate the string and when and when
not to free(), but seems to be correct.
> + /* prepend "refs/heads/" to the branch name */
> + prefixed = xstrfmt("refs/heads/%s", branch_name);
> + if (check_refname_format(prefixed, 0))
> + die(_("invalid default branch name: '%s'"), branch_name);
Here, the error message is not capitalized. It would be nice for both
messages to share a common capitalization scheme. These days, we tend
to favor _not_ capitalizing error messages, so perhaps remove
capitalization from the earlier one.
> +/*
> + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> + * branch name will be prefixed with "refs/heads/".
> + */
> +char *git_default_branch_name(int short_name);
Overall, the internal logic regarding duplicating/freeing strings
would probably be easier to grok if there were two separate functions:
char *git_default_branch_name(void);
char *git_default_ref_name(void);
but that's subjective.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Eric,
On Wed, 10 Jun 2020, Eric Sunshine wrote:
> On Wed, Jun 10, 2020 at 5:19 PM Don Goodman-Wilson via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > [...]
> > To make this process much less cumbersome, let's introduce support for
> > `core.defaultBranchName`. That way, users won't need to keep their
> > copied template files up to date, and won't interfere with default hooks
> > installed by their administrators.
> > [...]
> > Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com>
> > ---
> > diff --git a/refs.c b/refs.c
> > @@ -560,6 +560,40 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
> > + die(_("Could not retrieve `core.defaultBranchName`"));
>
> Nit: here the error message is capitalized...
I downcased it...
> > + if (from_config)
> > + branch_name = from_config;
> > + else
> > + branch_name = "master";
>
> Non-actionable nit: could be written:
>
> branch_name = from_config ? from_config : "master";
Good call.
> > + }
> > +
> > + if (short_name)
> > + return from_config ? from_config : xstrdup(branch_name);
>
> The logic overall is a bit difficult to follow when trying to
> understand when and when not to duplicate the string and when and when
> not to free(), but seems to be correct.
I agree that it is a bit hard to follow, but then, the function is really
short, so I hoped it is okay.
> > + /* prepend "refs/heads/" to the branch name */
> > + prefixed = xstrfmt("refs/heads/%s", branch_name);
> > + if (check_refname_format(prefixed, 0))
> > + die(_("invalid default branch name: '%s'"), branch_name);
>
> Here, the error message is not capitalized. It would be nice for both
> messages to share a common capitalization scheme. These days, we tend
> to favor _not_ capitalizing error messages, so perhaps remove
> capitalization from the earlier one.
>
> > +/*
> > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > + * branch name will be prefixed with "refs/heads/".
> > + */
> > +char *git_default_branch_name(int short_name);
>
> Overall, the internal logic regarding duplicating/freeing strings
> would probably be easier to grok if there were two separate functions:
>
> char *git_default_branch_name(void);
> char *git_default_ref_name(void);
>
> but that's subjective.
For such a tiny nuance, I'd rather keep it as one function...
Thank you,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Overall, the internal logic regarding duplicating/freeing strings
>> would probably be easier to grok if there were two separate functions:
>>
>> char *git_default_branch_name(void);
>> char *git_default_ref_name(void);
>>
>> but that's subjective.
>
> For such a tiny nuance, I'd rather keep it as one function...
And you'd need two functions, default and primary, possibly full and
short. Splitting these into two here would mean you'd need four.
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote:
> > +/*
> > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > + * branch name will be prefixed with "refs/heads/".
> > + */
> > +char *git_default_branch_name(int short_name);
>
> Overall, the internal logic regarding duplicating/freeing strings
> would probably be easier to grok if there were two separate functions:
>
> char *git_default_branch_name(void);
> char *git_default_ref_name(void);
>
> but that's subjective.
Having seen one of the callers, might it be worth avoiding handing off
ownership of the string entirely?
I.e., this comes from a string that's already owned for the lifetime of
the process (either the environment, or a string stored by the config
machinery). Could we just pass that back (or if we want to be more
careful about getenv() lifetimes, we can copy it into a static owned by
this function)?
Then all of the callers can stop dealing with the extra free(), and you
can do:
const char *git_default_branch_name(void)
{
return skip_prefix("refs/heads/", git_default_ref_name());
}
-Peff
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Tue, Jun 16, 2020 at 08:45:02AM -0400, Jeff King wrote:
> On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote:
>
> > > +/*
> > > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > > + * branch name will be prefixed with "refs/heads/".
> > > + */
> > > +char *git_default_branch_name(int short_name);
> >
> > Overall, the internal logic regarding duplicating/freeing strings
> > would probably be easier to grok if there were two separate functions:
> >
> > char *git_default_branch_name(void);
> > char *git_default_ref_name(void);
> >
> > but that's subjective.
>
> Having seen one of the callers, might it be worth avoiding handing off
> ownership of the string entirely?
>
> I.e., this comes from a string that's already owned for the lifetime of
> the process (either the environment, or a string stored by the config
> machinery). Could we just pass that back (or if we want to be more
> careful about getenv() lifetimes, we can copy it into a static owned by
> this function)?
>
> Then all of the callers can stop dealing with the extra free(), and you
> can do:
>
> const char *git_default_branch_name(void)
> {
> return skip_prefix("refs/heads/", git_default_ref_name());
> }
Actually, one small hiccup is that the config option specifies the
branch name, not the ref name. So you really would have to prepare a
static-owned copy of it to turn "foo" into "refs/heads/foo" to get the
refname.
On the other hand, that would also be a good time to run
check_ref_format(). In the patch as-is, the "short" return does not
check that the branch is a valid name.
-Peff
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Peff,
On Tue, 16 Jun 2020, Jeff King wrote:
> On Tue, Jun 16, 2020 at 08:45:02AM -0400, Jeff King wrote:
>
> > On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote:
> >
> > > > +/*
> > > > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > > > + * branch name will be prefixed with "refs/heads/".
> > > > + */
> > > > +char *git_default_branch_name(int short_name);
> > >
> > > Overall, the internal logic regarding duplicating/freeing strings
> > > would probably be easier to grok if there were two separate functions:
> > >
> > > char *git_default_branch_name(void);
> > > char *git_default_ref_name(void);
> > >
> > > but that's subjective.
> >
> > Having seen one of the callers, might it be worth avoiding handing off
> > ownership of the string entirely?
> >
> > I.e., this comes from a string that's already owned for the lifetime of
> > the process (either the environment, or a string stored by the config
> > machinery). Could we just pass that back (or if we want to be more
> > careful about getenv() lifetimes, we can copy it into a static owned by
> > this function)?
> >
> > Then all of the callers can stop dealing with the extra free(), and you
> > can do:
> >
> > const char *git_default_branch_name(void)
> > {
> > return skip_prefix("refs/heads/", git_default_ref_name());
> > }
>
> Actually, one small hiccup is that the config option specifies the
> branch name, not the ref name. So you really would have to prepare a
> static-owned copy of it to turn "foo" into "refs/heads/foo" to get the
> refname.
>
> On the other hand, that would also be a good time to run
> check_ref_format(). In the patch as-is, the "short" return does not
> check that the branch is a valid name.
Legit.
I will work on this.
Thanks,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Peff,
On Tue, 16 Jun 2020, Jeff King wrote:
> On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote:
>
> > > +/*
> > > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > > + * branch name will be prefixed with "refs/heads/".
> > > + */
> > > +char *git_default_branch_name(int short_name);
> >
> > Overall, the internal logic regarding duplicating/freeing strings
> > would probably be easier to grok if there were two separate functions:
> >
> > char *git_default_branch_name(void);
> > char *git_default_ref_name(void);
> >
> > but that's subjective.
>
> Having seen one of the callers, might it be worth avoiding handing off
> ownership of the string entirely?
For `git_default_branch_name()`: yes. For `repo_default_branch_name()`,
not really, as that is potentially repository-specific.
(Side note: while I cannot really think of a use case where you would want
to set `init.defaultBranch` in a repository-local config, there _might_ be
use cases for that out there, and it _is_ how our config machinery works.)
> I.e., this comes from a string that's already owned for the lifetime of
> the process (either the environment, or a string stored by the config
> machinery). Could we just pass that back (or if we want to be more
> careful about getenv() lifetimes, we can copy it into a static owned by
> this function)?
>
> Then all of the callers can stop dealing with the extra free(), and you
> can do:
>
> const char *git_default_branch_name(void)
> {
> return skip_prefix("refs/heads/", git_default_ref_name());
> }
For ease of use, I decided to only ever return the branch name (but check
the full ref).
Those callers that actually need the full ref usually also need the branch
name, and it is easy enough to call `xstrfmt("refs/heads/%s", ...)`.
It might make the code a bit less efficient (but who cares, it's not like
we're setting up a gazillion repositories per second all the time), but
quite a bit easier to reason about.
Ciao,
Dscho
Documentation/config/core.txt
Outdated
| @@ -626,3 +626,7 @@ core.abbrev:: | |||
| in your repository, which hopefully is enough for | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
> index 74619a9c03b..a11e1abdf59 100644
> --- a/Documentation/config/core.txt
> +++ b/Documentation/config/core.txt
> @@ -626,3 +626,7 @@ core.abbrev::
> in your repository, which hopefully is enough for
> abbreviated object names to stay unique for some time.
> The minimum length is 4.
> +
> +core.defaultBranchName::
> + Allows overriding the default branch name e.g. when initializing
> + a new repository or when cloning an empty repository.
As we saw in [PATCH 7/9], it also affects for which branch an
auto-generated merge message omits "into $branch" at the end.
The behaviour change of "merge" might matter more than we think here
in a hand-wavy way, and it certainly does affect more than 'init'
and 'clone' (both of which are one time operation per repository),
because setting this in ~/.gitconfig would affect all the
repositories, whose merges into their 'master' suddenly starts
saying "into master" at the end, unlike existing merges. So it
probably is worth highlighting it here.
> diff --git a/t/README b/t/README
> index cf863837ab9..b32f520a27f 100644
> --- a/t/README
> +++ b/t/README
> @@ -421,6 +421,10 @@ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=<boolean>, when true (which is
> the default when running tests), errors out when an abbreviated option
> is used.
>
> +GIT_TEST_DEFAULT_BRANCH_NAME allows overriding the default branch name
> +that is used for example when initializing new repositories, or when
> +cloning a repository that has no branches yet.
> +
> Naming Tests
> ------------
|
On the Git mailing list, Taylor Blau wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
| @@ -258,15 +258,17 @@ static int create_default_files(const char *template_path, | |||
| die("failed to set up refs db: %s", err.buf); | |||
There was a problem hiding this comment.
On the Git mailing list, Phillip Wood wrote (reply to this):
On 10/06/2020 22:19, Don Goodman-Wilson via GitGitGadget wrote:
> From: Don Goodman-Wilson <don@goodman-wilson.com>
>
> There is a growing number of projects trying to avoid the non-inclusive
> name `master` in their repositories.
I think it would be helpful to explain why 'master' is no-inclusive even
if it originates from the concept of a master copy. i.e. it suggests
master/slave even if git is not based on that concept.
Have you got some examples of projects that have changed and the names
that they are using? I think it would be helpful if we can agree on a
replacement for master - if every repository uses a different name for
its main branch it adds an extra complication for new contributors to
those projects.
For existing repositories, this
> requires manual work. For new repositories, the only way to do that
> automatically is by copying all of Git's template directory, then
> hard-coding the desired default branch name into the `.git/HEAD` file,
> and then configuring `init.templateDir` to point to those copied
> template files.
>
> To make this process much less cumbersome, let's introduce support for
> `core.defaultBranchName`. That way, users won't need to keep their
> copied template files up to date, and won't interfere with default hooks
> installed by their administrators.
>
> While at it, also let users set the default branch name via the
> environment variable `GIT_TEST_DEFAULT_BRANCH_NAME`,
I'm not sure we usually promote the use of GIT_TEST_... environment
variables outside of the test suite.
> in preparation for
> adjusting Git's test suite to a more inclusive default branch name. As
> is common in Git, the `GIT_TEST_*` variable takes precedence over the
> config setting.
>
> Note: we use the prefix `core.` instead of `init.` because we want to
> adjust also `git clone`, `git fmt-merge-msg` and other commands over the
> course of the next commits to respect this setting.
>
> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Helped-by: Derrick Stolee <dstolee@microsoft.com>
> Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com>
> ---
> builtin/init-db.c | 8 +++++---
> refs.c | 34 ++++++++++++++++++++++++++++++++++
> refs.h | 6 ++++++
> t/t0001-init.sh | 20 ++++++++++++++++++++
> 4 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 0b7222e7188..99792adfd43 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -258,15 +258,17 @@ static int create_default_files(const char *template_path,
> die("failed to set up refs db: %s", err.buf);
>
> /*
> - * Create the default symlink from ".git/HEAD" to the "master"
> - * branch, if it does not exist yet.
> + * Create the default symlink from ".git/HEAD" to the default
> + * branch name, if it does not exist yet.
> */
> path = git_path_buf(&buf, "HEAD");
> reinit = (!access(path, R_OK)
> || readlink(path, junk, sizeof(junk)-1) != -1);
> if (!reinit) {
> - if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
> + char *default_ref = git_default_branch_name(0);
> + if (create_symref("HEAD", default_ref, NULL) < 0)
> exit(1);
> + free(default_ref);
> }
>
> initialize_repository_version(fmt->hash_algo);
> diff --git a/refs.c b/refs.c
> index 224ff66c7bb..8499b3865cb 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -560,6 +560,40 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
> argv_array_pushf(prefixes, *p, len, prefix);
> }
>
> +char *git_default_branch_name(int short_name)
> +{
> + const char *branch_name = getenv("GIT_TEST_DEFAULT_BRANCH_NAME");
> + char *from_config = NULL, *prefixed;
> +
> + /*
> + * If the default branch name was not specified via the environment
> + * variable GIT_TEST_DEFAULT_BRANCH_NAME, retrieve it from the config
> + * setting core.defaultBranchName. If neither are set, fall back to the
> + * hard-coded default.
> + */
> + if (!branch_name || !*branch_name) {
> + if (git_config_get_string("core.defaultbranchname",
> + &from_config) < 0)
> + die(_("Could not retrieve `core.defaultBranchName`"));
> +
> + if (from_config)
> + branch_name = from_config;
> + else
> + branch_name = "master";
> + }
> +
> + if (short_name)
> + return from_config ? from_config : xstrdup(branch_name);
If short_name is set we return without validating the name is that
intentional?
> +
> + /* prepend "refs/heads/" to the branch name */
> + prefixed = xstrfmt("refs/heads/%s", branch_name);
> + if (check_refname_format(prefixed, 0))
> + die(_("invalid default branch name: '%s'"), branch_name);
> +
> + free(from_config);
> + return prefixed;
> +}
> +
> /*
> * *string and *len will only be substituted, and *string returned (for
> * later free()ing) if the string passed in is a magic short-hand form
> diff --git a/refs.h b/refs.h
> index a92d2c74c83..e8d4f6e2f13 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -154,6 +154,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len, struct object_
> int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
> int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
>
> +/*
> + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> + * branch name will be prefixed with "refs/heads/".
Isn't the other way around - the branch name is prefixed with
"refs/heads/" if short is zero.
> + */
> +char *git_default_branch_name(int short_name);
> +
> /*
> * A ref_transaction represents a collection of reference updates that
> * should succeed or fail together.
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 1edd5aeb8f0..b144cd8f46b 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -464,4 +464,24 @@ test_expect_success MINGW 'redirect std handles' '
> grep "Needed a single revision" output.txt
> '
>
> +test_expect_success 'custom default branch name from config' '
> + git config --global core.defaultbranchname nmb &&
In tests we usually use 'test_config' rather than 'git config' as the
former automatically cleans up the config at the end of the test.
> + GIT_TEST_DEFAULT_BRANCH_NAME= git init custom-config &&
> + git config --global --unset core.defaultbranchname &&
> + git -C custom-config symbolic-ref HEAD >actual &&
> + grep nmb actual
> +'
> +
> +test_expect_success 'custom default branch name from env' '
> + GIT_TEST_DEFAULT_BRANCH_NAME=nmb git init custom-env &&
It would be good to test that this overrides the config setting
Best Wishes
Phillip
> + git -C custom-env symbolic-ref HEAD >actual &&
> + grep nmb actual
> +'
> +
> +test_expect_success 'invalid custom default branch name' '
> + test_must_fail env GIT_TEST_DEFAULT_BRANCH_NAME="with space" \
> + git init custom-invalid 2>err &&
> + test_i18ngrep "invalid default branch name" err
> +'
> +
> test_done
>
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Phillip,
On Thu, 11 Jun 2020, Phillip Wood wrote:
> On 10/06/2020 22:19, Don Goodman-Wilson via GitGitGadget wrote:
> > From: Don Goodman-Wilson <don@goodman-wilson.com>
> >
> > There is a growing number of projects trying to avoid the non-inclusive
> > name `master` in their repositories.
>
> I think it would be helpful to explain why 'master' is no-inclusive even
> if it originates from the concept of a master copy. i.e. it suggests
> master/slave even if git is not based on that concept.
Unfortunately, we do not even have that defense: the term `master` was
copied from BitKeeper, which firmly uses it in the `master/slave` context.
See e.g.
https://mail.gnome.org/archives/desktop-devel-list/2019-May/msg00066.html
I added a _brief_ extension to the context to the first commit's commit
message. However, I do not want to go into details here because _this_
patch series is only about empowering users to change their default main
branch name.
> Have you got some examples of projects that have changed and the names
> that they are using?
Yes, there are plenty examples, and I do not want to pick a tiny subset to
demonstrate that. A more useful resource is probably this post:
https://www.hanselman.com/blog/EasilyRenameYourGitDefaultBranchFromMasterToMain.aspx
> I think it would be helpful if we can agree on a replacement for master
> - if every repository uses a different name for its main branch it adds
> an extra complication for new contributors to those projects.
Sure, and that's what I thought we'd discuss, too, maybe at the meeting I
proposed elsewhere (Emily started a new thread about it:
https://lore.kernel.org/git/20200610222719.GE148632@google.com/).
> > For existing repositories, this
> > requires manual work. For new repositories, the only way to do that
> > automatically is by copying all of Git's template directory, then
> > hard-coding the desired default branch name into the `.git/HEAD` file,
> > and then configuring `init.templateDir` to point to those copied
> > template files.
> >
> > To make this process much less cumbersome, let's introduce support for
> > `core.defaultBranchName`. That way, users won't need to keep their
> > copied template files up to date, and won't interfere with default hooks
> > installed by their administrators.
> >
> > While at it, also let users set the default branch name via the
> > environment variable `GIT_TEST_DEFAULT_BRANCH_NAME`,
>
> I'm not sure we usually promote the use of GIT_TEST_... environment
> variables outside of the test suite.
True. Together with Alban's suggestion to make this purely work in the
test suite (i.e. not even adjust the C code to respect that environment
variable), you convinced me to (re-)move that part of the commit.
> > diff --git a/refs.c b/refs.c
> > index 224ff66c7bb..8499b3865cb 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -560,6 +560,40 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
> > argv_array_pushf(prefixes, *p, len, prefix);
> > }
> >
> > +char *git_default_branch_name(int short_name)
> > +{
> > + const char *branch_name = getenv("GIT_TEST_DEFAULT_BRANCH_NAME");
> > + char *from_config = NULL, *prefixed;
> > +
> > + /*
> > + * If the default branch name was not specified via the environment
> > + * variable GIT_TEST_DEFAULT_BRANCH_NAME, retrieve it from the config
> > + * setting core.defaultBranchName. If neither are set, fall back to the
> > + * hard-coded default.
> > + */
> > + if (!branch_name || !*branch_name) {
> > + if (git_config_get_string("core.defaultbranchname",
> > + &from_config) < 0)
> > + die(_("Could not retrieve `core.defaultBranchName`"));
> > +
> > + if (from_config)
> > + branch_name = from_config;
> > + else
> > + branch_name = "master";
> > + }
> > +
> > + if (short_name)
> > + return from_config ? from_config : xstrdup(branch_name);
>
> If short_name is set we return without validating the name is that
> intentional?
No, unintentional. Thank you for pointing this out. It will be fixed in v2
(still working on it).
> > +
> > + /* prepend "refs/heads/" to the branch name */
> > + prefixed = xstrfmt("refs/heads/%s", branch_name);
> > + if (check_refname_format(prefixed, 0))
> > + die(_("invalid default branch name: '%s'"), branch_name);
> > +
> > + free(from_config);
> > + return prefixed;
> > +}
> > +
> > /*
> > * *string and *len will only be substituted, and *string returned (for
> > * later free()ing) if the string passed in is a magic short-hand form
> > diff --git a/refs.h b/refs.h
> > index a92d2c74c83..e8d4f6e2f13 100644
> > --- a/refs.h
> > +++ b/refs.h
> > @@ -154,6 +154,12 @@ int repo_dwim_log(struct repository *r, const char *str, int len, struct object_
> > int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
> > int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
> >
> > +/*
> > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > + * branch name will be prefixed with "refs/heads/".
>
> Isn't the other way around - the branch name is prefixed with
> "refs/heads/" if short is zero.
Absolutely. Thank you for your careful review, I read past this at least
half a dozen times.
> > + */
> > +char *git_default_branch_name(int short_name);
> > +
> > /*
> > * A ref_transaction represents a collection of reference updates that
> > * should succeed or fail together.
> > diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> > index 1edd5aeb8f0..b144cd8f46b 100755
> > --- a/t/t0001-init.sh
> > +++ b/t/t0001-init.sh
> > @@ -464,4 +464,24 @@ test_expect_success MINGW 'redirect std handles' '
> > grep "Needed a single revision" output.txt
> > '
> >
> > +test_expect_success 'custom default branch name from config' '
> > + git config --global core.defaultbranchname nmb &&
>
> In tests we usually use 'test_config' rather than 'git config' as the
> former automatically cleans up the config at the end of the test.
Right, and in this instance it is `test_config_global`.
> > + GIT_TEST_DEFAULT_BRANCH_NAME= git init custom-config &&
> > + git config --global --unset core.defaultbranchname &&
> > + git -C custom-config symbolic-ref HEAD >actual &&
> > + grep nmb actual
> > +'
> > +
> > +test_expect_success 'custom default branch name from env' '
> > + GIT_TEST_DEFAULT_BRANCH_NAME=nmb git init custom-env &&
>
> It would be good to test that this overrides the config setting
Except that we'll make this a thing that is internal to the test suite
now. So this test case can go.
Thank you for helping me improve the patch series!
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I added a _brief_ extension to the context to the first commit's commit
> message. However, I do not want to go into details here because _this_
> patch series is only about empowering users to change their default main
> branch name.
Sensible.
And I do not think the planned follow-up work to rename 'master' to
something else needs to be defended with lengthy history lessons.
Sufficiently large part of the user population are unhappy with the
use of the word 'master' as the default name of the primary branch
in a newly created repository, and the mere fact that we are aware
of that is good enough justification to move _away_ from 'master'.
In other words, we do not have to explain why 'master' was bad, as
it does not have to be bad for everybody to be replaced.
But you need to defend that the new word you picked is something
everybody is happy with. That is much harder ;-).
There was a problem hiding this comment.
On the Git mailing list, Phillip Wood wrote (reply to this):
On 12/06/2020 17:51, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> I added a _brief_ extension to the context to the first commit's commit
>> message. However, I do not want to go into details here because _this_
>> patch series is only about empowering users to change their default main
>> branch name.
>
> [...]
>
> Sufficiently large part of the user population are unhappy with the
> use of the word 'master' as the default name of the primary branch
> in a newly created repository, and the mere fact that we are aware
> of that is good enough justification to move _away_ from 'master'.
> In other words, we do not have to explain why 'master' was bad, as
> it does not have to be bad for everybody to be replaced.
This expresses what I was trying to get at with my comments much more
clearly than I managed - Thank you
>
> But you need to defend that the new word you picked is something
> everybody is happy with. That is much harder ;-).
Indeed, though I am more optimistic about that having seen a couple of
people have indicated that they don't mind too much what we choose so
long as it unlikely to cause future problems.
Best Wishes
Phillip
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Fri, 12 Jun 2020, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > I added a _brief_ extension to the context to the first commit's commit
> > message. However, I do not want to go into details here because _this_
> > patch series is only about empowering users to change their default main
> > branch name.
>
> Sensible.
>
> And I do not think the planned follow-up work to rename 'master' to
> something else needs to be defended with lengthy history lessons.
Yes, I agree. It is probably sufficient to just point at a couple
high-profile projects that already made the switch to `main`.
> Sufficiently large part of the user population are unhappy with the
> use of the word 'master' as the default name of the primary branch
> in a newly created repository, and the mere fact that we are aware
> of that is good enough justification to move _away_ from 'master'.
Yes, even Pasky says he regrets the choice of term (see
https://twitter.com/xpasky/status/1271477451756056577):
I picked the names "master" (and "origin") in the early Git tooling
back in 2005.
(this probably means you shouldn't give much weight to my name
preferences :) )
I have wished many times I would have named them "main" (and
"upstream") instead.
> In other words, we do not have to explain why 'master' was bad, as
> it does not have to be bad for everybody to be replaced.
>
> But you need to defend that the new word you picked is something
> everybody is happy with. That is much harder ;-).
To be honest, I stopped looking for got arguments in favor of one
name after a couple days, and instead focused on the consensus I
saw: Chrome [*1*] and node.js [*2*] apparently stated publicly that
they want to change their main branches to `main`, and GitLab [*3*]
and GitHub [*4*] seem to intend to change the default for new
repositories accordingly.
It is not like we have to decide for the community (as we did back
in 2005). I am actually quite relieved about that.
Ciao,
Dscho
URL *1*: https://twitter.com/Una/status/1271180494944829441
URL *2*: https://github.com/nodejs/node/issues/33864
URL *3*: https://gitlab.com/gitlab-org/gitlab/-/issues/221164
URL *4*: https://twitter.com/natfriedman/status/1271253144442253312
(this is not really a public announcement, I agree, but it is a
public Tweet by the CEO)
| @@ -258,15 +258,17 @@ static int create_default_files(const char *template_path, | |||
| die("failed to set up refs db: %s", err.buf); | |||
There was a problem hiding this comment.
On the Git mailing list, Alban Gruin wrote (reply to this):
Hi Don & Johannes,
Le 10/06/2020 à 23:19, Don Goodman-Wilson via GitGitGadget a écrit :
> From: Don Goodman-Wilson <don@goodman-wilson.com>
>
> There is a growing number of projects trying to avoid the non-inclusive
> name `master` in their repositories. For existing repositories, this
> requires manual work. For new repositories, the only way to do that
> automatically is by copying all of Git's template directory, then
> hard-coding the desired default branch name into the `.git/HEAD` file,
> and then configuring `init.templateDir` to point to those copied
> template files.
>
> To make this process much less cumbersome, let's introduce support for
> `core.defaultBranchName`. That way, users won't need to keep their
> copied template files up to date, and won't interfere with default hooks
> installed by their administrators.
>
> While at it, also let users set the default branch name via the
> environment variable `GIT_TEST_DEFAULT_BRANCH_NAME`, in preparation for
> adjusting Git's test suite to a more inclusive default branch name. As
> is common in Git, the `GIT_TEST_*` variable takes precedence over the
> config setting.
>
Why adding yet another environment variable instead of relying only on a
config option? I understand it's for the tests, but can't we add a
shell function in test-lib.sh (and friends) that tries to read
`GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
`core.defaultBranchName'?
Cheers,
Alban
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Alban Gruin <alban.gruin@gmail.com> writes:
> Why adding yet another environment variable instead of relying only on a
> config option? I understand it's for the tests, but can't we add a
> shell function in test-lib.sh (and friends) that tries to read
> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> `core.defaultBranchName'?
Can you produce such a patch that does it cleanly? My knee jerk
reaction is that I would suspect that you end up having to touch
many places in the t/ scripts, but if you prove otherwise, that
would certainly be appreciated.
And no,
git () { command git -c core.defaultBranchName=master "$@" }
is not an acceptable solution.
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
--YLwdFo+Xu6B7B0Yp
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On 2020-06-11 at 23:14:46, Junio C Hamano wrote:
> Alban Gruin <alban.gruin@gmail.com> writes:
>=20
> > Why adding yet another environment variable instead of relying only on a
> > config option? I understand it's for the tests, but can't we add a
> > shell function in test-lib.sh (and friends) that tries to read
> > `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> > `core.defaultBranchName'?
>=20
> Can you produce such a patch that does it cleanly? My knee jerk
> reaction is that I would suspect that you end up having to touch
> many places in the t/ scripts, but if you prove otherwise, that
> would certainly be appreciated.
>=20
> And no,=20
>=20
> git () { command git -c core.defaultBranchName=3Dmaster "$@" }
>=20
> is not an acceptable solution.
I would also be delighted to see such a solution, but my experience with
the SHA-256 work tells me there's unlikely to be one. We do a lot of
"git init" operations in random places in the test suite and as a
consequence it's very hard to make a change without touching a large
number of tests.
If we were writing things today, perhaps we would use a function (e.g.,
test_init_repo or such) to wrap this case, but we unfortunately didn't
think about that and we're stuck with what we have now unless someone
retrofits the test suite.
--=20
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
--YLwdFo+Xu6B7B0Yp
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.2.20 (GNU/Linux)
iHUEABYKAB0WIQQILOaKnbxl+4PRw5F8DEliiIeigQUCXuLCQwAKCRB8DEliiIei
gfboAPwPOWvrtndE4Tf6OyE/Dlrn0w0VVNPWD6Wbo2G/I/jVHwD/QkWS8+mUYFKb
f/yZFjgAyHz+el1vM15ysxgSkrQo6Qk=
=SFrp
-----END PGP SIGNATURE-----
--YLwdFo+Xu6B7B0Yp--
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi brian,
On Thu, 11 Jun 2020, brian m. carlson wrote:
> On 2020-06-11 at 23:14:46, Junio C Hamano wrote:
> > Alban Gruin <alban.gruin@gmail.com> writes:
> >
> > > Why adding yet another environment variable instead of relying only on a
> > > config option? I understand it's for the tests, but can't we add a
> > > shell function in test-lib.sh (and friends) that tries to read
> > > `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> > > `core.defaultBranchName'?
> >
> > Can you produce such a patch that does it cleanly? My knee jerk
> > reaction is that I would suspect that you end up having to touch
> > many places in the t/ scripts, but if you prove otherwise, that
> > would certainly be appreciated.
> >
> > And no,
> >
> > git () { command git -c core.defaultBranchName=master "$@" }
> >
> > is not an acceptable solution.
>
> I would also be delighted to see such a solution, but my experience with
> the SHA-256 work tells me there's unlikely to be one. We do a lot of
> "git init" operations in random places in the test suite and as a
> consequence it's very hard to make a change without touching a large
> number of tests.
That's a valid point, indeed.
> If we were writing things today, perhaps we would use a function (e.g.,
> test_init_repo or such) to wrap this case, but we unfortunately didn't
> think about that and we're stuck with what we have now unless someone
> retrofits the test suite.
There is actually `test_create_repo` (see
https://github.com/git/git/blob/v2.27.0/t/test-lib-functions.sh#L1145-L1159):
# Most tests can use the created repository, but some may need to create
# more.
# Usage: test_create_repo <directory>
test_create_repo () {
test "$#" = 1 ||
BUG "not 1 parameter to test-create-repo"
repo="$1"
mkdir -p "$repo"
(
cd "$repo" || error "Cannot setup test environment"
"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" 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
) || exit
}
But I agree that few test scripts use it:
$ git grep 'git init' v2.27.0 -- t/ | wc -l
765
$ git grep 'test_create_repo' v2.27.0 -- t/ | wc -l
296
Ciao,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Alban Gruin wrote (reply to this):
Hi Junio,
Le 12/06/2020 à 01:14, Junio C Hamano a écrit :
> Alban Gruin <alban.gruin@gmail.com> writes:
>
>> Why adding yet another environment variable instead of relying only on a
>> config option? I understand it's for the tests, but can't we add a
>> shell function in test-lib.sh (and friends) that tries to read
>> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
>> `core.defaultBranchName'?
>
> Can you produce such a patch that does it cleanly? My knee jerk
> reaction is that I would suspect that you end up having to touch
> many places in the t/ scripts, but if you prove otherwise, that
> would certainly be appreciated.
>
> And no,
>
> git () { command git -c core.defaultBranchName=master "$@" }
>
> is not an acceptable solution.
>
I wanted to to do something like this:
if test -n "$GIT_TEST_DEFAULT_BRANCH_NAME";
then
git config core.defaultBranchName "$GIT_TEST_DEFAULT_BRANCH_NAME"
fi
But since we do not have a repository to store the config, it won't
work. Sorry for the noise.
Alban
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323328-1464577387-1592125063=:56
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hi Alban,
On Sat, 13 Jun 2020, Alban Gruin wrote:
> Hi Junio,
>
> Le 12/06/2020 =C3=A0 01:14, Junio C Hamano a =C3=A9crit=C2=A0:
> > Alban Gruin <alban.gruin@gmail.com> writes:
> >
> >> Why adding yet another environment variable instead of relying only o=
n a
> >> config option? I understand it's for the tests, but can't we add a
> >> shell function in test-lib.sh (and friends) that tries to read
> >> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> >> `core.defaultBranchName'?
> >
> > Can you produce such a patch that does it cleanly? My knee jerk
> > reaction is that I would suspect that you end up having to touch
> > many places in the t/ scripts, but if you prove otherwise, that
> > would certainly be appreciated.
> >
> > And no,
> >
> > git () { command git -c core.defaultBranchName=3Dmaster "$@" }
> >
> > is not an acceptable solution.
> >
>
> I wanted to to do something like this:
>
> if test -n "$GIT_TEST_DEFAULT_BRANCH_NAME";
> then
> git config core.defaultBranchName "$GIT_TEST_DEFAULT_BRANCH_NAME"
> fi
>
> But since we do not have a repository to store the config, it won't
> work. Sorry for the noise.
We actually would have `~/.gitconfig` because `HOME` is set to `t/trash
directory.<test-name>/`.
However, that would cause all kinds of issues when test scripts expect the
directory to be pristine, containing only `.git/` but not `.gitconfig`.
It was a good idea to bring up; I share brian's sentiment that it would
have been nice if it would have worked out.
Ciao,
Dscho
--8323328-1464577387-1592125063=:56--
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Sun, Jun 14, 2020 at 10:57:41AM +0200, Johannes Schindelin wrote:
> > >> Why adding yet another environment variable instead of relying only on a
> > >> config option? I understand it's for the tests, but can't we add a
> > >> shell function in test-lib.sh (and friends) that tries to read
> > >> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> > >> `core.defaultBranchName'?
> > >
> > > Can you produce such a patch that does it cleanly? My knee jerk
> > > reaction is that I would suspect that you end up having to touch
> > > many places in the t/ scripts, but if you prove otherwise, that
> > > would certainly be appreciated.
> > >
> > > And no,
> > >
> > > git () { command git -c core.defaultBranchName=master "$@" }
> > >
> > > is not an acceptable solution.
> > >
> >
> > I wanted to to do something like this:
> >
> > if test -n "$GIT_TEST_DEFAULT_BRANCH_NAME";
> > then
> > git config core.defaultBranchName "$GIT_TEST_DEFAULT_BRANCH_NAME"
> > fi
> >
> > But since we do not have a repository to store the config, it won't
> > work. Sorry for the noise.
>
> We actually would have `~/.gitconfig` because `HOME` is set to `t/trash
> directory.<test-name>/`.
>
> However, that would cause all kinds of issues when test scripts expect the
> directory to be pristine, containing only `.git/` but not `.gitconfig`.
Putting:
GIT_CONFIG_PARAMETERS="'core.defaultBranchName=...'"
into the environment would work (and yes, you need the single quotes
embedded in the variable), and solves all of the complaints above.
Further "git -c" invocations properly append to it. But:
- there are a few tests which explicitly tweak that variable
- it technically changes any tests of "-c" because now we'd never
cover the case where we start without the variable defined
I think baking in a special environment variable like you have is not so
bad. If this did become too common a pattern, though (special test-only
environment variables that do have a separate config option), I wouldn't
be opposed to a GIT_TEST_CONFIG_PARAMETERS which takes precedence over
other config, and comes with a big warning label that it shouldn't be
relied upon outside the test suite. That's equally ugly to
GIT_TEST_DEFAULT_BRANCH_NAME, but at least solves the problem once for
all of them. I'm just not sure we have enough "all of them" to make it
worth doing.
-Peff
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Peff,
On Tue, 16 Jun 2020, Jeff King wrote:
> On Sun, Jun 14, 2020 at 10:57:41AM +0200, Johannes Schindelin wrote:
>
> > > >> Why adding yet another environment variable instead of relying only on a
> > > >> config option? I understand it's for the tests, but can't we add a
> > > >> shell function in test-lib.sh (and friends) that tries to read
> > > >> `GIT_TEST_DEFAULT_BRANCH_NAME', and, if it exists, sets
> > > >> `core.defaultBranchName'?
> > > >
> > > > Can you produce such a patch that does it cleanly? My knee jerk
> > > > reaction is that I would suspect that you end up having to touch
> > > > many places in the t/ scripts, but if you prove otherwise, that
> > > > would certainly be appreciated.
> > > >
> > > > And no,
> > > >
> > > > git () { command git -c core.defaultBranchName=master "$@" }
> > > >
> > > > is not an acceptable solution.
> > > >
> > >
> > > I wanted to to do something like this:
> > >
> > > if test -n "$GIT_TEST_DEFAULT_BRANCH_NAME";
> > > then
> > > git config core.defaultBranchName "$GIT_TEST_DEFAULT_BRANCH_NAME"
> > > fi
> > >
> > > But since we do not have a repository to store the config, it won't
> > > work. Sorry for the noise.
> >
> > We actually would have `~/.gitconfig` because `HOME` is set to `t/trash
> > directory.<test-name>/`.
> >
> > However, that would cause all kinds of issues when test scripts expect the
> > directory to be pristine, containing only `.git/` but not `.gitconfig`.
>
> Putting:
>
> GIT_CONFIG_PARAMETERS="'core.defaultBranchName=...'"
>
> into the environment would work (and yes, you need the single quotes
> embedded in the variable), and solves all of the complaints above.
> Further "git -c" invocations properly append to it. But:
>
> - there are a few tests which explicitly tweak that variable
>
> - it technically changes any tests of "-c" because now we'd never
> cover the case where we start without the variable defined
Indeed.
> I think baking in a special environment variable like you have is not so
> bad. If this did become too common a pattern, though (special test-only
> environment variables that do have a separate config option), I wouldn't
> be opposed to a GIT_TEST_CONFIG_PARAMETERS which takes precedence over
> other config, and comes with a big warning label that it shouldn't be
> relied upon outside the test suite. That's equally ugly to
> GIT_TEST_DEFAULT_BRANCH_NAME, but at least solves the problem once for
> all of them. I'm just not sure we have enough "all of them" to make it
> worth doing.
FWIW I do not plan on using that variable for a long time. And it is not
in this here patch series any longer, so let's discuss it in the future
patch contribution of mine.
Thanks,
Dscho
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
dscho
force-pushed
the
default-branch-name-option
branch
from
f680e66 to
b0ae5b1
Compare
|
This branch is now known as |
|
This patch series was integrated into pu via git@5c7a45a. |
|
This patch series was integrated into pu via git@3c647f7. |
dscho
force-pushed
the
default-branch-name-option
branch
from
b0ae5b1 to
456ba57
Compare
|
There is an issue in commit 70fde53830486d6f06f92c8537655353c7db0ca4: |
dscho
force-pushed
the
default-branch-name-option
branch
2 times, most recently
from
a6d326c to
a92b103
Compare
dscho
force-pushed
the
default-branch-name-option
branch
from
a92b103 to
b2192a8
Compare
…onfig We just introduced the command-line option `--initial-branch=<branch-name>` to allow initializing a new repository with a different initial branch than the hard-coded one. To allow users to override the initial branch name more permanently (i.e. without having to specify the name manually for each and every `git init` invocation), let's introduce the `init.defaultBranch` config setting. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Don Goodman-Wilson <don@goodman-wilson.com>
When cloning a repository without any branches, Git chooses a default branch name for the as-yet unborn branch. As part of the implicit initialization of the local repository, Git just learned to respect `init.defaultBranch` to choose a different initial branch name. We now really want that branch name to be used as a fall-back. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When guessing the default branch name of a remote, and there are no refs to guess from, we want to go with the preference specified by the user for the fall-back, i.e. the default name to be used for the initial branch of new repositories (because as far as the user is concerned, a remote that has no branches yet is a new repository). At the same time, when talking to an older Git server that does not report a symref for `HEAD` (but instead reports a commit hash), let's try to guess the configured default branch name first. If it does not match the reported commit hash, let's fall back to `master` as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The default name of the initial branch in new repositories can now be configured. The `testsvn` remote helper translates the remote Subversion repository's branch name `trunk` to the hard-coded name `master`. Clearly, the intention was to make the name align with Git's defaults. So while we are not talking about a newly-created repository in the `testsvn` context, it is a newly-created _Git_ repository, si it _still_ makes sense to use the overridden default name for the initial branch whenever users configured it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
default-branch-name-option
branch
from
1aa0e63 to
6c72abf
Compare
|
/submit |
|
Submitted as pull.656.v4.git.1593009996.gitgitgadget@gmail.com |
| @@ -451,10 +451,7 @@ static void fmt_merge_msg_title(struct strbuf *out, | |||
| strbuf_addf(out, " of %s", srcs.items[i].string); | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In the context of many projects renaming their primary branch names away
> from `master`, Git wants to stop treating the `master` branch specially.
>
> Let's start with `git fmt-merge-msg`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> fmt-merge-msg.c | 5 +-
> t/t1507-rev-parse-upstream.sh | 2 +-
> t/t4013-diff-various.sh | 4 +-
> t/t4013/diff.log_--decorate=full_--all | 2 +-
> t/t4013/diff.log_--decorate_--all | 2 +-
> ...--patch-with-stat_--summary_master_--_dir_ | 2 +-
> t/t4013/diff.log_--patch-with-stat_master | 2 +-
> .../diff.log_--patch-with-stat_master_--_dir_ | 2 +-
> ...ot_--cc_--patch-with-stat_--summary_master | 2 +-
> ..._--root_--patch-with-stat_--summary_master | 2 +-
> .../diff.log_--root_--patch-with-stat_master | 2 +-
> ...root_-c_--patch-with-stat_--summary_master | 2 +-
> t/t4013/diff.log_--root_-p_master | 2 +-
> t/t4013/diff.log_--root_master | 2 +-
> t/t4013/diff.log_-m_-p_--first-parent_master | 2 +-
> t/t4013/diff.log_-m_-p_master | 4 +-
> t/t4013/diff.log_-p_--first-parent_master | 2 +-
> t/t4013/diff.log_-p_master | 2 +-
> t/t4013/diff.log_master | 2 +-
> t/t4013/diff.show_--first-parent_master | 2 +-
> t/t4013/diff.show_-c_master | 2 +-
> t/t4013/diff.show_-m_master | 4 +-
> t/t4013/diff.show_master | 2 +-
> ...ot_--cc_--patch-with-stat_--summary_master | 2 +-
> ...root_-c_--patch-with-stat_--summary_master | 2 +-
> t/t4202-log.sh | 72 +++++++++----------
> t/t6200-fmt-merge-msg.sh | 36 +++++-----
> t/t7600-merge.sh | 14 ++--
> t/t7608-merge-messages.sh | 10 +--
> 29 files changed, 94 insertions(+), 97 deletions(-)
This must have been tedious as the tests with merge commits are all
over the place (I know updating t4013 would not have been too much
of the work as it has its own self-update knob, but it still is a
lot of work to verify that the changes make sense).
Thanks.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Wed, 24 Jun 2020, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In the context of many projects renaming their primary branch names away
> > from `master`, Git wants to stop treating the `master` branch specially.
> >
> > Let's start with `git fmt-merge-msg`.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > fmt-merge-msg.c | 5 +-
> > t/t1507-rev-parse-upstream.sh | 2 +-
> > t/t4013-diff-various.sh | 4 +-
> > t/t4013/diff.log_--decorate=full_--all | 2 +-
> > t/t4013/diff.log_--decorate_--all | 2 +-
> > ...--patch-with-stat_--summary_master_--_dir_ | 2 +-
> > t/t4013/diff.log_--patch-with-stat_master | 2 +-
> > .../diff.log_--patch-with-stat_master_--_dir_ | 2 +-
> > ...ot_--cc_--patch-with-stat_--summary_master | 2 +-
> > ..._--root_--patch-with-stat_--summary_master | 2 +-
> > .../diff.log_--root_--patch-with-stat_master | 2 +-
> > ...root_-c_--patch-with-stat_--summary_master | 2 +-
> > t/t4013/diff.log_--root_-p_master | 2 +-
> > t/t4013/diff.log_--root_master | 2 +-
> > t/t4013/diff.log_-m_-p_--first-parent_master | 2 +-
> > t/t4013/diff.log_-m_-p_master | 4 +-
> > t/t4013/diff.log_-p_--first-parent_master | 2 +-
> > t/t4013/diff.log_-p_master | 2 +-
> > t/t4013/diff.log_master | 2 +-
> > t/t4013/diff.show_--first-parent_master | 2 +-
> > t/t4013/diff.show_-c_master | 2 +-
> > t/t4013/diff.show_-m_master | 4 +-
> > t/t4013/diff.show_master | 2 +-
> > ...ot_--cc_--patch-with-stat_--summary_master | 2 +-
> > ...root_-c_--patch-with-stat_--summary_master | 2 +-
> > t/t4202-log.sh | 72 +++++++++----------
> > t/t6200-fmt-merge-msg.sh | 36 +++++-----
> > t/t7600-merge.sh | 14 ++--
> > t/t7608-merge-messages.sh | 10 +--
> > 29 files changed, 94 insertions(+), 97 deletions(-)
>
> This must have been tedious as the tests with merge commits are all
> over the place (I know updating t4013 would not have been too much
> of the work as it has its own self-update knob, but it still is a
> lot of work to verify that the changes make sense).
I missed the self-update knob, but in any case, I would not have used it,
anyway, to make sure that I do look closely at all the sites.
Ciao,
Dscho
| @@ -183,7 +183,7 @@ set-branch (-d|--default) [--] <path>:: | |||
| Sets the default remote tracking branch for the submodule. The | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> Even so, it should be okay to fix this behavior without anything like a
> longer transition period:
>
> - The `git submodule update --remote` command is not really common.
>
> - Current Git's behavior when running this command is outright
> confusing, unless the remote repository's current branch _is_ `master`
> (in which case the proposed behavior matches the old behavior).
>
> - If a user encounters a regression due to the changed behavior, the fix
> is actually trivial: setting `submodule.<name>.branch` to `master`
> will reinstate the old behavior.
>
> Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Well explained. Thanks.
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
This patch series was integrated into seen via git@91276ad. |
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
|
This patch series was integrated into seen via git@abc27b3. |
|
This patch series was integrated into next via git@8f962f9. |
| @@ -451,10 +451,7 @@ static void fmt_merge_msg_title(struct strbuf *out, | |||
| strbuf_addf(out, " of %s", srcs.items[i].string); | |||
There was a problem hiding this comment.
On the Git mailing list, Đoàn Trần Công Danh wrote (reply to this):
On 2020-06-24 14:46:28+0000, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In the context of many projects renaming their primary branch names away
> from `master`, Git wants to stop treating the `master` branch specially.
>
> Let's start with `git fmt-merge-msg`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Hi Dscho,
This change will also affect git-subtree test.
We'll need this patch for subtree:
----------------8<-------------------
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
<congdanhqx@gmail.com>
Date: Mon, 29 Jun 2020 22:56:37 +0700
Subject: [PATCH] contrib: subtree: adjust test to change in fmt-merge-msg
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We're starting to stop treating `master' specially in fmt-merge-msg.
Adjust the test to reflect that change.
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
contrib/subtree/t/t7900-subtree.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 57ff4b25c1..53d7accf94 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -196,7 +196,8 @@ test_expect_success 'merge new subproj history into sub dir/ with --prefix' '
cd "$subtree_test_count" &&
git fetch ./"sub proj" master &&
git subtree merge --prefix="sub dir" FETCH_HEAD &&
- check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse FETCH_HEAD)'\''"
+ check_equal "$(last_commit_message)" \
+ "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
)
'
@@ -273,7 +274,8 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende
cd "$test_count" &&
git fetch ./subproj master &&
git subtree merge --prefix=subdir/ FETCH_HEAD &&
- check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse FETCH_HEAD)'\''"
+ check_equal "$(last_commit_message)" \
+ "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
)
'
--
2.27.0.111.gc72c7da667
Danh
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323328-1652750913-1593437267=:56
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hi Danh,
On Mon, 29 Jun 2020, =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh wrote:
> On 2020-06-24 14:46:28+0000, Johannes Schindelin via GitGitGadget <gitgi=
tgadget@gmail.com> wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > In the context of many projects renaming their primary branch names aw=
ay
> > from `master`, Git wants to stop treating the `master` branch speciall=
y.
> >
> > Let's start with `git fmt-merge-msg`.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
>
> Hi Dscho,
>
> This change will also affect git-subtree test.
Good point. The patch looks good. I wonder whether we should also address
these:
Documentation/git-rebase.txt:* Merge branch 'report-a-bug'
Documentation/git-rebase.txt:* | Merge branch 'refactor-button'
Documentation/git-switch.txt:HEAD is now at 9fc9555312 Merge branch 'cc/sh=
ared-index-permbits'
Documentation/howto/using-signed-tag-in-pull-request.txt: Merge tag 'frotz=
-for-xyzzy' of example.com:/git/froboz.git/
Documentation/howto/using-signed-tag-in-pull-request.txt: Merge tag 'f=
rotz-for-xyzzy' of example.com:/git/froboz.git/
t/t7606-merge-custom.sh:* (HEAD, master) Merge commit 'c3'
The first three matches are in manual pages, the next two in technical
documentation, and the last one in a comment in one of the test scripts.
So none of them are super critical, but maybe there are different
opinions?
Ciao,
Dscho
> We'll need this patch for subtree:
> ----------------8<-------------------
> From: =3D?UTF-8?q?=3DC4=3D90o=3DC3=3DA0n=3D20Tr=3DE1=3DBA=3DA7n=3D20C=3D=
C3=3DB4ng=3D20Danh?=3D
> <congdanhqx@gmail.com>
> Date: Mon, 29 Jun 2020 22:56:37 +0700
> Subject: [PATCH] contrib: subtree: adjust test to change in fmt-merge-ms=
g
> MIME-Version: 1.0
> Content-Type: text/plain; charset=3DUTF-8
> Content-Transfer-Encoding: 8bit
>
> We're starting to stop treating `master' specially in fmt-merge-msg.
> Adjust the test to reflect that change.
>
> Signed-off-by: =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh <congdanhqx@gm=
ail.com>
> ---
> contrib/subtree/t/t7900-subtree.sh | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t790=
0-subtree.sh
> index 57ff4b25c1..53d7accf94 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -196,7 +196,8 @@ test_expect_success 'merge new subproj history into =
sub dir/ with --prefix' '
> cd "$subtree_test_count" &&
> git fetch ./"sub proj" master &&
> git subtree merge --prefix=3D"sub dir" FETCH_HEAD &&
> - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-pars=
e FETCH_HEAD)'\''"
> + check_equal "$(last_commit_message)" \
> + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> )
> '
>
> @@ -273,7 +274,8 @@ test_expect_success 'merge new subproj history into =
subdir/ with a slash appende
> cd "$test_count" &&
> git fetch ./subproj master &&
> git subtree merge --prefix=3Dsubdir/ FETCH_HEAD &&
> - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-pars=
e FETCH_HEAD)'\''"
> + check_equal "$(last_commit_message)" \
> + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> )
> '
>
> --
> 2.27.0.111.gc72c7da667
> Danh
>
--8323328-1652750913-1593437267=:56--
There was a problem hiding this comment.
On the Git mailing list, Đoàn Trần Công Danh wrote (reply to this):
Hi Dscho,
On 2020-06-29 15:27:44+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > This change will also affect git-subtree test.
>
> Good point. The patch looks good. I wonder whether we should also address
> these:
>
> Documentation/git-rebase.txt:* Merge branch 'report-a-bug'
> Documentation/git-rebase.txt:* | Merge branch 'refactor-button'
> Documentation/git-switch.txt:HEAD is now at 9fc9555312 Merge branch 'cc/shared-index-permbits'
> Documentation/howto/using-signed-tag-in-pull-request.txt: Merge tag 'frotz-for-xyzzy' of example.com:/git/froboz.git/
> Documentation/howto/using-signed-tag-in-pull-request.txt: Merge tag 'frotz-for-xyzzy' of example.com:/git/froboz.git/
> t/t7606-merge-custom.sh:* (HEAD, master) Merge commit 'c3'
>
> The first three matches are in manual pages, the next two in technical
> documentation, and the last one in a comment in one of the test scripts.
> So none of them are super critical, but maybe there are different
> opinions?
In _my very opinion_, I don't think it's that critical.
We allow git merge --edit and git fmt-merge-msg -m.
Someone may have configured their Git to remove branch name already.
And some others may always remove the target branch manually.
We probably don't want to introduce another master occurence.
(For sideline watcher: Please not argue on this,
I don't have any opinions about the word: master.)
If there're a consensus on changing those documentation,
I won't mind to do that manual work ;)
The test is a different story, since some (or most?) distro enable
check (or test) phase for their build infrastructure.
And, we shouldn't break their infrastructures.
>
> Ciao,
> Dscho
>
> > We'll need this patch for subtree:
> > ----------------8<-------------------
> > From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
> > <congdanhqx@gmail.com>
> > Date: Mon, 29 Jun 2020 22:56:37 +0700
> > Subject: [PATCH] contrib: subtree: adjust test to change in fmt-merge-msg
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > We're starting to stop treating `master' specially in fmt-merge-msg.
> > Adjust the test to reflect that change.
> >
> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> > ---
> > contrib/subtree/t/t7900-subtree.sh | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> > index 57ff4b25c1..53d7accf94 100755
> > --- a/contrib/subtree/t/t7900-subtree.sh
> > +++ b/contrib/subtree/t/t7900-subtree.sh
> > @@ -196,7 +196,8 @@ test_expect_success 'merge new subproj history into sub dir/ with --prefix' '
> > cd "$subtree_test_count" &&
> > git fetch ./"sub proj" master &&
> > git subtree merge --prefix="sub dir" FETCH_HEAD &&
> > - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse FETCH_HEAD)'\''"
> > + check_equal "$(last_commit_message)" \
> > + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> > )
> > '
> >
> > @@ -273,7 +274,8 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende
> > cd "$test_count" &&
> > git fetch ./subproj master &&
> > git subtree merge --prefix=subdir/ FETCH_HEAD &&
> > - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-parse FETCH_HEAD)'\''"
> > + check_equal "$(last_commit_message)" \
> > + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> > )
> > '
> >
> > --
> > 2.27.0.111.gc72c7da667
> > Danh
> >
--
Danh
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323328-439506614-1593599949=:56
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hi Danh,
On Tue, 30 Jun 2020, =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh wrote:
> On 2020-06-29 15:27:44+0200, Johannes Schindelin <Johannes.Schindelin@gm=
x.de> wrote:
> > > This change will also affect git-subtree test.
> >
> > Good point. The patch looks good. I wonder whether we should also addr=
ess
> > these:
> >
> > Documentation/git-rebase.txt:* Merge branch 'report-a-bug'
> > Documentation/git-rebase.txt:* | Merge branch 'refactor-button'
> > Documentation/git-switch.txt:HEAD is now at 9fc9555312 Merge branch 'c=
c/shared-index-permbits'
> > Documentation/howto/using-signed-tag-in-pull-request.txt: Merge tag 'f=
rotz-for-xyzzy' of example.com:/git/froboz.git/
> > Documentation/howto/using-signed-tag-in-pull-request.txt: Merge ta=
g 'frotz-for-xyzzy' of example.com:/git/froboz.git/
> > t/t7606-merge-custom.sh:* (HEAD, master) Merge commit 'c3'
> >
> > The first three matches are in manual pages, the next two in technical
> > documentation, and the last one in a comment in one of the test script=
s.
> > So none of them are super critical, but maybe there are different
> > opinions?
>
> In _my very opinion_, I don't think it's that critical.
> We allow git merge --edit and git fmt-merge-msg -m.
> Someone may have configured their Git to remove branch name already.
> And some others may always remove the target branch manually.
> We probably don't want to introduce another master occurence.
> (For sideline watcher: Please not argue on this,
> I don't have any opinions about the word: master.)
True.
> If there're a consensus on changing those documentation,
> I won't mind to do that manual work ;)
I actually agree that it is not _really_ necessary.
> The test is a different story, since some (or most?) distro enable
> check (or test) phase for their build infrastructure.
> And, we shouldn't break their infrastructures.
Actually, the hit in t7606 is in the initial _comment_. So I highly doubt
that it would break any build infrastructure to leave it alone.
Ciao,
Dscho
> >
> > Ciao,
> > Dscho
> >
> > > We'll need this patch for subtree:
> > > ----------------8<-------------------
> > > From: =3D?UTF-8?q?=3DC4=3D90o=3DC3=3DA0n=3D20Tr=3DE1=3DBA=3DA7n=3D20=
C=3DC3=3DB4ng=3D20Danh?=3D
> > > <congdanhqx@gmail.com>
> > > Date: Mon, 29 Jun 2020 22:56:37 +0700
> > > Subject: [PATCH] contrib: subtree: adjust test to change in fmt-merg=
e-msg
> > > MIME-Version: 1.0
> > > Content-Type: text/plain; charset=3DUTF-8
> > > Content-Transfer-Encoding: 8bit
> > >
> > > We're starting to stop treating `master' specially in fmt-merge-msg.
> > > Adjust the test to reflect that change.
> > >
> > > Signed-off-by: =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh <congdanhq=
x@gmail.com>
> > > ---
> > > contrib/subtree/t/t7900-subtree.sh | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/=
t7900-subtree.sh
> > > index 57ff4b25c1..53d7accf94 100755
> > > --- a/contrib/subtree/t/t7900-subtree.sh
> > > +++ b/contrib/subtree/t/t7900-subtree.sh
> > > @@ -196,7 +196,8 @@ test_expect_success 'merge new subproj history i=
nto sub dir/ with --prefix' '
> > > cd "$subtree_test_count" &&
> > > git fetch ./"sub proj" master &&
> > > git subtree merge --prefix=3D"sub dir" FETCH_HEAD &&
> > > - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-=
parse FETCH_HEAD)'\''"
> > > + check_equal "$(last_commit_message)" \
> > > + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> > > )
> > > '
> > >
> > > @@ -273,7 +274,8 @@ test_expect_success 'merge new subproj history i=
nto subdir/ with a slash appende
> > > cd "$test_count" &&
> > > git fetch ./subproj master &&
> > > git subtree merge --prefix=3Dsubdir/ FETCH_HEAD &&
> > > - check_equal "$(last_commit_message)" "Merge commit '\''$(git rev-=
parse FETCH_HEAD)'\''"
> > > + check_equal "$(last_commit_message)" \
> > > + "Merge commit '\''$(git rev-parse FETCH_HEAD)'\'' into master"
> > > )
> > > '
> > >
> > > --
> > > 2.27.0.111.gc72c7da667
> > > Danh
> > >
>
>
> --
> Danh
>
--8323328-439506614-1593599949=:56--
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> In _my very opinion_, I don't think it's that critical.
>> We allow git merge --edit and git fmt-merge-msg -m.
>> ...
>
> True.
>
>> If there're a consensus on changing those documentation,
>> I won't mind to do that manual work ;)
>
> I actually agree that it is not _really_ necessary.
>
>> The test is a different story, since some (or most?) distro enable
>> check (or test) phase for their build infrastructure.
>> And, we shouldn't break their infrastructures.
>
> Actually, the hit in t7606 is in the initial _comment_. So I highly doubt
> that it would break any build infrastructure to leave it alone.
I guess it's settled, then.
Thank you to all for being extra careful.
|
This patch series was integrated into seen via git@0cf66eb. |
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): |
|
This patch series was integrated into seen via git@3e69a6e. |
|
This patch series was integrated into seen via git@3e54a70. |
|
This patch series was integrated into seen via git@11cbda2. |
|
This patch series was integrated into master via git@11cbda2. |
|
Closed via 11cbda2. |
|
On the Git mailing list, Edward Thomson wrote (reply to this): |
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A growing number of open source projects aims to avoid the branch name
masterdue to its negative connotation. See [1] for an existing discussion on this. The links [2], [3], and [4] describe community-driven ways for users to rename their default branches or use template edits to set a new default branch name.[1] https://lore.kernel.org/git/CAOAHyQwyXC1Z3v7BZAC+Bq6JBaM7FvBenA-1fcqeDV==apdWDg@mail.gmail.com/
[2] https://twitter.com/mislav/status/1270388510684598272
[3] https://www.hanselman.com/blog/EasilyRenameYourGitDefaultBranchFromMasterToMain.aspx
[4] https://github.com/ethomson/retarget_prs
By necessity, existing repositories require a lot of manual work to move away from that branch name, but it should be much easier for new repositories.
This patch series allows overriding the branch name being used for new repositories' main branch. This can be configured via
init.defaultBranch.The initial patch was started by newcomer Don Goodman-Wilson, as well as the bigger change that morphed into #655, where we demonstrate how to change Git's hard-coded default branch name for new repositories to
mainbased on this here patch series, verifying the approach. Thanks for the contribution!This series DOES NOT change the default automatically, but only provides an opt-in mechanism for interested users. The plan for that is to convert the test scripts incrementally (by introducing
GIT_TEST_DEFAULT_MAIN_BRANCH_NAME, which overridesinit.defaultBranch, and then converting the tricky test scripts first, one by one, using that environment variable).Changes since v3:
We now avoid the phrasing "specify a specific" in the messages.
The
git submodulepatch has a better commit title now, and it also adjusts the documentation more completely now (thanks, Philippe!).A code comment in
builtin/init-db.cwas updated to no longer talk about a symlink when it comes toHEAD.Made it the responsibility of the caller to warn about
reinit && initial_branch.Changes since v2:
Dropped the
fast-exportpatches, as they have been superseded by Peff'sjk/fast-export-anonym.Adjusted the
fmt-merge-msgpatch so as to not special-casemasteranymore (instead of special-casing a specific "main" branch).Modified the
git submodulepatch so that it usesorigin/HEADinstead of trying to look for a remote branch with the name indicated byinit.defaultBranch.Changes since v1:
The modifications to respect
GIT_TEST_DEFAULT_BRANCH_NAMEhave been backed out from this patch series, as they are only relevant once we start converting the test suite to accommodate for a new default main branch name.An error message that started with an upper-case letter was downcased.
git_default_branch_name()'s logic was simplified by replacing anif ... else ...by a ternary assignment.The
git_default_branch_name()function was renamed togit_main_branch_name()and a correspondingrepo_main_branch_name()was introduced.The "init" commit message's first paragraph now elaborates a little bit on the reasoning why projects want to move away from the current default branch name.
The "init" commit was split into two.
There are now two config settings:
init.defaultBranch(defining the branch name to use when initializing new repositories) andcore.mainBranch(which is now configured byinit_db(), declaring what the name of the main branch is in this repository).The commits were re-ordered to introduce the concept of
core.mainBranchfirst because technically, this is something that we could do even without changinggit initat all.git fast-export --anonymizenow always uses the ref nameref0for the main branch, no matter whether it was overridden or whether the fall-back is in effect.The code comment regarding anonymizing the main branch to
ref0ingit fast-export --anonymizewas enhanced.A new patch was added to rename
core.mainBranchif the main branch is renamed viagit branch -m.Added a patch that introduces support for
git init --main-branch=<branch-name>.Where possible, I added tests (although I did not try to extend test coverage to all changes: the
send-pack.c/transport-helper.cpatch only adds a test for thesend-pack.cadjustment, for example).Cc: don@goodman-wilson.com, stolee@gmail.com, peff@peff.net, sandals@crustytoothpaste.net, Matt Rogers mattr94@gmail.com, Eric Sunshine sunshine@sunshineco.com, Taylor Blau me@ttaylorr.com, Phillip Wood phillip.wood123@gmail.com, Alban Gruin alban.gruin@gmail.com, Johannes Sixt j6t@kdbg.org, Denton Liu liu.denton@gmail.com, Ævar Arnfjörð Bjarmason avarab@gmail.com, Philippe Blain levraiphilippeblain@gmail.com