| Message ID | a500e0f214a0ea4bf5cf4e26f688ae68a0b84bcd.1592951611.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Allow overriding the default name of the default branch | expand |
"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.
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. >
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.
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