git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 15/20] init: allow alternate backends to be set for new repos

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Jan 15, 2016, 12:51 UTC
Message-ID
<20160115125139.GI10612@hank>
In-Reply-To
<1452788777-24954-16-git-send-email-dturner@twopensource.com>
On 01/14, David Turner wrote:
Show 111 quoted lines
> git init learns a new argument --ref-storage.  Presently, only
> "files" is supported, but later we will add other backends.
>
> When this argument is used, the repository's extensions.refStorage
> configuration value is set (as well as core.repositoryformatversion),
> and the refs backend's initdb function is used to set up the ref
> database.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  Documentation/git-init-db.txt |  2 +-
>  Documentation/git-init.txt    |  7 ++++++-
>  builtin/init-db.c             | 31 ++++++++++++++++++++++++++-----
>  cache.h                       |  2 ++
>  path.c                        | 29 +++++++++++++++++++++++++++--
>  refs.c                        |  8 ++++++++
>  refs.h                        |  8 ++++++++
>  setup.c                       | 12 ++++++++++++
>  t/t0001-init.sh               | 24 ++++++++++++++++++++++++
>  9 files changed, 114 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-init-db.txt b/Documentation/git-init-db.txt
> index 648a6cd..d03fb69 100644
> --- a/Documentation/git-init-db.txt
> +++ b/Documentation/git-init-db.txt
> @@ -9,7 +9,7 @@ git-init-db - Creates an empty Git repository
>  SYNOPSIS
>  --------
>  [verse]
> -'git init-db' [-q | --quiet] [--bare] [--template=<template_directory>] [--separate-git-dir <git dir>] [--shared[=<permissions>]]
> +'git init-db' [-q | --quiet] [--bare] [--template=<template_directory>] [--separate-git-dir <git dir>] [--shared[=<permissions>]] [--ref-storage=<name>]
>
>
>  DESCRIPTION
> 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.
> +
>  TEMPLATE DIRECTORY
>  ------------------
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 4771e7e..ebc747c 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);
> @@ -204,9 +206,6 @@ static int create_default_files(const char *template_path)
>  		adjust_shared_perm(get_git_dir());
>  	}
>
> -	if (refs_init_db(&err, shared_repository))
> -		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.
> @@ -214,14 +213,34 @@ static int create_default_files(const char *template_path)
>  	path = git_path_buf(&buf, "HEAD");
>  	reinit = (!access(path, R_OK)
>  		  || readlink(path, junk, sizeof(junk)-1) != -1);
> -	if (!reinit) {
> +	if (reinit) {
> +		if (requested_ref_storage_backend &&
> +		    strcmp(ref_storage_backend, requested_ref_storage_backend))
> +			die("You can't change the refs storage type (was %s; you requested %s)",
> +			    ref_storage_backend, requested_ref_storage_backend);
> +	} else {
>  		if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
>  			exit(1);
>  	}
>
> +	if (requested_ref_storage_backend)
> +		ref_storage_backend = requested_ref_storage_backend;
> +	if (strcmp(ref_storage_backend, "files")) {
> +		git_config_set("extensions.refStorage", ref_storage_backend);
> +		git_config_set("core.repositoryformatversion", ref_storage_backend);
> +#ifdef USE_LIBLMDB
> +		register_ref_storage_backend(&refs_be_lmdb);

refs_be_lmdb is not yet defined at this point in the patch series. It doesn't break anything, because USE_LIBLMDB doesn't leak through the makefile yet, but I still think it would make more sense to have the ifdef in 19/20.

> +#endif
> +		set_ref_storage_backend(ref_storage_backend);

More importantly, there is no check whether setting the ref storage backend succeeds. If a user accidentally sets an invalid value for the ref backend, a broken repository will be created without even warning the user.

While the repository will still work fine at this point in the series, git will die with an invalid ref backend in the config after 19/20. It can be fixed by setting extensions.refStorage to files in the config, because the backend is initialized to the default files backend below when the ref backend cannot be set.

I think it would be best to die() here, if setting the ref backend doesn't succeed, and clean up the files that were already written, instead of leaving the user with a broken repository.

Show 22 quoted lines
> +		repo_version = 1;
> +	}
> +
> +	if (refs_init_db(&err, shared_repository))
> +		die("failed to set up refs db: %s", err.buf);
> +
>  	/* This forces creation of new config file */
>  	xsnprintf(repo_version_string, sizeof(repo_version_string),
> -		  "%d", GIT_REPO_VERSION);
> +		  "%d", repo_version);
>  	git_config_set("core.repositoryformatversion", repo_version_string);
>
>  	/* Check filemode trustability */
> @@ -469,6 +488,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(0, "ref-storage", &requested_ref_storage_backend,
> +			   N_("name"), N_("name of backend type to use")),
>  		OPT_END()
>  	};
>
[...]
Show 8 quoted lines
> +test_expect_success 'init with bogus storage backend fails' '
> +
> +	(
> +		mkdir again2 &&
> +		cd again2 &&
> +		git init --ref-storage=test >out2 2>err2
> +	)
> +'

