Skip to content

early-config-v1

Hopefully these patches will lead to something that we can integrate,
and that eventually will make Git's startup sequence much less
surprising.

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

Notes:

- I find the diff pretty ugly: I wish there was a more elegant way to
  *disable* discovery of .git/ *just* for `init` and `clone`. I
  considered a function `about_to_create_git_dir()` that is called in a
  hard-coded manner *only* for `init` and `clone`, but that would
  introduce another magic side effect, when all I want is to reduce those.

- For the moment, I do not handle dashed invocations of `init` and
  `clone` correctly. The real problem is the continued existence of
  the dashed git-init and git-clone, of course.

- There is still duplicated code. For the sake of this RFC, I did not
  address that yet.

- The read_early_config() function is called multiple times, re-reading
  all the config files and re-discovering the .git/ directory multiple
  times, which is quite wasteful. For the sake of this RFC, I did not
  address that yet.

- t7006 fails and the error message is a bit cryptic (not to mention the
  involved function trace, which is mind-boggling for what is supposed
  to be a simply, shell script-based test suite). I did not have time to
  look into that yet.

- after discover_git_directory_gently() did its work, the code happily
  uses its result *only* for the current read_early_config() run, and
  lets setup_git_dir_gently() do the whole work *again*. For the sake of
  this RFC, I did not address that yet.

Johannes Schindelin (7):
  Make read_early_config() reusable
  read_early_config(): avoid .git/config hack when unneeded
  Mark builtins that create .git/ directories
  read_early_config(): special-case `init` and `clone`
  read_early_config(): really discover .git/
  WIP read_config_early(): respect ceiling directories
  WIP: read_early_config(): add tests

 builtin/am.c            |   2 +-
 builtin/blame.c         |   2 +-
 builtin/grep.c          |   4 +-
 builtin/log.c           |   4 +-
 builtin/var.c           |   2 +-
 cache.h                 |   8 ++--
 config.c                | 110 ++++++++++++++++++++++++++++++++++++++++++++++++
 diff.c                  |   4 +-
 git.c                   |  25 +++++------
 pager.c                 |  44 +++----------------
 t/helper/test-config.c  |  15 +++++++
 t/t1309-early-config.sh |  50 ++++++++++++++++++++++
 12 files changed, 209 insertions(+), 61 deletions(-)
 create mode 100755 t/t1309-early-config.sh

base-commit: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
--
2.11.0.rc3.windows.1

From fc498e3720358b918977d693eef05132bce00116 Mon Sep 17 00:00:00 2001
Message-Id: <fc498e3720358b918977d693eef05132bce00116.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 14:26:31 +0100
Subject: [PATCH/RFC 1/7] Make read_early_config() reusable
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

The pager configuration needs to be read early, possibly before
discovering any .git/ directory.

Let's not hide this function in pager.c, but make it available to other
callers.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h  |  1 +
 config.c | 31 +++++++++++++++++++++++++++++++
 pager.c  | 31 -------------------------------
 3 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/cache.h b/cache.h
index a50a61a197..96e0cb2523 100644
--- a/cache.h
+++ b/cache.h
@@ -1692,6 +1692,7 @@ extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
 					const char *name, const char *buf, size_t len, void *data);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
+extern void read_early_config(config_fn_t cb, void *data);
 extern void git_config(config_fn_t fn, void *);
 extern int git_config_with_options(config_fn_t fn, void *,
 				   struct git_config_source *config_source,
diff --git a/config.c b/config.c
index 83fdecb1bc..7dcd8d8622 100644
--- a/config.c
+++ b/config.c
@@ -1385,6 +1385,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 	}
 }

