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

[PATCH v4 00/10] Fix the early config

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Mar 7, 2017, 14:32 UTC
Message-ID
<cover.1488897111.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1488562287.git.johannes.schindelin@gmx.de>

These patches are an attempt to make Git's startup sequence a bit less surprising.

The idea here is to discover the .git/ directory gently (i.e. without changing the current working directory, nor any global variables), and to use it to read the .git/config file early, before we actually called setup_git_directory() (if we ever do that).

This also allows us to fix the early config e.g. to determine the pager or to resolve aliases in a non-surprising manner.

My own use case: in the GVFS Git fork, we need to execute pre-command and post-command hooks before and after *every* Git command. A previous version of the pre-command/post-command hook support was broken, as it used run_hook() which implicitly called setup_git_directory() too early. The discover_git_directory() function (and due to core.hooksPath also the read_early_config() function) helped me fix this.

Notable notes:
- Even if it can cause surprising problems, `init` and `clone` are not
  special-cased. Rationale: it would introduce technical debt and
  violate the Principle Of Least Astonishment.
- The read_early_config() function does not cache Git directory
  discovery nor read values. This is left for another patch series, if
  it ever becomes necessary.
- The alias handling in git.c could possibly benefit from this work, but
  again, this is a separate topic from the current patch series.
Changes since v3:
- instead of just fixing the `== '/'` comparison, 2/9 now *also* changes
  the style of the original to something Linux' checkpatch.pl prefers.
- fixed the comment added in 8/9 which was stale from an earlier
  iteration of this patch series.
- adjusted the commit above discover_git_directory()'s declaration, to
  make it more understandable.
- touched up the commit message of 5/9 to make the flow more natural.
- moved a fault fixup: by mistake, the repository_format check was added
  to 8/9 when it really needed to go to 5/9, where the
  discover_git_directory() function was introduced (I noticed this while
  rebasing).
- fixed discover_git_directory() when setup_git_directory_gently_1()
  returned GIT_DIR_NONE (the previous iteration would *not* return NULL
  in that case).
Johannes Schindelin (10):
  t7006: replace dubious test
  setup_git_directory(): use is_dir_sep() helper
  Prepare setup_discovered_git_directory() the root directory
  setup_git_directory_1(): avoid changing global state
  Introduce the discover_git_directory() function
  Make read_early_config() reusable
  read_early_config(): avoid .git/config hack when unneeded
  read_early_config(): really discover .git/
  Test read_early_config()
  setup_git_directory_gently_1(): avoid die()ing
 cache.h                 |   8 ++
 config.c                |  25 +++++
 pager.c                 |  31 ------
 setup.c                 | 246 +++++++++++++++++++++++++++++++++---------------
 t/helper/test-config.c  |  15 +++
 t/t1309-early-config.sh |  50 ++++++++++
 t/t7006-pager.sh        |  18 +++-
 7 files changed, 282 insertions(+), 111 deletions(-)
 create mode 100755 t/t1309-early-config.sh
