[v3,5/8] init: allow setting the default for the initial branch name via the config
diff mbox series

Message ID a500e0f214a0ea4bf5cf4e26f688ae68a0b84bcd.1592951611.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • Allow overriding the default name of the default branch
Related show

Commit Message

Elijah Newren via GitGitGadget June 23, 2020, 10:33 p.m. UTC
From: Don Goodman-Wilson <don@goodman-wilson.com>

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>
---
 Documentation/config/init.txt |  4 ++++
 builtin/init-db.c             |  2 +-
 refs.c                        | 30 ++++++++++++++++++++++++++++++
 refs.h                        |  9 +++++++++
 t/t0001-init.sh               | 13 +++++++++++++
 5 files changed, 57 insertions(+), 1 deletion(-)

Comments

Junio C Hamano June 24, 2020, 1:05 a.m. UTC | #1
"Don Goodman-Wilson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index a898153901..b8634b5f35 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -269,7 +269,7 @@ static int create_default_files(const char *template_path,
>  		char *ref;
>  
>  		if (!initial_branch)
> -			initial_branch = "master";
> +			initial_branch = git_default_branch_name();
>  
>  		ref = xstrfmt("refs/heads/%s", initial_branch);
>  		if (check_refname_format(ref, 0) < 0)

Continuing with the division of labor between this helper and its
caller, I had this funny dislike of falling back here and not in the
caller.  But with the same idea of using "reinit", we could get rid
of this "if the caller didn't give us initial_branch, fall back
to..." logic from the function.  The caller may do

	reinit = create_default_files(...
			initial_branch ? initial_branch : "master",
			...);
	if (reinit || initial_branch)
		warning(_(...));
	
in the previous step and then we can teach the caller to use the
configured value instead of the hardcoded "master".

That's much better ;-)

Other than that, looks good to me.

Thanks.
Johannes Schindelin June 24, 2020, 12:56 p.m. UTC | #2
Hi Junio,

On Tue, 23 Jun 2020, Junio C Hamano wrote:

> "Don Goodman-Wilson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index a898153901..b8634b5f35 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -269,7 +269,7 @@ static int create_default_files(const char *template_path,
> >  		char *ref;
> >
> >  		if (!initial_branch)
> > -			initial_branch = "master";
> > +			initial_branch = git_default_branch_name();
> >
> >  		ref = xstrfmt("refs/heads/%s", initial_branch);
> >  		if (check_refname_format(ref, 0) < 0)
>
> Continuing with the division of labor between this helper and its
> caller, I had this funny dislike of falling back here and not in the
> caller.  But with the same idea of using "reinit", we could get rid
> of this "if the caller didn't give us initial_branch, fall back
> to..." logic from the function.  The caller may do
>
> 	reinit = create_default_files(...
> 			initial_branch ? initial_branch : "master",
> 			...);
> 	if (reinit || initial_branch)
> 		warning(_(...));
>
> in the previous step and then we can teach the caller to use the
> configured value instead of the hardcoded "master".

While that is really tempting, there is another called of `init_db()`
(which calls `create_default_files()`): `builtin/clone.c`. And I do not
wish to duplicate the logic there.

So I left this as-is.

Ciao,
Dscho

>
> That's much better ;-)
>
> Other than that, looks good to me.
>
> Thanks.
>
Junio C Hamano June 24, 2020, 4:25 p.m. UTC | #3
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> 	reinit = create_default_files(...
>> 			initial_branch ? initial_branch : "master",
>> 			...);
>> 	if (reinit || initial_branch)
>> 		warning(_(...));
>>
>> in the previous step and then we can teach the caller to use the
>> configured value instead of the hardcoded "master".
>
> While that is really tempting, there is another called of `init_db()`
> (which calls `create_default_files()`): `builtin/clone.c`. And I do not
> wish to duplicate the logic there.
>
> So I left this as-is.

I am still on the fence after seeing v4, but let's leave it as is.
The reason why I wanted to leave the "default to" logic out of the
helper was to make sure it implements little or no policy, which
would leave the door open to let other callers of the helper to use
their own and different default, but we can revisit when we acquire
the third caller.  I do not see an immediate need to make the
clone's fallback default configurable separately from what init uses
for the default initial branch name, and with modern servers it is
doubtful that the fallback default by clone is ever used anyway.

Thanks.

Patch
diff mbox series

diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt
index 46fa8c6a08..6ae4a38416 100644
--- a/Documentation/config/init.txt
+++ b/Documentation/config/init.txt
@@ -1,3 +1,7 @@ 
 init.templateDir::
 	Specify the directory from which templates will be copied.
 	(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+
+init.defaultBranch::
+	Allows overriding the default branch name when initializing
+	a new repository.
diff --git a/builtin/init-db.c b/builtin/init-db.c
index a898153901..b8634b5f35 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -269,7 +269,7 @@  static int create_default_files(const char *template_path,
 		char *ref;
 
 		if (!initial_branch)
-			initial_branch = "master";
+			initial_branch = git_default_branch_name();
 
 		ref = xstrfmt("refs/heads/%s", initial_branch);
 		if (check_refname_format(ref, 0) < 0)
diff --git a/refs.c b/refs.c
index 224ff66c7b..b98dea5217 100644
--- a/refs.c
+++ b/refs.c
@@ -560,6 +560,36 @@  void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
 		argv_array_pushf(prefixes, *p, len, prefix);
 }
 
+char *repo_default_branch_name(struct repository *r)
+{
+	const char *config_key = "init.defaultbranch";
+	const char *config_display_key = "init.defaultBranch";
+	char *ret = NULL, *full_ref;
+
+	if (repo_config_get_string(r, config_key, &ret) < 0)
+		die(_("could not retrieve `%s`"), config_display_key);
+
+	if (!ret)
+		ret = xstrdup("master");
+
+	full_ref = xstrfmt("refs/heads/%s", ret);
+	if (check_refname_format(full_ref, 0))
+		die(_("invalid branch name: %s = %s"), config_display_key, ret);
+	free(full_ref);
+
+	return ret;
+}
+
+const char *git_default_branch_name(void)
+{
+	static char *ret;
+
+	if (!ret)
+		ret = repo_default_branch_name(the_repository);
+
+	return ret;
+}
+
 /*
  * *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 e010f8aec2..f212f8945e 100644
--- a/refs.h
+++ b/refs.h
@@ -154,6 +154,15 @@  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 default branch name for newly-initialized repositories.
+ *
+ * The return value of `repo_default_branch_name()` is an allocated string. The
+ * return value of `git_default_branch_name()` is a singleton.
+ */
+const char *git_default_branch_name(void);
+char *repo_default_branch_name(struct repository *r);
+
 /*
  * 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 61837ca25f..047197d08f 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -477,4 +477,17 @@  test_expect_success '--initial-branch' '
 	grep hello actual
 '
 
+test_expect_success 'overridden default initial branch name (config)' '
+	test_config_global init.defaultBranch nmb &&
+	git init initial-branch-config &&
+	git -C initial-branch-config symbolic-ref HEAD >actual &&
+	grep nmb actual
+'
+
+test_expect_success 'invalid default branch name' '
+	test_config_global init.defaultBranch "with space" &&
+	test_must_fail git init initial-branch-invalid 2>err &&
+	test_i18ngrep "invalid branch name" err
+'
+
 test_done