+void read_early_config(config_fn_t cb, void *data)
+{
+	git_config_with_options(cb, data, NULL, 1);
+
+	/*
+	 * Note that this is a really dirty hack that does the wrong thing in
+	 * many cases. The crux of the problem is that we cannot run
+	 * setup_git_directory() early on in git's setup, so we have no idea if
+	 * we are in a repository or not, and therefore are not sure whether
+	 * and how to read repository-local config.
+	 *
+	 * So if we _aren't_ in a repository (or we are but we would reject its
+	 * core.repositoryformatversion), we'll read whatever is in .git/config
+	 * blindly. Similarly, if we _are_ in a repository, but not at the
+	 * root, we'll fail to find .git/config (because it's really
+	 * ../.git/config, etc). See t7006 for a complete set of failures.
+	 *
+	 * However, we have historically provided this hack because it does
+	 * work some of the time (namely when you are at the top-level of a
+	 * valid repository), and would rarely make things worse (i.e., you do
+	 * not generally have a .git/config file sitting around).
+	 */
+	if (!startup_info->have_repository) {
+		struct git_config_source repo_config;
+
+		memset(&repo_config, 0, sizeof(repo_config));
+		repo_config.file = ".git/config";
+		git_config_with_options(cb, data, &repo_config, 1);
+	}
+}
+
 static void git_config_check_init(void);

 void git_config(config_fn_t fn, void *data)
diff --git a/pager.c b/pager.c
index ae79643363..73ca8bc3b1 100644
--- a/pager.c
+++ b/pager.c
@@ -43,37 +43,6 @@ static int core_pager_config(const char *var, const char *value, void *data)
 	return 0;
 }

