diff mbox series

[v3,4/8] init: allow specifying the initial branch name for the new repository

Message ID e8a6587c1a783476413277296c6ce3db74e217e6.1592951611.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Allow overriding the default name of the default branch | expand

Commit Message

Linus Arver via GitGitGadget June 23, 2020, 10:33 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

There is a growing number of projects and companies desiring to change
the main branch name of their repositories (see e.g.
https://twitter.com/mislav/status/1270388510684598272 for background on
this).

To change that branch name for new repositories, currently 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 a new option:
`--initial-branch=<branch-name>`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-init.txt |  7 +++++++
 builtin/clone.c            |  3 ++-
 builtin/init-db.c          | 34 +++++++++++++++++++++++++++-------
 cache.h                    |  2 +-
 t/t0001-init.sh            | 13 +++++++++++++
 5 files changed, 50 insertions(+), 9 deletions(-)

Comments

Junio C Hamano June 24, 2020, 12:58 a.m. UTC | #1 
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  [verse]
>  'git init' [-q | --quiet] [--bare] [--template=<template_directory>]
>  	  [--separate-git-dir <git dir>] [--object-format=<format]

Completely offtopic.  We lack the closing ket> here.

> +	  [-b <branch-name> | --initial-branch=<branch-name>]
>  	  [--shared[=<permissions>]] [directory]
>  
>  
> @@ -67,6 +68,12 @@ repository.
>  +
>  If this is reinitialization, the repository will be moved to the specified path.
>  
> +-b <branch-name::
> +--initial-branch=<branch-name>::
> +
> +Use the specified name for the initial branch in the newly created repository.
> +If not specified, fall back to the default name: `master`.

OK.

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 2a8e3aaaed..b751bdf13e 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1111,7 +1111,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  		}
>  	}
>  
> -	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, INIT_DB_QUIET);
> +	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
> +		INIT_DB_QUIET);
>  
>  	if (real_git_dir)
>  		git_dir = real_git_dir;
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 0b7222e718..a898153901 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -203,6 +203,7 @@ void initialize_repository_version(int hash_algo)
>  
>  static int create_default_files(const char *template_path,
>  				const char *original_git_dir,
> +				const char *initial_branch,
>  				const struct repository_format *fmt)
>  {
>  	struct stat st1;
> @@ -258,16 +259,29 @@ 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.
>  	 */

To the caller of this helper, it may be "the default", but as far as
this helper is concerned, it is not "default" but the initial branch
that was given by the caller.  How about...

	/*
	 * Point the initial branch with HEAD symref, if HEAD does
	 * not exist yet.
	 */

... to modernize the reference to symlink and replace it with
symref?

> +		if (!initial_branch)
> +			initial_branch = "master";
> +
> +		ref = xstrfmt("refs/heads/%s", initial_branch);
> +		if (check_refname_format(ref, 0) < 0)
> +			die(_("invalid initial branch name: '%s'"),
> +			    initial_branch);

Good.  We make sure to prefix with "refs/heads/" so the callers
cannot abuse us to point HEAD outside the local branches.

> +		if (create_symref("HEAD", ref, NULL) < 0)
>  			exit(1);
> -	}
> +		free(ref);
> +	} else if (initial_branch)
> +		warning(_("re-init: ignoring --initial-branch=%s"),
> +			initial_branch);

Somehow the error checking convention feels uneven in this API.  It
is a warning-worthy offense for the caller to give initial_branch
when we are re-initializing, but it is not an error for the caller
not to supply the initial branch name on the other side of if/else.
Worse yet, this helper function even knows the command line option
name that resulted in the parameter coming to it.

That unevenness ultimately comes from the fact that the caller does
not know if we are dealing with a repository that already has HEAD
before calling, but at least we should be able to tell the caller
if we initialized or not with our return value and allow the caller
to issue this warning---that way we can lose the warning from here
and get rid of the uneven feeling.  Oh, and ...

