Re: [PATCH v4 15/21] init: allow alternate ref strorage to be set for new repos

[David Turner <dturner@twopensource.com>]

On Fri, 2016-02-12 at 16:26 +0100, Michael Haggerty wrote:
> On 02/05/2016 08:44 PM, David Turner wrote:
> > git init learns a new argument --ref-storage.  Presently, only
> > "files" is supported, but later we will add other storage backends.
> > 
> > When this argument is used, the repository's extensions.refStorage
> > configuration value is set (as well as
> > core.repositoryformatversion),
> > and the ref storage backend's initdb function is used to set up the
> > ref database.
> > 
> > Signed-off-by: David Turner <dturner@twopensource.com>
> > Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> > ---
> >  Documentation/git-init-db.txt          |  2 +-
> >  Documentation/git-init.txt             |  7 +++++-
> >  builtin/init-db.c                      | 44
> > +++++++++++++++++++++++++++-------
> >  cache.h                                |  2 ++
> >  contrib/completion/git-completion.bash |  3 ++-
> >  path.c                                 | 29 ++++++++++++++++++++--
> >  refs.c                                 |  8 +++++++
> >  refs.h                                 |  8 +++++++
> >  setup.c                                | 12 ++++++++++
> >  t/t0001-init.sh                        | 25 +++++++++++++++++++
> >  10 files changed, 127 insertions(+), 13 deletions(-)
> > 
> > [...]
> > diff --git a/Documentation/git-init.txt b/Documentation/git
> > -init.txt
> > index 8174d27..d2b150f 100644
> > --- a/Documentation/git-init.txt
> > +++ b/Documentation/git-init.txt
> > @@ -12,7 +12,7 @@ SYNOPSIS
> >  'git init' [-q | --quiet] [--bare] [-
> > -template=<template_directory>]
> >  	  [--separate-git-dir <git dir>]
> >  	  [--shared[=<permissions>]] [directory]
> > -
> > +	  [--ref-storage=<name>]
> >  
> >  DESCRIPTION
> >  -----------
> > @@ -113,6 +113,11 @@ does not exist, it will be created.
> >  
> >  --
> >  
> > +--ref-storage=<name>::
> > +Type of refs storage backend. Default is to use the original
> > "files"
> > +storage, which stores ref data in files in .git/refs and
> > +.git/packed-refs.
> > +
> 
> Technically, that should be $GIT_DIR/refs and $GIT_DIR/packed-refs.
> But
> it might be that we are not so picky about the distinction in the
> user docs.

We're not (according to grep), but we probably should be.

> >  TEMPLATE DIRECTORY
> >  ------------------
> >  
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index 801e977..d331ce8 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -24,6 +24,7 @@ static int init_is_bare_repository = 0;
> >  static int init_shared_repository = -1;
> >  static const char *init_db_template_dir;
> >  static const char *git_link;
> > +static char *requested_ref_storage_backend;
> >  
> >  static void copy_templates_1(struct strbuf *path, struct strbuf
> > *template,
> >  			     DIR *dir)
> > @@ -179,6 +180,7 @@ static int create_default_files(const char
> > *template_path)
> >  	int reinit;
> >  	int filemode;
> >  	struct strbuf err = STRBUF_INIT;
> > +	int repo_version = 0;
> >  
> >  	/* Just look for `init.templatedir` */
> >  	git_config(git_init_db_config, NULL);
> > @@ -205,6 +207,32 @@ static int create_default_files(const char
> > *template_path)
> >  	}
> >  
> >  	/*
> > +	 * Create the default symlink from ".git/HEAD" to the
> > "master"
> 
> I know that you are just moving this comment, but s/symlink/symref/
> would make it more up-to-date.

OK.

> > +	 * branch, if it does not exist yet.
> > +	 */
> > [...]
> > diff --git a/setup.c b/setup.c
> > index 0deb022..1a62277 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -1,5 +1,6 @@
> >  #include "cache.h"
> >  #include "dir.h"
> > +#include "refs.h"
> >  #include "string-list.h"
> >  
> >  static int inside_git_dir = -1;
> > @@ -263,6 +264,15 @@ int get_common_dir_noenv(struct strbuf *sb,
> > const char *gitdir)
> >  	return ret;
> >  }
> >  
> > +int ref_storage_backend_config(const char *var, const char *value,
> > void *ptr)
> > +{
> > +	char **cdata = ptr;
> > +
> > +	if (!strcmp(var, "extensions.refstorage"))
> > +		*cdata = xstrdup(value);
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Test if it looks like we're at a git directory.
> >   * We want to see:
> > @@ -390,6 +400,8 @@ static int check_repo_format(const char *var,
> > const char *value, void *cb)
> >  			;
> >  		else if (!strcmp(ext, "preciousobjects"))
> >  			repository_format_precious_objects =
> > git_config_bool(var, value);
> > +		else if (!strcmp(ext, "refstorage"))
> > +			ref_storage_backend = xstrdup(value);
> >  		else
> >  			string_list_append(&unknown_extensions,
> > ext);
> >  	}
> > diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> > index 295aa59..31c8279 100755
> > --- a/t/t0001-init.sh
> > +++ b/t/t0001-init.sh
> > @@ -174,6 +174,31 @@ test_expect_success 'reinit' '
> >  	test_i18ncmp again/empty again/err2
> >  '
> >  
> > +test_expect_success 'init with bogus storage backend fails' '
> > +
> > +	(
> > +		mkdir again2 &&
> > +		cd again2 &&
> > +		test_must_fail git init --ref-storage=test >out2
> > 2>err2 &&
> > +		test_i18ngrep "Unknown ref storage backend test"
> > err2
> > +	)
> > +'
> > +
> > +test_expect_failure 'reinit with changed storage backend fails' '
> > +
> > +	(
> > +		mkdir again3 &&
> > +		cd again3 &&
> > +		git init >out1 2>err1 &&
> > +		git init --ref-storage=test >out2 2>err2
> > +	) &&
> > +	test_i18ngrep "Initialized empty" again3/out1 &&
> > +	test_i18ngrep "Reinitialized existing" again3/out2 &&
> > +	>again3/empty &&
> > +	test_i18ncmp again3/empty again3/err1 &&
> > +	test_i18ncmp again3/empty again3/err2
> > +'
> > +
> 
> Is it worth testing re-initializing with the same --ref-storage?
> Perhaps
> not.

Would be nice, but since we cannot guarantee that any alternate backend
exists, we have no way to test this.  We'd have to add an entire "test"
ref backend just for tests, which seems a bit overboard.

> >  test_expect_success 'init with --template' '
> >  	mkdir template-source &&
> >  	echo content >template-source/file &&
> > 
> 
> Michael
>