I noticed the above mainly because of this test, which doesn't seem to test what it claims to test. It only seems to test that git init succeeds, and writes something to out2 and err2, it's missing the checks that the contents are actually what they're supposed to be.

Show 26 quoted lines
> +
> +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
> +'
> +
>  test_expect_success 'init with --template' '
>  	mkdir template-source &&
>  	echo content >template-source/file &&
> --
> 2.4.2.749.g730654d-twtrsrc
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous: SZEDER GáborNext: David Turner
Message 20 of 49 in “refs backend rebase on pu”
  1. 00/20 refs backend rebase on puDavid Turner, Jan 14, 2016
  2. 01/20 refs: add a backend method structure with transaction functionsDavid Turner, Jan 14, 2016
  3. 02/20 refs: add methods for misc ref operationsDavid Turner, Jan 14, 2016
  4. 03/20 refs: add methods for the ref iteratorsDavid Turner, Jan 14, 2016
  5. 04/20 refs: add do_for_each_per_worktree_refDavid Turner, Jan 14, 2016
  6. 05/20 refs: add methods for reflogDavid Turner, Jan 14, 2016
  7. 06/20 refs: add method for initial ref transaction commitDavid Turner, Jan 14, 2016
  8. 07/20 refs: add method for delete_refsDavid Turner, Jan 14, 2016
  9. 08/20 refs: add methods to init refs dbDavid Turner, Jan 14, 2016
  10. 09/20 refs: add method to rename refsDavid Turner, Jan 14, 2016
  11. 10/20 refs: make lock genericDavid Turner, Jan 14, 2016
  12. 11/20 refs: move duplicate check to common codeDavid Turner, Jan 14, 2016
  13. 12/20 refs: allow log-only updatesDavid Turner, Jan 14, 2016
  14. 13/20 refs: resolve symbolic refs firstDavid Turner, Jan 14, 2016
  15. Jeff KingFeb 4, 2016
  16. David TurnerFeb 4, 2016
  17. 14/20 refs: always handle non-normal refs in files backendDavid Turner, Jan 14, 2016
  18. 15/20 init: allow alternate backends to be set for new reposDavid Turner, Jan 14, 2016
  19. SZEDER GáborJan 15, 2016
  20. Thomas GummererJan 15, 2016
  21. David TurnerJan 19, 2016
  22. Duy NguyenFeb 4, 2016
  23. David TurnerFeb 4, 2016
  24. 16/20 refs: check submodules ref storage configDavid Turner, Jan 14, 2016
  25. 17/20 refs: allow ref backend to be set for cloneDavid Turner, Jan 14, 2016
  26. SZEDER GáborJan 15, 2016
  27. David TurnerJan 19, 2016
  28. SZEDER GáborJan 21, 2016
  29. 18/20 svn: learn ref-storage argumentDavid Turner, Jan 14, 2016
  30. SZEDER GáborJan 15, 2016
  31. 19/20 refs: add LMDB refs backendDavid Turner, Jan 14, 2016
  32. Thomas GummererJan 15, 2016
  33. David TurnerJan 19, 2016
  34. Duy NguyenFeb 4, 2016
  35. David TurnerFeb 4, 2016
  36. 20/20 refs: tests for lmdb backendDavid Turner, Jan 14, 2016
  37. David TurnerFeb 2, 2016
  38. Junio C HamanoFeb 2, 2016
  39. Junio C HamanoFeb 4, 2016
  40. David TurnerFeb 4, 2016
  41. Ramsay JonesFeb 4, 2016
  42. Jeff KingFeb 4, 2016
  43. David TurnerFeb 4, 2016
  44. Duy NguyenFeb 4, 2016
  45. David TurnerFeb 4, 2016
  46. Duy NguyenFeb 4, 2016
  47. David TurnerFeb 4, 2016
  48. Ramsay JonesFeb 4, 2016
  49. David TurnerFeb 4, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.