> @@ -383,7 +397,8 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
>  }
>  
>  int init_db(const char *git_dir, const char *real_git_dir,
> -	    const char *template_dir, int hash, unsigned int flags)
> +	    const char *template_dir, int hash, const char *initial_branch,
> +	    unsigned int flags)
>  {
>  	int reinit;
>  	int exist_ok = flags & INIT_DB_EXIST_OK;
> @@ -425,7 +440,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
>  
>  	validate_hash_algorithm(&repo_fmt, hash);
>  
> -	reinit = create_default_files(template_dir, original_git_dir, &repo_fmt);
> +	reinit = create_default_files(template_dir, original_git_dir,
> +				      initial_branch, &repo_fmt);

... we are telling the caller if we are in reinit situation, so we
can afford to do exactly that.  

	reinit = create_default_files...
 	if (reinit && initial_branch)
		warning(_("re-init: ignored --initial-branch"));

That's much better ;-)

Other than that, looks good to me.

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

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

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> >  [verse]
> >  'git init' [-q | --quiet] [--bare] [--template=<template_directory>]
> >  	  [--separate-git-dir <git dir>] [--object-format=<format]
>
> Completely offtopic.  We lack the closing ket> here.

Not so off-topic: the fix for this would cause conflicts. In the interest
of avoiding merge conflicts, I incorporated a patch to fix that (`git
grep` found two more instances, which I fixed "while at it").

> > +	  [-b <branch-name> | --initial-branch=<branch-name>]
> >  	  [--shared[=<permissions>]] [directory]
> >
> >
> > @@ -67,6 +68,12 @@ repository.
> >  +
> >  If this is reinitialization, the repository will be moved to the specified path.
> >
> > +-b <branch-name::
> > +--initial-branch=<branch-name>::
> > +
> > +Use the specified name for the initial branch in the newly created repository.
> > +If not specified, fall back to the default name: `master`.
>
> OK.
>
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 2a8e3aaaed..b751bdf13e 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -1111,7 +1111,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >  		}
> >  	}
> >
> > -	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, INIT_DB_QUIET);
> > +	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
> > +		INIT_DB_QUIET);
> >
> >  	if (real_git_dir)
> >  		git_dir = real_git_dir;
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index 0b7222e718..a898153901 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -203,6 +203,7 @@ void initialize_repository_version(int hash_algo)
> >
> >  static int create_default_files(const char *template_path,
> >  				const char *original_git_dir,
> > +				const char *initial_branch,
> >  				const struct repository_format *fmt)
> >  {
> >  	struct stat st1;
> > @@ -258,16 +259,29 @@ 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.
> >  	 */
>
> To the caller of this helper, it may be "the default", but as far as
> this helper is concerned, it is not "default" but the initial branch
> that was given by the caller.  How about...
>
> 	/*
> 	 * Point the initial branch with HEAD symref, if HEAD does
> 	 * not exist yet.
> 	 */
>
> ... to modernize the reference to symlink and replace it with
> symref?

Good point. I massaged your proposed comment and replaced the old one with
it.

> > +		if (!initial_branch)
> > +			initial_branch = "master";
> > +
> > +		ref = xstrfmt("refs/heads/%s", initial_branch);
> > +		if (check_refname_format(ref, 0) < 0)
> > +			die(_("invalid initial branch name: '%s'"),
> > +			    initial_branch);
>
> Good.  We make sure to prefix with "refs/heads/" so the callers
> cannot abuse us to point HEAD outside the local branches.

Yes, Peff offered that concern, and I agree.

> > +		if (create_symref("HEAD", ref, NULL) < 0)
> >  			exit(1);
> > -	}
> > +		free(ref);
> > +	} else if (initial_branch)
> > +		warning(_("re-init: ignoring --initial-branch=%s"),
> > +			initial_branch);
>
> Somehow the error checking convention feels uneven in this API.  It
> is a warning-worthy offense for the caller to give initial_branch
> when we are re-initializing, but it is not an error for the caller
> not to supply the initial branch name on the other side of if/else.
> Worse yet, this helper function even knows the command line option
> name that resulted in the parameter coming to it.
>
> That unevenness ultimately comes from the fact that the caller does
> not know if we are dealing with a repository that already has HEAD
> before calling, but at least we should be able to tell the caller
> if we initialized or not with our return value and allow the caller
> to issue this warning---that way we can lose the warning from here
> and get rid of the uneven feeling.  Oh, and ...
>
> > @@ -383,7 +397,8 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
> >  }
> >
> >  int init_db(const char *git_dir, const char *real_git_dir,
> > -	    const char *template_dir, int hash, unsigned int flags)
> > +	    const char *template_dir, int hash, const char *initial_branch,
> > +	    unsigned int flags)
> >  {
> >  	int reinit;
> >  	int exist_ok = flags & INIT_DB_EXIST_OK;
> > @@ -425,7 +440,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
> >
> >  	validate_hash_algorithm(&repo_fmt, hash);
> >
> > -	reinit = create_default_files(template_dir, original_git_dir, &repo_fmt);
> > +	reinit = create_default_files(template_dir, original_git_dir,
> > +				      initial_branch, &repo_fmt);
>
> ... we are telling the caller if we are in reinit situation, so we
> can afford to do exactly that.
>
> 	reinit = create_default_files...
>  	if (reinit && initial_branch)
> 		warning(_("re-init: ignored --initial-branch"));

I changed it accordingly.

Thanks,
Dscho

>
> That's much better ;-)
>
> Other than that, looks good to me.
>
> Thanks.
>
>
diff mbox series

Patch

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index adc6adfd38..36dccc6e52 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -11,6 +11,7 @@  SYNOPSIS
 [verse]
 'git init' [-q | --quiet] [--bare] [--template=<template_directory>]
 	  [--separate-git-dir <git dir>] [--object-format=<format]
+	  [-b <branch-name> | --initial-branch=<branch-name>]
 	  [--shared[=<permissions>]] [directory]
 
 
@@ -67,6 +68,12 @@  repository.
 +
 If this is reinitialization, the repository will be moved to the specified path.
 
+-b <branch-name::
+--initial-branch=<branch-name>::
+
+Use the specified name for the initial branch in the newly created repository.
+If not specified, fall back to the default name: `master`.
+
 --shared[=(false|true|umask|group|all|world|everybody|0xxx)]::
 
 Specify that the Git repository is to be shared amongst several users.  This
diff --git a/builtin/clone.c b/builtin/clone.c
index 2a8e3aaaed..b751bdf13e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1111,7 +1111,8 @@  int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, INIT_DB_QUIET);
+	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
+		INIT_DB_QUIET);
 
 	if (real_git_dir)
 		git_dir = real_git_dir;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 0b7222e718..a898153901 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -203,6 +203,7 @@  void initialize_repository_version(int hash_algo)
 
 static int create_default_files(const char *template_path,
 				const char *original_git_dir,
+				const char *initial_branch,
 				const struct repository_format *fmt)
 {
 	struct stat st1;
@@ -258,16 +259,29 @@  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 *ref;
+
+		if (!initial_branch)
+			initial_branch = "master";
+
+		ref = xstrfmt("refs/heads/%s", initial_branch);
+		if (check_refname_format(ref, 0) < 0)
+			die(_("invalid initial branch name: '%s'"),
+			    initial_branch);
+
+		if (create_symref("HEAD", ref, NULL) < 0)
 			exit(1);
-	}
+		free(ref);
+	} else if (initial_branch)
+		warning(_("re-init: ignoring --initial-branch=%s"),
+			initial_branch);
 
 	initialize_repository_version(fmt->hash_algo);
 