-static void read_early_config(config_fn_t cb, void *data)
-{
-	git_config_with_options(cb, data, NULL, 1);
-
-	/*
-	 * Note that this is a really dirty hack that does the wrong thing in
-	 * many cases. The crux of the problem is that we cannot run
-	 * setup_git_directory() early on in git's setup, so we have no idea if
-	 * we are in a repository or not, and therefore are not sure whether
-	 * and how to read repository-local config.
-	 *
-	 * So if we _aren't_ in a repository (or we are but we would reject its
-	 * core.repositoryformatversion), we'll read whatever is in .git/config
-	 * blindly. Similarly, if we _are_ in a repository, but not at the
-	 * root, we'll fail to find .git/config (because it's really
-	 * ../.git/config, etc). See t7006 for a complete set of failures.
-	 *
-	 * However, we have historically provided this hack because it does
-	 * work some of the time (namely when you are at the top-level of a
-	 * valid repository), and would rarely make things worse (i.e., you do
-	 * not generally have a .git/config file sitting around).
-	 */
-	if (!startup_info->have_repository) {
-		struct git_config_source repo_config;
-
-		memset(&repo_config, 0, sizeof(repo_config));
-		repo_config.file = ".git/config";
-		git_config_with_options(cb, data, &repo_config, 1);
-	}
-}
-
 const char *git_pager(int stdout_is_tty)
 {
 	const char *pager;
--
2.11.0.rc3.windows.1

From ac26b262fa2429897064118bcbe6b80847556b1b Mon Sep 17 00:00:00 2001
Message-Id: <ac26b262fa2429897064118bcbe6b80847556b1b.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 14:44:19 +0100
Subject: [PATCH/RFC 2/7] read_early_config(): avoid .git/config hack when
 unneeded
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

So far, we only look whether the startup_info claims to have seen a
git_dir.

However, do_git_config_sequence() (and consequently the
git_config_with_options() call used by read_early_config() asks the
have_git_dir() function whether we have a .git/ directory, which in turn
also looks at git_dir and at the environment variable GIT_DIR.

Let's just use the same function, have_git_dir(), to determine whether we
have to look for .git/config specifically.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 7dcd8d8622..104c3c3dcd 100644
--- a/config.c
+++ b/config.c
@@ -1407,7 +1407,7 @@ void read_early_config(config_fn_t cb, void *data)
 	 * valid repository), and would rarely make things worse (i.e., you do
 	 * not generally have a .git/config file sitting around).
 	 */
-	if (!startup_info->have_repository) {
+	if (!have_git_dir()) {
 		struct git_config_source repo_config;

 		memset(&repo_config, 0, sizeof(repo_config));
--
2.11.0.rc3.windows.1

From 43a2d85a3bf92c9efacad722fafdbea77b409acd Mon Sep 17 00:00:00 2001
Message-Id: <43a2d85a3bf92c9efacad722fafdbea77b409acd.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 16:07:43 +0100
Subject: [PATCH/RFC 3/7] Mark builtins that create .git/ directories
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

To refactor read_early_config() so that it discovers .git/config
properly, we have to make certain that commands such as `git init` (i.e.
commands that create their own .git/ and therefore do *not* want to
be affected by any other .git/ directory) skip this discovery.

Let's introduce a flag that states for every builtin whether it creates
its own .git/ directory or not.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index dce529fcbf..61df78afc8 100644
--- a/git.c
+++ b/git.c
@@ -318,12 +318,13 @@ static int handle_alias(int *argcp, const char ***argv)
 #define RUN_SETUP		(1<<0)
 #define RUN_SETUP_GENTLY	(1<<1)
 #define USE_PAGER		(1<<2)
+#define CREATES_GIT_DIR         (1<<3)
 /*
  * require working tree to be present -- anything uses this needs
  * RUN_SETUP for reading from the configuration file.
  */
-#define NEED_WORK_TREE		(1<<3)
-#define SUPPORT_SUPER_PREFIX	(1<<4)
+#define NEED_WORK_TREE		(1<<4)
+#define SUPPORT_SUPER_PREFIX	(1<<5)

 struct cmd_struct {
 	const char *cmd;
@@ -412,7 +413,7 @@ static struct cmd_struct commands[] = {
 	{ "cherry", cmd_cherry, RUN_SETUP },
 	{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
 	{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-	{ "clone", cmd_clone },
+	{ "clone", cmd_clone, CREATES_GIT_DIR },
 	{ "column", cmd_column, RUN_SETUP_GENTLY },
 	{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
 	{ "commit-tree", cmd_commit_tree, RUN_SETUP },
@@ -438,7 +439,7 @@ static struct cmd_struct commands[] = {
 	{ "hash-object", cmd_hash_object },
 	{ "help", cmd_help },
 	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
-	{ "init", cmd_init_db },
+	{ "init", cmd_init_db, CREATES_GIT_DIR },
 	{ "init-db", cmd_init_db },
 	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
 	{ "log", cmd_log, RUN_SETUP },
--
2.11.0.rc3.windows.1

From fcc3262f1245b6fe846f434ae9d500aa953e39f7 Mon Sep 17 00:00:00 2001
Message-Id: <fcc3262f1245b6fe846f434ae9d500aa953e39f7.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 16:18:54 +0100
Subject: [PATCH/RFC 4/7] read_early_config(): special-case `init` and `clone`
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

The `init` and `clone` commands create their own .git/ directory,
therefore we must be careful not to read any repository config when
determining the pager settings.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/am.c    |  2 +-
 builtin/blame.c |  2 +-
 builtin/grep.c  |  4 ++--
 builtin/log.c   |  4 ++--
 builtin/var.c   |  2 +-
 cache.h         |  9 +++++----
 config.c        |  4 ++--
 diff.c          |  4 ++--
 git.c           | 16 ++++++++--------
 pager.c         | 13 +++++++------
 10 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 6981f42ce9..e6c2ee01bc 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1791,7 +1791,7 @@ static int do_interactive(struct am_state *state)
 			}
 			strbuf_release(&msg);
 		} else if (*reply == 'v' || *reply == 'V') {
-			const char *pager = git_pager(1);
+			const char *pager = git_pager(1, 1);
 			struct child_process cp = CHILD_PROCESS_INIT;

 			if (!pager)
diff --git a/builtin/blame.c b/builtin/blame.c
index 4ddfadb71f..5b7daa3686 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2913,7 +2913,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	assign_blame(&sb, opt);

 	if (!incremental)
-		setup_pager();
+		setup_pager(1);

 	free(final_commit_name);

diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6addb..363a753369 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -800,7 +800,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}

 	if (show_in_pager == default_pager)
-		show_in_pager = git_pager(1);
+		show_in_pager = git_pager(1, 1);
 	if (show_in_pager) {
 		opt.color = 0;
 		opt.name_only = 1;
@@ -896,7 +896,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}

 	if (!show_in_pager && !opt.status_only)
-		setup_pager();
+		setup_pager(1);

 	if (!use_index && (untracked || cached))
 		die(_("--cached or --untracked cannot be used with --no-index."));
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc2d8..96618d38cb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	if (rev->line_level_traverse)
 		line_log_init(rev, line_cb.prefix, &line_cb.args);

-	setup_pager();
+	setup_pager(1);
 }

 static void cmd_log_init(int argc, const char **argv, const char *prefix,
@@ -1600,7 +1600,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
-		setup_pager();
+		setup_pager(1);

 	if (output_directory) {
 		if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53a2d..879867b842 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -19,7 +19,7 @@ static const char *editor(int flag)

 static const char *pager(int flag)
 {
-	const char *pgm = git_pager(1);
+	const char *pgm = git_pager(1, 1);

 	if (!pgm)
 		pgm = "cat";
diff --git a/cache.h b/cache.h
index 96e0cb2523..6627239de8 100644
--- a/cache.h
+++ b/cache.h
@@ -1325,7 +1325,7 @@ extern const char *fmt_name(const char *name, const char *email);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
-extern const char *git_pager(int stdout_is_tty);
+extern const char *git_pager(int stdout_is_tty, int discover_git_dir);
 extern int git_ident_config(const char *, const char *, void *);
 extern void reset_ident_date(void);

@@ -1692,7 +1692,8 @@ extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
 					const char *name, const char *buf, size_t len, void *data);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern void read_early_config(config_fn_t cb, void *data);
+extern void read_early_config(config_fn_t cb, void *data,
+			      int discover_git_dir);
 extern void git_config(config_fn_t fn, void *);
 extern int git_config_with_options(config_fn_t fn, void *,
 				   struct git_config_source *config_source,
@@ -1888,12 +1889,12 @@ __attribute__((format (printf, 2, 3)))
 extern void write_file(const char *path, const char *fmt, ...);

 /* pager.c */
-extern void setup_pager(void);
+extern void setup_pager(int discover_git_dir);
 extern int pager_in_use(void);
 extern int pager_use_color;
 extern int term_columns(void);
 extern int decimal_width(uintmax_t);
-extern int check_pager_config(const char *cmd);
+extern int check_pager_config(const char *cmd, int discover_git_dir);
 extern void prepare_pager_args(struct child_process *, const char *pager);

 extern const char *editor_program;
diff --git a/config.c b/config.c
index 104c3c3dcd..4c756edca1 100644
--- a/config.c
+++ b/config.c
@@ -1385,7 +1385,7 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 	}
 }

-void read_early_config(config_fn_t cb, void *data)
+void read_early_config(config_fn_t cb, void *data, int discover_git_dir)
 {
 	git_config_with_options(cb, data, NULL, 1);

@@ -1407,7 +1407,7 @@ void read_early_config(config_fn_t cb, void *data)
 	 * valid repository), and would rarely make things worse (i.e., you do
 	 * not generally have a .git/config file sitting around).
 	 */
-	if (!have_git_dir()) {
+	if (discover_git_dir && !have_git_dir()) {
 		struct git_config_source repo_config;

 		memset(&repo_config, 0, sizeof(repo_config));
diff --git a/diff.c b/diff.c
index ec8728362d..0769c0590a 100644
--- a/diff.c
+++ b/diff.c
@@ -5267,6 +5267,6 @@ void setup_diff_pager(struct diff_options *opt)
 	 * --exit-code" in hooks and other scripts, we do not do so.
 	 */
 	if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
-	    check_pager_config("diff") != 0)
-		setup_pager();
+	    check_pager_config("diff", 1) != 0)
+		setup_pager(1);
 }
diff --git a/git.c b/git.c
index 61df78afc8..2b007b83ec 100644
--- a/git.c
+++ b/git.c
@@ -61,13 +61,13 @@ static void restore_env(int external_alias)
 	}
 }

-static void commit_pager_choice(void) {
+static void commit_pager_choice(int discover_git_dir) {
 	switch (use_pager) {
 	case 0:
 		setenv("GIT_PAGER", "cat", 1);
 		break;
 	case 1:
-		setup_pager();
+		setup_pager(discover_git_dir);
 		break;
 	default:
 		break;
@@ -261,7 +261,7 @@ static int handle_alias(int *argcp, const char ***argv)
 		if (alias_string[0] == '!') {
 			struct child_process child = CHILD_PROCESS_INIT;

-			commit_pager_choice();
+			commit_pager_choice(1);
 			restore_env(1);

 			child.use_shell = 1;
@@ -349,7 +349,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 		}

 		if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY))
-			use_pager = check_pager_config(p->cmd);
+			use_pager = check_pager_config(p->cmd, !(p->option & CREATES_GIT_DIR));
 		if (use_pager == -1 && p->option & USE_PAGER)
 			use_pager = 1;

@@ -357,7 +357,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 		    startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */
 			trace_repo_setup(prefix);
 	}
-	commit_pager_choice();
+	commit_pager_choice(!(p->option & CREATES_GIT_DIR));

 	if (!help && get_super_prefix()) {
 		if (!(p->option & SUPPORT_SUPER_PREFIX))
@@ -584,8 +584,8 @@ static void execv_dashed_external(const char **argv)
 		die("%s doesn't support --super-prefix", argv[0]);

 	if (use_pager == -1)
-		use_pager = check_pager_config(argv[0]);
-	commit_pager_choice();
+		use_pager = check_pager_config(argv[0], 1);
+	commit_pager_choice(1);

 	strbuf_addf(&cmd, "git-%s", argv[0]);

@@ -688,7 +688,7 @@ int cmd_main(int argc, const char **argv)
 		skip_prefix(argv[0], "--", &argv[0]);
 	} else {
 		/* The user didn't specify a command; give them help */
-		commit_pager_choice();
+		commit_pager_choice(1);
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
 		printf("\n%s\n", _(git_more_info_string));
diff --git a/pager.c b/pager.c
index 73ca8bc3b1..16b3cbe232 100644
--- a/pager.c
+++ b/pager.c
@@ -43,7 +43,7 @@ static int core_pager_config(const char *var, const char *value, void *data)
 	return 0;
 }

-const char *git_pager(int stdout_is_tty)
+const char *git_pager(int stdout_is_tty, int discover_git_dir)
 {
 	const char *pager;

@@ -53,7 +53,8 @@ const char *git_pager(int stdout_is_tty)
 	pager = getenv("GIT_PAGER");
 	if (!pager) {
 		if (!pager_program)
-			read_early_config(core_pager_config, NULL);
+			read_early_config(core_pager_config, NULL,
+					  discover_git_dir);
 		pager = pager_program;
 	}
 	if (!pager)
@@ -100,9 +101,9 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
 	setup_pager_env(&pager_process->env_array);
 }

-void setup_pager(void)
+void setup_pager(int discover_git_dir)
 {
-	const char *pager = git_pager(isatty(1));
+	const char *pager = git_pager(isatty(1), discover_git_dir);

 	if (!pager)
 		return;
@@ -208,7 +209,7 @@ static int pager_command_config(const char *var, const char *value, void *vdata)
 }

 /* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
-int check_pager_config(const char *cmd)
+int check_pager_config(const char *cmd, int discover_git_dir)
 {
 	struct pager_command_config_data data;

@@ -216,7 +217,7 @@ int check_pager_config(const char *cmd)
 	data.want = -1;
 	data.value = NULL;

-	read_early_config(pager_command_config, &data);
+	read_early_config(pager_command_config, &data, discover_git_dir);

 	if (data.value)
 		pager_program = data.value;
--
2.11.0.rc3.windows.1

From 15fa873f3d6960c14fb3195ed2d9dc4ec58ba402 Mon Sep 17 00:00:00 2001
Message-Id: <15fa873f3d6960c14fb3195ed2d9dc4ec58ba402.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 16:27:44 +0100
Subject: [PATCH/RFC 5/7] read_early_config(): really discover .git/
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

Earlier, we punted and simply assumed that we are in the top-level
directory of the project, and that there is no .git file but a .git/
directory so that we can read directly from .git/config.

Let's discover the .git/ directory correctly.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 config.c | 69 +++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/config.c b/config.c
index 4c756edca1..286d3cad66 100644
--- a/config.c
+++ b/config.c
@@ -1385,35 +1385,64 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 	}
 }

+/*
+ * Note that this is a really dirty hack that replicates what the
+ * setup_git_directory() function does, without changing the current
+ * working directory. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have to
+ * duplicate the work that setup_git_directory() would otherwise do.
+ */
+static int discover_git_directory_gently(struct strbuf *result)
+{
+	const char *p;
+
+	if (strbuf_getcwd(result) < 0)
+		return -1;
+	p = real_path_if_valid(result->buf);
+	if (!p)
+		return -1;
+	strbuf_reset(result);
+	strbuf_addstr(result, p);
+	for (;;) {
+		int len = result->len, i;
+
+		strbuf_addstr(result, "/" DEFAULT_GIT_DIR_ENVIRONMENT);
+		p = read_gitfile_gently(result->buf, &i);
+		if (p) {
+			strbuf_reset(result);
+			strbuf_addstr(result, p);
+			return 0;
+		}
+		if (is_git_directory(result->buf))
+			return 0;
+		strbuf_setlen(result, len);
+		if (is_git_directory(result->buf))
+			return 0;
+		for (i = len; i > 0; )
+			if (is_dir_sep(result->buf[--i]))
+				break;
+		if (!i)
+			return -1;
+		strbuf_setlen(result, i);
+	}
+}
+
 void read_early_config(config_fn_t cb, void *data, int discover_git_dir)
 {
+	struct strbuf buf = STRBUF_INIT;
+
 	git_config_with_options(cb, data, NULL, 1);

-	/*
-	 * Note that this is a really dirty hack that does the wrong thing in
-	 * many cases. The crux of the problem is that we cannot run
-	 * setup_git_directory() early on in git's setup, so we have no idea if
-	 * we are in a repository or not, and therefore are not sure whether
-	 * and how to read repository-local config.
-	 *
-	 * So if we _aren't_ in a repository (or we are but we would reject its
-	 * core.repositoryformatversion), we'll read whatever is in .git/config
-	 * blindly. Similarly, if we _are_ in a repository, but not at the
-	 * root, we'll fail to find .git/config (because it's really
-	 * ../.git/config, etc). See t7006 for a complete set of failures.
-	 *
-	 * However, we have historically provided this hack because it does
-	 * work some of the time (namely when you are at the top-level of a
-	 * valid repository), and would rarely make things worse (i.e., you do
-	 * not generally have a .git/config file sitting around).
-	 */
-	if (discover_git_dir && !have_git_dir()) {
+	if (discover_git_dir && !have_git_dir() &&
+	    !discover_git_directory_gently(&buf)) {
 		struct git_config_source repo_config;

 		memset(&repo_config, 0, sizeof(repo_config));
-		repo_config.file = ".git/config";
+		strbuf_addstr(&buf, "/config");
+		repo_config.file = buf.buf;
 		git_config_with_options(cb, data, &repo_config, 1);
 	}
+	strbuf_release(&buf);
 }

 static void git_config_check_init(void);
--
2.11.0.rc3.windows.1

From 07c329e9d4875c2137d7b9ceedcb9241fc372b31 Mon Sep 17 00:00:00 2001
Message-Id: <07c329e9d4875c2137d7b9ceedcb9241fc372b31.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 18:15:08 +0100
Subject: [PATCH/RFC 6/7] WIP read_config_early(): respect ceiling directories
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

This ports the part from setup_git_directory_gently_1() where the
GIT_CEILING_DIRECTORIES environment variable is handled.

TODO: DRY up code again (exporting canonicalize_ceiling_directories()?)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 config.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 286d3cad66..eda3546491 100644
--- a/config.c
+++ b/config.c
@@ -1386,6 +1386,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 }

 /*
+ * A "string_list_each_func_t" function that canonicalizes an entry
+ * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
+ * discards it if unusable.  The presence of an empty entry in
+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all
+ * subsequent entries.
+ */
+static int canonicalize_ceiling_entry(struct string_list_item *item,
+				      void *cb_data)
+{
+	int *empty_entry_found = cb_data;
+	char *ceil = item->string;
+
+	if (!*ceil) {
+		*empty_entry_found = 1;
+		return 0;
+	} else if (!is_absolute_path(ceil)) {
+		return 0;
+	} else if (*empty_entry_found) {
+		/* Keep entry but do not canonicalize it */
+		return 1;
+	} else {
+		const char *real_path = real_path_if_valid(ceil);
+		if (!real_path)
+			return 0;
+		free(item->string);
+		item->string = xstrdup(real_path);
+		return 1;
+	}
+}
+
+/*
  * Note that this is a really dirty hack that replicates what the
  * setup_git_directory() function does, without changing the current
  * working directory. The crux of the problem is that we cannot run
@@ -1394,6 +1425,8 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
  */
 static int discover_git_directory_gently(struct strbuf *result)
 {
+	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
+	int ceiling_offset = -1;
 	const char *p;

 	if (strbuf_getcwd(result) < 0)
@@ -1403,6 +1436,23 @@ static int discover_git_directory_gently(struct strbuf *result)
 		return -1;
 	strbuf_reset(result);
 	strbuf_addstr(result, p);
+
+	if (env_ceiling_dirs) {
+		struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+		int empty_entry_found = 0;
+
+		string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP,
+				  -1);
+		filter_string_list(&ceiling_dirs, 0, canonicalize_ceiling_entry,
+				   &empty_entry_found);
+		ceiling_offset = longest_ancestor_length(result->buf,
+							 &ceiling_dirs);
+		string_list_clear(&ceiling_dirs, 0);
+	}
+
+	if (ceiling_offset < 0 && has_dos_drive_prefix(result->buf))
+		ceiling_offset = 1;
+
 	for (;;) {
 		int len = result->len, i;

@@ -1418,10 +1468,10 @@ static int discover_git_directory_gently(struct strbuf *result)
 		strbuf_setlen(result, len);
 		if (is_git_directory(result->buf))
 			return 0;
-		for (i = len; i > 0; )
-			if (is_dir_sep(result->buf[--i]))
+		for (i = len; --i > ceiling_offset; )
+			if (is_dir_sep(result->buf[i]))
 				break;
-		if (!i)
+		if (i <= ceiling_offset)
 			return -1;
 		strbuf_setlen(result, i);
 	}
--
2.11.0.rc3.windows.1

From 62f04326163842ccfcffce5e5d6ffb48f8d3035e Mon Sep 17 00:00:00 2001
Message-Id: <62f04326163842ccfcffce5e5d6ffb48f8d3035e.1481211338.git.johannes.schindelin@gmx.de>
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
References: <cover.1481211338.git.johannes.schindelin@gmx.de>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 7 Dec 2016 18:22:14 +0100
Subject: [PATCH/RFC 7/7] WIP: read_early_config(): add tests
Content-Type: text/plain; charset=UTF-8
Fcc: Sent
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
    Jeff King <peff@peff.net>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/helper/test-config.c  | 15 +++++++++++++++
 t/t1309-early-config.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100755 t/t1309-early-config.sh

diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 83a4f2ab86..5105069587 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -66,6 +66,16 @@ static int iterate_cb(const char *var, const char *value, void *data)
 	return 0;
 }

+static int early_config_cb(const char *var, const char *value, void *vdata)
+{
+	const char *key = vdata;
+
+	if (!strcmp(key, var))
+		printf("%s\n", value);
+
+	return 0;
+}
+
 int cmd_main(int argc, const char **argv)
 {
 	int i, val;
@@ -73,6 +83,11 @@ int cmd_main(int argc, const char **argv)
 	const struct string_list *strptr;
 	struct config_set cs;

+	if (argc == 3 && !strcmp(argv[1], "read_early_config")) {
+		read_early_config(early_config_cb, (void *)argv[2], 1);
+		return 0;
+	}
+
 	setup_git_directory();

 	git_configset_init(&cs);
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
new file mode 100755
index 0000000000..0c55dee514
--- /dev/null
+++ b/t/t1309-early-config.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test read_early_config()'
+
+. ./test-lib.sh
+
+test_expect_success 'read early config' '
+	test_config early.config correct &&
+	test-config read_early_config early.config >output &&
+	test correct = "$(cat output)"
+'
+
+test_expect_success 'in a sub-directory' '
+	test_config early.config sub &&
+	mkdir -p sub &&
+	(
+		cd sub &&
+		test-config read_early_config early.config
+	) >output &&
+	test sub = "$(cat output)"
+'
+
+test_expect_success 'ceiling' '
+	test_config early.config ceiling &&
+	mkdir -p sub &&
+	(
+		GIT_CEILING_DIRECTORIES="$PWD" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd sub &&
+		test-config read_early_config early.config
+	) >output &&
+	test -z "$(cat output)"
+'
+
+test_expect_success 'ceiling #2' '
+	mkdir -p xdg/git &&
+	git config -f xdg/git/config early.config xdg &&
+	test_config early.config ceiling &&
+	mkdir -p sub &&
+	(
+		XDG_CONFIG_HOME="$PWD"/xdg &&
+		GIT_CEILING_DIRECTORIES="$PWD" &&
+		export GIT_CEILING_DIRECTORIES XDG_CONFIG_HOME &&
+		cd sub &&
+		test-config read_early_config early.config
+	) >output &&
+	test xdg = "$(cat output)"
+'
+
+test_done
--
2.11.0.rc3.windows.1

Submitted-As: https://public-inbox.org/git/cover.1481211338.git.johannes.schindelin@gmx.de
Assets 2