base-commit: 3bc53220cb2dcf709f7a027a3f526befd021d858
Published-As: https://github.com/dscho/git/releases/tag/early-config-v4
Fetch-It-Via: git fetch https://github.com/dscho/git early-config-v4
Interdiff vs v3:
 diff --git a/cache.h b/cache.h
 index 8a4580f921d..e7b57457e73 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -518,7 +518,12 @@ extern void set_git_work_tree(const char *tree);
  #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
  
  extern void setup_work_tree(void);
 -/* Find GIT_DIR without changing the working directory or other global state */
 +/*
 + * Find GIT_DIR of the repository that contains the current working directory,
 + * without changing the working directory or other global state. The result is
 + * appended to gitdir. The return value is either NULL if no repository was
 + * found, or pointing to the path inside gitdir's buffer.
 + */
  extern const char *discover_git_directory(struct strbuf *gitdir);
  extern const char *setup_git_directory_gently(int *);
  extern const char *setup_git_directory(void);
 diff --git a/config.c b/config.c
 index 749623a9649..a88df53fdbc 100644
 --- a/config.c
 +++ b/config.c
 @@ -1419,14 +1419,12 @@ void read_early_config(config_fn_t cb, void *data)
  	git_config_with_options(cb, data, NULL, 1);
  
  	/*
 -	 * When we are not about to create a repository ourselves (init or
 -	 * clone) and when no .git/ directory was set up yet (in which case
 -	 * git_config_with_options() would already have picked up the
 -	 * repository config), we ask discover_git_directory() to figure out
 -	 * whether there is any repository config we should use (but unlike
 +	 * When setup_git_directory() was not yet asked to discover the
 +	 * GIT_DIR, we ask discover_git_directory() to figure out whether there
 +	 * is any repository config we should use (but unlike
  	 * setup_git_directory_gently(), no global state is changed, most
 -	 * notably, the current working directory is still the same after
 -	 * the call).
 +	 * notably, the current working directory is still the same after the
 +	 * call).
  	 */
  	if (!have_git_dir() && discover_git_directory(&buf)) {
  		struct git_config_source repo_config;
 diff --git a/setup.c b/setup.c
 index 5320ae37314..9118b48590a 100644
 --- a/setup.c
 +++ b/setup.c
 @@ -825,7 +825,8 @@ enum discovery_result {
  	GIT_DIR_BARE,
  	/* these are errors */
  	GIT_DIR_HIT_CEILING = -1,
 -	GIT_DIR_HIT_MOUNT_POINT = -2
 +	GIT_DIR_HIT_MOUNT_POINT = -2,
 +	GIT_DIR_INVALID_GITFILE = -3
  };
  
  /*
 @@ -842,7 +843,8 @@ enum discovery_result {
   * `dir` (i.e. *not* necessarily the cwd).
   */
  static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 -							  struct strbuf *gitdir)
 +							  struct strbuf *gitdir,
 +							  int die_on_error)
  {
  	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
  	struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
 @@ -890,14 +892,22 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
  	if (one_filesystem)
  		current_device = get_device_or_die(dir->buf, NULL, 0);
  	for (;;) {
 -		int offset = dir->len;
 +		int offset = dir->len, error_code = 0;
  
  		if (offset > min_offset)
  			strbuf_addch(dir, '/');
  		strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
 -		gitdirenv = read_gitfile(dir->buf);
 -		if (!gitdirenv && is_git_directory(dir->buf))
 -			gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
 +		gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
 +						NULL : &error_code);
 +		if (!gitdirenv) {
 +			if (die_on_error ||
 +			    error_code == READ_GITFILE_ERR_NOT_A_FILE) {
 +				if (is_git_directory(dir->buf))
 +				    gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
 +			} else if (error_code &&
 +				   error_code != READ_GITFILE_ERR_STAT_FAILED)
 +				return GIT_DIR_INVALID_GITFILE;
 +		}
  		strbuf_setlen(dir, offset);
  		if (gitdirenv) {
  			strbuf_addstr(gitdir, gitdirenv);
 @@ -912,7 +922,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
  		if (offset <= min_offset)
  			return GIT_DIR_HIT_CEILING;
  
 -		while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]));
 +		while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]))
 +			; /* continue */
  		if (offset <= ceil_offset)
  			return GIT_DIR_HIT_CEILING;
  
 @@ -933,7 +944,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
  		return NULL;
  
  	cwd_len = dir.len;
 -	if (setup_git_directory_gently_1(&dir, gitdir) < 0) {
 +	if (setup_git_directory_gently_1(&dir, gitdir, 0) <= 0) {
  		strbuf_release(&dir);
  		return NULL;
  	}
 @@ -963,7 +974,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
  		return NULL;
  	}
  
 -	return gitdir->buf;
 +	return gitdir->buf + gitdir_offset;
  }
  
  const char *setup_git_directory_gently(int *nongit_ok)
 @@ -992,7 +1003,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
  		die_errno(_("Unable to read current working directory"));
  	strbuf_addbuf(&dir, &cwd);
  
 -	switch (setup_git_directory_gently_1(&dir, &gitdir)) {
 +	switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
  	case GIT_DIR_NONE:
  		prefix = NULL;
  		break;
-- 
2.12.0.windows.1.7.g94dafc3b124
Previous: Junio C HamanoNext: Johannes Schindelin
Message 70 of 123 in “Pie-in-the-sky attempt to fix the early config”
  1. 0/7 Pie-in-the-sky attempt to fix the early configJohannes Schindelin, Dec 8, 2016
  2. 3/7 Mark builtins that create .git/ directoriesJohannes Schindelin, Dec 8, 2016
  3. 1/7 Make read_early_config() reusableJohannes Schindelin, Dec 8, 2016
  4. 2/7 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Dec 8, 2016
  5. 4/7 read_early_config(): special-case `init` and `clone`Johannes Schindelin, Dec 8, 2016
  6. 5/7 read_early_config(): really discover .git/Johannes Schindelin, Dec 8, 2016
  7. 7/7 WIP: read_early_config(): add testsJohannes Schindelin, Dec 8, 2016
  8. 6/7 WIP read_config_early(): respect ceiling directoriesJohannes Schindelin, Dec 8, 2016
  9. Jeff KingDec 8, 2016
  10. Johannes SchindelinDec 9, 2016
  11. Jeff KingDec 9, 2016
  12. Duy NguyenDec 9, 2016
  13. Johannes SchindelinDec 9, 2016
  14. 0/9 Fix the early configJohannes Schindelin, Mar 3, 2017
  15. 3/9 setup_git_directory(): avoid changing global state during discoveryJohannes Schindelin, Mar 3, 2017
  16. Jeff KingMar 3, 2017
  17. Johannes SchindelinMar 3, 2017
  18. 4/9 Export the discover_git_directory() functionJohannes Schindelin, Mar 3, 2017
  19. Jeff KingMar 3, 2017
  20. Johannes SchindelinMar 3, 2017
  21. 2/9 setup_git_directory(): use is_dir_sep() helperJohannes Schindelin, Mar 3, 2017
  22. Jeff KingMar 3, 2017
  23. Johannes SchindelinMar 3, 2017
  24. Jeff KingMar 3, 2017
  25. Johannes SchindelinMar 3, 2017
  26. 1/9 t7006: replace dubious testJohannes Schindelin, Mar 3, 2017
  27. Jeff KingMar 3, 2017
  28. Johannes SchindelinMar 3, 2017
  29. 5/9 Make read_early_config() reusableJohannes Schindelin, Mar 3, 2017
  30. Jeff KingMar 3, 2017
  31. Johannes SchindelinMar 3, 2017
  32. 6/9 read_early_config(): special-case builtins that create a repositoryJohannes Schindelin, Mar 3, 2017
  33. Jeff KingMar 3, 2017
  34. Johannes SchindelinMar 3, 2017
  35. 7/9 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Mar 3, 2017
  36. Jeff KingMar 3, 2017
  37. 9/9 Test read_early_config()Johannes Schindelin, Mar 3, 2017
  38. Jeff KingMar 3, 2017
  39. Johannes SchindelinMar 3, 2017
  40. 8/9 read_early_config(): really discover .git/Johannes Schindelin, Mar 3, 2017
  41. Jeff KingMar 3, 2017
  42. Johannes SchindelinMar 3, 2017
  43. Jeff KingMar 3, 2017
  44. Johannes SchindelinMar 3, 2017
  45. 0/9 Fix the early configJohannes Schindelin, Mar 3, 2017
  46. 1/9 t7006: replace dubious testJohannes Schindelin, Mar 3, 2017
  47. 2/9 setup_git_directory(): use is_dir_sep() helperJohannes Schindelin, Mar 3, 2017
  48. 3/9 Prepare setup_discovered_git_directory() the root directoryJohannes Schindelin, Mar 3, 2017
  49. 4/9 setup_git_directory_1(): avoid changing global stateJohannes Schindelin, Mar 3, 2017
  50. 7/9 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Mar 3, 2017
  51. 5/9 Export the discover_git_directory() functionJohannes Schindelin, Mar 3, 2017
  52. 6/9 Make read_early_config() reusableJohannes Schindelin, Mar 3, 2017
  53. 9/9 Test read_early_config()Johannes Schindelin, Mar 3, 2017
  54. 8/9 read_early_config(): really discover .git/Johannes Schindelin, Mar 3, 2017
  55. Junio C HamanoMar 3, 2017
  56. Johannes SchindelinMar 7, 2017
  57. Johannes SchindelinMar 7, 2017
  58. Jeff KingMar 4, 2017
  59. Junio C HamanoMar 5, 2017
  60. Johannes SchindelinMar 7, 2017
  61. Jeff KingMar 8, 2017
  62. Johannes SchindelinMar 8, 2017
  63. Jeff KingMar 8, 2017
  64. Junio C HamanoMar 8, 2017
  65. Jeff KingMar 8, 2017
  66. Junio C HamanoMar 8, 2017
  67. Johannes SchindelinMar 9, 2017
  68. Jeff KingMar 9, 2017
  69. Junio C HamanoMar 10, 2017
  70. 00/10 Fix the early configJohannes Schindelin, Mar 7, 2017
  71. 01/10 t7006: replace dubious testJohannes Schindelin, Mar 7, 2017
  72. 02/10 setup_git_directory(): use is_dir_sep() helperJohannes Schindelin, Mar 7, 2017
  73. 03/10 Prepare setup_discovered_git_directory() the root directoryJohannes Schindelin, Mar 7, 2017
  74. 04/10 setup_git_directory_1(): avoid changing global stateJohannes Schindelin, Mar 7, 2017
  75. Junio C HamanoMar 7, 2017
  76. Brandon WilliamsMar 7, 2017
  77. Johannes SchindelinMar 8, 2017
  78. Brandon WilliamsMar 8, 2017
  79. 05/10 Introduce the discover_git_directory() functionJohannes Schindelin, Mar 7, 2017
  80. 06/10 Make read_early_config() reusableJohannes Schindelin, Mar 7, 2017
  81. 08/10 read_early_config(): really discover .git/Johannes Schindelin, Mar 7, 2017
  82. 10/10 setup_git_directory_gently_1(): avoid die()ingJohannes Schindelin, Mar 7, 2017
  83. 09/10 Test read_early_config()Johannes Schindelin, Mar 7, 2017
  84. 07/10 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Mar 7, 2017
  85. 00/11 Fix the early configJohannes Schindelin, Mar 9, 2017
  86. 01/11 t7006: replace dubious testJohannes Schindelin, Mar 9, 2017
  87. 02/11 setup_git_directory(): use is_dir_sep() helperJohannes Schindelin, Mar 9, 2017
  88. 03/11 Prepare setup_discovered_git_directory() the root directoryJohannes Schindelin, Mar 9, 2017
  89. 04/11 setup_git_directory_1(): avoid changing global stateJohannes Schindelin, Mar 9, 2017
  90. Junio C HamanoMar 10, 2017
  91. 05/11 Introduce the discover_git_directory() functionJohannes Schindelin, Mar 9, 2017
  92. 07/11 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Mar 9, 2017
  93. 08/11 read_early_config(): really discover .git/Johannes Schindelin, Mar 9, 2017
  94. 06/11 Make read_early_config() reusableJohannes Schindelin, Mar 9, 2017
  95. 09/11 Test read_early_config()Johannes Schindelin, Mar 9, 2017
  96. Junio C HamanoMar 10, 2017
  97. Johannes SchindelinMar 13, 2017
  98. Junio C HamanoMar 13, 2017
  99. 10/11 setup_git_directory_gently_1(): avoid die()ingJohannes Schindelin, Mar 9, 2017
  100. Junio C HamanoMar 10, 2017
  101. Johannes SchindelinMar 13, 2017
  102. Junio C HamanoMar 13, 2017
  103. Junio C HamanoMar 13, 2017
  104. Johannes SchindelinMar 13, 2017
  105. Junio C HamanoMar 13, 2017
  106. 11/11 t1309: document cases where we would want early config not to die()Johannes Schindelin, Mar 9, 2017
  107. 00/12 Fix the early configJohannes Schindelin, Mar 13, 2017
  108. 01/12 t7006: replace dubious testJohannes Schindelin, Mar 13, 2017
  109. 02/12 setup_git_directory(): use is_dir_sep() helperJohannes Schindelin, Mar 13, 2017
  110. 03/12 Prepare setup_discovered_git_directory() the root directoryJohannes Schindelin, Mar 13, 2017
  111. Junio C HamanoMar 13, 2017
  112. Johannes SchindelinMar 13, 2017
  113. 04/12 setup_git_directory_1(): avoid changing global stateJohannes Schindelin, Mar 13, 2017
  114. 05/12 Introduce the discover_git_directory() functionJohannes Schindelin, Mar 13, 2017
  115. 08/12 read_early_config(): really discover .git/Johannes Schindelin, Mar 13, 2017
  116. 07/12 read_early_config(): avoid .git/config hack when unneededJohannes Schindelin, Mar 13, 2017
  117. 06/12 Make read_early_config() reusableJohannes Schindelin, Mar 13, 2017
  118. 09/12 Add t1309 to test read_early_config()Johannes Schindelin, Mar 13, 2017
  119. 10/12 setup_git_directory_gently_1(): avoid die()ingJohannes Schindelin, Mar 13, 2017
  120. 11/12 t1309: document cases where we would want early config not to die()Johannes Schindelin, Mar 13, 2017
  121. 12/12 setup.c: mention unresolved problemsJohannes Schindelin, Mar 13, 2017
  122. Junio C HamanoMar 13, 2017
  123. Jeff KingMar 14, 2017

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.