@@ -383,7 +397,8 @@  static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
 }
 
 int init_db(const char *git_dir, const char *real_git_dir,
-	    const char *template_dir, int hash, unsigned int flags)
+	    const char *template_dir, int hash, const char *initial_branch,
+	    unsigned int flags)
 {
 	int reinit;
 	int exist_ok = flags & INIT_DB_EXIST_OK;
@@ -425,7 +440,8 @@  int init_db(const char *git_dir, const char *real_git_dir,
 
 	validate_hash_algorithm(&repo_fmt, hash);
 
-	reinit = create_default_files(template_dir, original_git_dir, &repo_fmt);
+	reinit = create_default_files(template_dir, original_git_dir,
+				      initial_branch, &repo_fmt);
 
 	create_object_directory();
 
@@ -528,6 +544,7 @@  int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
 	const char *object_format = NULL;
+	const char *initial_branch = NULL;
 	int hash_algo = GIT_HASH_UNKNOWN;
 	const struct option init_db_options[] = {
 		OPT_STRING(0, "template", &template_dir, N_("template-directory"),
@@ -541,6 +558,8 @@  int cmd_init_db(int argc, const char **argv, const char *prefix)
 		OPT_BIT('q', "quiet", &flags, N_("be quiet"), INIT_DB_QUIET),
 		OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
 			   N_("separate git dir from working tree")),
+		OPT_STRING('b', "initial-branch", &initial_branch, N_("name"),
+			   N_("override the name of the initial branch")),
 		OPT_STRING(0, "object-format", &object_format, N_("hash"),
 			   N_("specify the hash algorithm to use")),
 		OPT_END()
@@ -652,5 +671,6 @@  int cmd_init_db(int argc, const char **argv, const char *prefix)
 	UNLEAK(work_tree);
 
 	flags |= INIT_DB_EXIST_OK;
-	return init_db(git_dir, real_git_dir, template_dir, hash_algo, flags);
+	return init_db(git_dir, real_git_dir, template_dir, hash_algo,
+		       initial_branch, flags);
 }
diff --git a/cache.h b/cache.h
index 0f0485ecfe..654426460c 100644
--- a/cache.h
+++ b/cache.h
@@ -628,7 +628,7 @@  int path_inside_repo(const char *prefix, const char *path);
 
 int init_db(const char *git_dir, const char *real_git_dir,
 	    const char *template_dir, int hash_algo,
-	    unsigned int flags);
+	    const char *initial_branch, unsigned int flags);
 void initialize_repository_version(int hash_algo);
 
 void sanitize_stdfds(void);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 1edd5aeb8f..61837ca25f 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -464,4 +464,17 @@  test_expect_success MINGW 'redirect std handles' '
 	grep "Needed a single revision" output.txt
 '
 
+test_expect_success '--initial-branch' '
+	git init --initial-branch=hello initial-branch-option &&
+	git -C initial-branch-option symbolic-ref HEAD >actual &&
+	echo refs/heads/hello >expect &&
+	test_cmp expect actual &&
+
+	: re-initializing should not change the branch name &&
+	git init --initial-branch=ignore initial-branch-option 2>err &&
+	test_i18ngrep "ignoring --initial-branch" err &&
+	git -C initial-branch-option symbolic-ref HEAD >actual &&
+	grep hello actual
+'
+
 test_done