git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 00/19] nd/setup part two, second round
@ 2010-03-21 10:30 Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 01/19] Move enter_repo() to setup.c Nguyễn Thái Ngọc Duy
                   ` (18 more replies)
  0 siblings, 19 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Again, this series is to make repository setup more predictable, i.e. must be done
in either setup_git_dir*, enter_repo or init_db. Compared to the first version [1]:

 - More detail in commit messages, mostly extracted from [2]
 - Fix "git foo -h" code path (that skips repo setup) in patches 14-16

[1] http://mid.gmane.org/1268313754-28179-1-git-send-email-pclouds@gmail.com
[2] http://mid.gmane.org/fcaeb9bf1003200110w721903e7v7a5823cb312cbc71@mail.gmail.com

Nguyễn Thái Ngọc Duy (19):
  Move enter_repo() to setup.c
  enter_repo(): initialize other variables as setup_git_directory_gently() does
  rev-parse --git-dir: print relative gitdir correctly
  worktree setup: call set_git_dir explicitly
  Add git_config_early()
  Use git_config_early() instead of git_config() during repo setup
  worktree setup: restore original state when things go wrong
  init/clone: turn on startup->have_repository properly
  git_config(): do not read .git/config if there is no repository
  Do not read .git/info/exclude if there is no repository
  Do not read .git/info/attributes if there is no repository
  apply: do not check sha1 when repository has not been found
  config: do not read .git/config if there is no repository
  run_builtin(): save "-h" detection result for later use
  builtins: utilize startup_info->help where possible
  builtins: check for startup_info->help, print and exit early
  Allow to undo setup_git_directory_gently() gracefully (and fix alias code)
  alias: keep repository found while collecting aliases as long as possible
  Guard unallowed access to repository when it's not set up

 attr.c                     |    5 +-
 builtin/apply.c            |    2 +-
 builtin/branch.c           |    3 +
 builtin/check-ref-format.c |    2 +-
 builtin/checkout-index.c   |    3 +
 builtin/clone.c            |    3 +-
 builtin/commit.c           |    6 ++
 builtin/config.c           |    9 ++-
 builtin/gc.c               |    3 +
 builtin/grep.c             |    2 +-
 builtin/index-pack.c       |    2 +-
 builtin/init-db.c          |   10 ++-
 builtin/log.c              |    7 +-
 builtin/ls-files.c         |    3 +
 builtin/merge-ours.c       |    2 +-
 builtin/merge.c            |    3 +
 builtin/pack-redundant.c   |    2 +-
 builtin/rev-parse.c        |    8 ++
 builtin/show-ref.c         |    2 +-
 builtin/update-index.c     |    3 +
 builtin/upload-archive.c   |    7 +-
 cache.h                    |    7 ++-
 config.c                   |   22 ++++-
 dir.c                      |    8 +-
 environment.c              |   33 +++++++-
 git.c                      |   33 +++++---
 path.c                     |   91 ---------------------
 setup.c                    |  191 +++++++++++++++++++++++++++++++++++++++++---
 t/t1300-repo-config.sh     |   14 +++
 t/t1302-repo-version.sh    |    2 +-
 t/t7002-grep.sh            |   24 ++++++
 31 files changed, 360 insertions(+), 152 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 01/19] Move enter_repo() to setup.c
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 02/19] enter_repo(): initialize other variables as setup_git_directory_gently() does Nguyễn Thái Ngọc Duy
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

enter_repo() is to set up a repository, it fits better in setup.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 path.c  |   91 ---------------------------------------------------------------
 setup.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 91 deletions(-)

diff --git a/path.c b/path.c
index b4c8d91..f42eb1b 100644
--- a/path.c
+++ b/path.c
@@ -332,97 +332,6 @@ return_null:
 	return NULL;
 }
 
-/*
- * First, one directory to try is determined by the following algorithm.
- *
- * (0) If "strict" is given, the path is used as given and no DWIM is
- *     done. Otherwise:
- * (1) "~/path" to mean path under the running user's home directory;
- * (2) "~user/path" to mean path under named user's home directory;
- * (3) "relative/path" to mean cwd relative directory; or
- * (4) "/absolute/path" to mean absolute directory.
- *
- * Unless "strict" is given, we try access() for existence of "%s.git/.git",
- * "%s/.git", "%s.git", "%s" in this order.  The first one that exists is
- * what we try.
- *
- * Second, we try chdir() to that.  Upon failure, we return NULL.
- *
- * Then, we try if the current directory is a valid git repository.
- * Upon failure, we return NULL.
- *
- * If all goes well, we return the directory we used to chdir() (but
- * before ~user is expanded), avoiding getcwd() resolving symbolic
- * links.  User relative paths are also returned as they are given,
- * except DWIM suffixing.
- */
-char *enter_repo(char *path, int strict)
-{
-	static char used_path[PATH_MAX];
-	static char validated_path[PATH_MAX];
-
-	if (!path)
-		return NULL;
-
-	if (!strict) {
-		static const char *suffix[] = {
-			".git/.git", "/.git", ".git", "", NULL,
-		};
-		int len = strlen(path);
-		int i;
-		while ((1 < len) && (path[len-1] == '/')) {
-			path[len-1] = 0;
-			len--;
-		}
-		if (PATH_MAX <= len)
-			return NULL;
-		if (path[0] == '~') {
-			char *newpath = expand_user_path(path);
-			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
-				free(newpath);
-				return NULL;
-			}
-			/*
-			 * Copy back into the static buffer. A pity
-			 * since newpath was not bounded, but other
-			 * branches of the if are limited by PATH_MAX
-			 * anyway.
-			 */
-			strcpy(used_path, newpath); free(newpath);
-			strcpy(validated_path, path);
-			path = used_path;
-		}
-		else if (PATH_MAX - 10 < len)
-			return NULL;
-		else {
-			path = strcpy(used_path, path);
-			strcpy(validated_path, path);
-		}
-		len = strlen(path);
-		for (i = 0; suffix[i]; i++) {
-			strcpy(path + len, suffix[i]);
-			if (!access(path, F_OK)) {
-				strcat(validated_path, suffix[i]);
-				break;
-			}
-		}
-		if (!suffix[i] || chdir(path))
-			return NULL;
-		path = validated_path;
-	}
-	else if (chdir(path))
-		return NULL;
-
-	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
-	    validate_headref("HEAD") == 0) {
-		set_git_dir(".");
-		check_repository_format();
-		return path;
-	}
-
-	return NULL;
-}
-
 int set_shared_perm(const char *path, int mode)
 {
 	struct stat st;
diff --git a/setup.c b/setup.c
index 8796c6f..3019da2 100644
--- a/setup.c
+++ b/setup.c
@@ -452,6 +452,97 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	return prefix;
 }
 
+/*
+ * First, one directory to try is determined by the following algorithm.
+ *
+ * (0) If "strict" is given, the path is used as given and no DWIM is
+ *     done. Otherwise:
+ * (1) "~/path" to mean path under the running user's home directory;
+ * (2) "~user/path" to mean path under named user's home directory;
+ * (3) "relative/path" to mean cwd relative directory; or
+ * (4) "/absolute/path" to mean absolute directory.
+ *
+ * Unless "strict" is given, we try access() for existence of "%s.git/.git",
+ * "%s/.git", "%s.git", "%s" in this order.  The first one that exists is
+ * what we try.
+ *
+ * Second, we try chdir() to that.  Upon failure, we return NULL.
+ *
+ * Then, we try if the current directory is a valid git repository.
+ * Upon failure, we return NULL.
+ *
+ * If all goes well, we return the directory we used to chdir() (but
+ * before ~user is expanded), avoiding getcwd() resolving symbolic
+ * links.  User relative paths are also returned as they are given,
+ * except DWIM suffixing.
+ */
+char *enter_repo(char *path, int strict)
+{
+	static char used_path[PATH_MAX];
+	static char validated_path[PATH_MAX];
+
+	if (!path)
+		return NULL;
+
+	if (!strict) {
+		static const char *suffix[] = {
+			".git/.git", "/.git", ".git", "", NULL,
+		};
+		int len = strlen(path);
+		int i;
+		while ((1 < len) && (path[len-1] == '/')) {
+			path[len-1] = 0;
+			len--;
+		}
+		if (PATH_MAX <= len)
+			return NULL;
+		if (path[0] == '~') {
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
+				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
+			strcpy(validated_path, path);
+			path = used_path;
+		}
+		else if (PATH_MAX - 10 < len)
+			return NULL;
+		else {
+			path = strcpy(used_path, path);
+			strcpy(validated_path, path);
+		}
+		len = strlen(path);
+		for (i = 0; suffix[i]; i++) {
+			strcpy(path + len, suffix[i]);
+			if (!access(path, F_OK)) {
+				strcat(validated_path, suffix[i]);
+				break;
+			}
+		}
+		if (!suffix[i] || chdir(path))
+			return NULL;
+		path = validated_path;
+	}
+	else if (chdir(path))
+		return NULL;
+
+	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
+	    validate_headref("HEAD") == 0) {
+		set_git_dir(".");
+		check_repository_format();
+		return path;
+	}
+
+	return NULL;
+}
+
 int git_config_perm(const char *var, const char *value)
 {
 	int i;
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 02/19] enter_repo(): initialize other variables as setup_git_directory_gently() does
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 01/19] Move enter_repo() to setup.c Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 03/19] rev-parse --git-dir: print relative gitdir correctly Nguyễn Thái Ngọc Duy
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 setup.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/setup.c b/setup.c
index 3019da2..e067292 100644
--- a/setup.c
+++ b/setup.c
@@ -535,8 +535,14 @@ char *enter_repo(char *path, int strict)
 
 	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
 	    validate_headref("HEAD") == 0) {
-		set_git_dir(".");
+		inside_work_tree = 0;
+		inside_git_dir = 1;
 		check_repository_format();
+		set_git_dir(".");
+		if (startup_info) {
+			startup_info->prefix = NULL;
+			startup_info->have_repository = 1;
+		}
 		return path;
 	}
 
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 03/19] rev-parse --git-dir: print relative gitdir correctly
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 01/19] Move enter_repo() to setup.c Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 02/19] enter_repo(): initialize other variables as setup_git_directory_gently() does Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 04/19] worktree setup: call set_git_dir explicitly Nguyễn Thái Ngọc Duy
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

When git_dir is relative, it is relative to Git's current working
directory, which is worktree top directory. "git rev-parse --git-dir"
is expected to output relative to user's current working directory.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/rev-parse.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 8fbf9d0..8819e8a 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -646,6 +646,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				static char cwd[PATH_MAX];
 				int len;
 				if (gitdir) {
+					if (prefix && !is_absolute_path(gitdir)) {
+						int len;
+						if (!getcwd(cwd, PATH_MAX))
+							die_errno("unable to get current working directory");
+						len = strlen(cwd);
+						printf("%s%s%s\n", cwd, len && cwd[len-1] != '/' ? "/" : "", gitdir);
+						continue;
+					}
 					puts(gitdir);
 					continue;
 				}
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 04/19] worktree setup: call set_git_dir explicitly
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 03/19] rev-parse --git-dir: print relative gitdir correctly Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 05/19] Add git_config_early() Nguyễn Thái Ngọc Duy
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Currently setup_git_env() can automatically set git directory to
".git", relative to current working directory, no matter where it
is. Any access to repository may call setup_git_env() automatically if
there is no repository found yet. This leads to obscure faults where
we don't expect a repository setup to happen.

The ultimate goal (**) is:

 - Git repository will be searched and set up by either one of these
   functions (and no other functions):

   + setup_git_directory_gently()
   + setup_git_directory()
   + enter_repo()
   + init_db()

 - Repository setup can't be called more than once. If it is really
   needed, then un-setup the previous repo before moving on with
   another one.

 - Any attempts to access repository when it's not found/set up should
   be caught.

This patch is the first step to accomplish this goal. It ensures that
after the "setup function" (mentioned above) is called, if a
repository is found, then setup_git_env() must be called already.

The two other cases (no setup function called,
setup_git_directory_gently() called but no repo found) will be covered
by the next patches, to ensure setup_git_env() will not be triggered.

(**) This only applies to builtin commands, for now.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 setup.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/setup.c b/setup.c
index e067292..43a8609 100644
--- a/setup.c
+++ b/setup.c
@@ -350,14 +350,17 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 				/* config may override worktree */
 				if (check_repository_format_gently(nongit_ok))
 					return NULL;
+				set_git_dir(gitdirenv);
 				return retval;
 			}
 			if (check_repository_format_gently(nongit_ok))
 				return NULL;
 			retval = get_relative_cwd(buffer, sizeof(buffer) - 1,
 					get_git_work_tree());
-			if (!retval || !*retval)
+			if (!retval || !*retval) {
+				set_git_dir(gitdirenv);
 				return NULL;
+			}
 			set_git_dir(make_absolute_path(gitdirenv));
 			if (chdir(work_tree_env) < 0)
 				die_errno ("Could not chdir to '%s'", work_tree_env);
@@ -392,8 +395,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	offset = len = strlen(cwd);
 	for (;;) {
 		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
-		if (gitfile_dir || is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT)) {
-			if (gitfile_dir && set_git_dir(gitfile_dir))
+		if (!gitfile_dir && is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
+			gitfile_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+		if (gitfile_dir) {
+			if (set_git_dir(gitfile_dir))
 				die("Repository setup failed");
 			inside_git_dir = 0;
 			if (!work_tree_env)
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 05/19] Add git_config_early()
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 04/19] worktree setup: call set_git_dir explicitly Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 06/19] Use git_config_early() instead of git_config() during repo setup Nguyễn Thái Ngọc Duy
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This version of git_config() will be used during repository setup.
As a repository is being set up, $GIT_DIR is not nailed down yet,
git_pathdup() should not be used to get $GIT_DIR/config.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h  |    1 +
 config.c |   19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 68412c0..3bd219c 100644
--- a/cache.h
+++ b/cache.h
@@ -933,6 +933,7 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
 extern int git_default_config(const char *, const char *, void *);
 extern int git_config_from_file(config_fn_t fn, const char *, void *);
 extern int git_config(config_fn_t fn, void *);
+extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_config_int(const char *, const char *);
 extern unsigned long git_config_ulong(const char *, const char *);
diff --git a/config.c b/config.c
index 6963fbe..2d38096 100644
--- a/config.c
+++ b/config.c
@@ -699,10 +699,9 @@ int git_config_global(void)
 	return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0);
 }
 
-int git_config(config_fn_t fn, void *data)
+int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	char *repo_config = NULL;
 	const char *home = NULL;
 
 	/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
@@ -724,17 +723,27 @@ int git_config(config_fn_t fn, void *data)
 		free(user_config);
 	}
 
-	repo_config = git_pathdup("config");
-	if (!access(repo_config, R_OK)) {
+	if (repo_config && !access(repo_config, R_OK)) {
 		ret += git_config_from_file(fn, repo_config, data);
 		found += 1;
 	}
-	free(repo_config);
 	if (found == 0)
 		return -1;
 	return ret;
 }
 
+int git_config(config_fn_t fn, void *data)
+{
+	char *repo_config = NULL;
+	int ret;
+
+	repo_config = git_pathdup("config");
+	ret = git_config_early(fn, data, repo_config);
+	if (repo_config)
+		free(repo_config);
+	return ret;
+}
+
 /*
  * Find all the stuff for git_config_set() below.
  */
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 06/19] Use git_config_early() instead of git_config() during repo setup
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (4 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 05/19] Add git_config_early() Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 07/19] worktree setup: restore original state when things go wrong Nguyễn Thái Ngọc Duy
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 setup.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/setup.c b/setup.c
index 43a8609..2c1b64f 100644
--- a/setup.c
+++ b/setup.c
@@ -241,9 +241,21 @@ void setup_work_tree(void)
 	initialized = 1;
 }
 
-static int check_repository_format_gently(int *nongit_ok)
+static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 {
-	git_config(check_repository_format_version, NULL);
+	char repo_config[PATH_MAX+1];
+
+	/*
+	 * git_config() can't be used here because it calls git_pathdup()
+	 * to get $GIT_CONFIG/config. That call will make setup_git_env()
+	 * set git_dir to ".git".
+	 *
+	 * We are in gitdir setup, no git dir has been found useable yet.
+	 * Use a gentler version of git_config() to check if this repo
+	 * is a good one.
+	 */
+	snprintf(repo_config, PATH_MAX, "%s/config", gitdir);
+	git_config_early(check_repository_format_version, NULL, repo_config);
 	if (GIT_REPO_VERSION < repository_format_version) {
 		if (!nongit_ok)
 			die ("Expected git repo version <= %d, found %d",
@@ -348,12 +360,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 			if (!work_tree_env) {
 				retval = set_work_tree(gitdirenv);
 				/* config may override worktree */
-				if (check_repository_format_gently(nongit_ok))
+				if (check_repository_format_gently(gitdirenv, nongit_ok))
 					return NULL;
 				set_git_dir(gitdirenv);
 				return retval;
 			}
-			if (check_repository_format_gently(nongit_ok))
+			if (check_repository_format_gently(gitdirenv, nongit_ok))
 				return NULL;
 			retval = get_relative_cwd(buffer, sizeof(buffer) - 1,
 					get_git_work_tree());
@@ -403,6 +415,8 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 			inside_git_dir = 0;
 			if (!work_tree_env)
 				inside_work_tree = 1;
+			if (check_repository_format_gently(gitfile_dir, nongit_ok))
+				return NULL;
 			root_len = offset_1st_component(cwd);
 			git_work_tree_cfg = xstrndup(cwd, offset > root_len ? offset : root_len);
 			break;
@@ -411,6 +425,8 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 			inside_git_dir = 1;
 			if (!work_tree_env)
 				inside_work_tree = 0;
+			if (check_repository_format_gently(".", nongit_ok))
+				return NULL;
 			if (offset != len) {
 				root_len = offset_1st_component(cwd);
 				cwd[offset > root_len ? offset : root_len] = '\0';
@@ -433,8 +449,6 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 			die_errno("Cannot change to '%s/..'", cwd);
 	}
 
-	if (check_repository_format_gently(nongit_ok))
-		return NULL;
 	if (offset == len)
 		return NULL;
 
@@ -542,7 +556,7 @@ char *enter_repo(char *path, int strict)
 	    validate_headref("HEAD") == 0) {
 		inside_work_tree = 0;
 		inside_git_dir = 1;
-		check_repository_format();
+		check_repository_format_gently(".", NULL);
 		set_git_dir(".");
 		if (startup_info) {
 			startup_info->prefix = NULL;
@@ -627,7 +641,7 @@ int check_repository_format_version(const char *var, const char *value, void *cb
 
 int check_repository_format(void)
 {
-	return check_repository_format_gently(NULL);
+	return check_repository_format_gently(get_git_dir(), NULL);
 }
 
 const char *setup_git_directory(void)
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 07/19] worktree setup: restore original state when things go wrong
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (5 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 06/19] Use git_config_early() instead of git_config() during repo setup Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 08/19] init/clone: turn on startup->have_repository properly Nguyễn Thái Ngọc Duy
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

In current state of setup_git_directory_gently(), when a repository is
not found, many things will not be the same as when setup procedure is
started (while they should be). For one thing, current working
directory will not be the same in certain cases.

This patch reorders actions in clear steps and rolls back if one step
is wrong. (Actually the last step, setting git_dir, can't be rolled
back by this patch).

The steps and their state change are:

 1. Looking in working directory for repository candidates (or $GIT_DIR
    if it is set)

    This step may move $(cwd) to somewhere.

 2. Set internal variables to record repository features, based only
    on repository location and environment variables.

    This step sets variables:
    - inside_git_dir
    - inside_work_tree

 3. Inspect $GIT_DIR/config to see if it's a valid repository.

    This step may change variables:
    - repository_format_version
    - shared_repository
    - is_bare_repository_cfg
    - git_work_tree_cfg

 4. Save the repository location for later use.

    This step calls setup_git_dir() to set many variables in environment.c

 5. Calculate prefix (relative path to the original current working
    directory)

Setup procedure is completed at step 5. Stopping at any other steps
is considered a setup failure.

In case of setup failure, if it's not fatal and nongit_ok is not NULL,
prefix must be calculated before returning to caller. In other words:

  if (!fatal && gently)
    goto step_5;

Things that go wrong before step 4 will be cleaned up by
unset_git_directory(). Step 4 is irreversible, for now.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h |    1 +
 setup.c |   46 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 3bd219c..8e9d818 100644
--- a/cache.h
+++ b/cache.h
@@ -418,6 +418,7 @@ extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern void setup_work_tree(void);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
+extern void unset_git_directory(const char *prefix);
 extern const char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern int check_filename(const char *prefix, const char *name);
diff --git a/setup.c b/setup.c
index 2c1b64f..4de7bf0 100644
--- a/setup.c
+++ b/setup.c
@@ -323,6 +323,33 @@ const char *read_gitfile_gently(const char *path)
 	return path;
 }
 
+void unset_git_directory(const char *prefix)
+{
+	/*
+	 * FIXME:
+	 * chdir(prefix) may be enough for most of cases,
+	 * if original cwd is outside worktree, prefix
+	 * will always be set NULL, thus impossible to move
+	 * back to orignal cwd
+	 */
+	if (prefix && chdir(prefix))
+		die("Cannot change to '%s'", prefix);
+
+	if (startup_info) {
+		startup_info->prefix = NULL;
+		startup_info->have_repository = 0;
+	}
+
+	/* Initialized in setup_git_directory_gently_1() */
+	inside_work_tree = -1;
+	inside_git_dir = -1;
+
+	/* Initialized in check_repository_format_version() */
+	repository_format_version = 0xFF;
+	shared_repository = PERM_UMASK;
+	is_bare_repository_cfg = -1;
+	git_work_tree_cfg = NULL;
+}
 /*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
@@ -403,6 +430,13 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	 * - ../ (bare)
 	 * - ../../.git/
 	 *   etc.
+	 *
+	 * When a repository is found:
+	 * - inside_git_dir/inside_work_tree are set
+	 * - check_repository_format_gently() is called
+	 *   if repo version is not supported, restore cwd
+	 * - set_git_dir
+	 * - calculate and return prefix
 	 */
 	offset = len = strlen(cwd);
 	for (;;) {
@@ -410,13 +444,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 		if (!gitfile_dir && is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
 			gitfile_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 		if (gitfile_dir) {
-			if (set_git_dir(gitfile_dir))
-				die("Repository setup failed");
 			inside_git_dir = 0;
 			if (!work_tree_env)
 				inside_work_tree = 1;
-			if (check_repository_format_gently(gitfile_dir, nongit_ok))
+			if (check_repository_format_gently(gitfile_dir, nongit_ok)) {
+				unset_git_directory(offset != len ? cwd + offset + 1: NULL);
 				return NULL;
+			}
+			if (set_git_dir(gitfile_dir))
+				die("Repository setup failed");
 			root_len = offset_1st_component(cwd);
 			git_work_tree_cfg = xstrndup(cwd, offset > root_len ? offset : root_len);
 			break;
@@ -425,8 +461,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 			inside_git_dir = 1;
 			if (!work_tree_env)
 				inside_work_tree = 0;
-			if (check_repository_format_gently(".", nongit_ok))
+			if (check_repository_format_gently(".", nongit_ok)) {
+				unset_git_directory(offset != len ? cwd + offset + 1: NULL);
 				return NULL;
+			}
 			if (offset != len) {
 				root_len = offset_1st_component(cwd);
 				cwd[offset > root_len ? offset : root_len] = '\0';
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 08/19] init/clone: turn on startup->have_repository properly
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (6 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 07/19] worktree setup: restore original state when things go wrong Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 09/19] git_config(): do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

With startup_info != NULL, many code path may be disabled, depending
on repo setup. Also move set_git_dir() closer to have_repository
assignment to make it clear about repo setup.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c   |    3 +--
 builtin/init-db.c |    9 +++++----
 cache.h           |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 58bacbd..ab7a3c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -460,9 +460,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	if (safe_create_leading_directories_const(git_dir) < 0)
 		die("could not create leading directories of '%s'", git_dir);
-	set_git_dir(make_absolute_path(git_dir));
 
-	init_db(option_template, option_quiet ? INIT_DB_QUIET : 0);
+	init_db(git_dir, option_template, option_quiet ? INIT_DB_QUIET : 0);
 
 	/*
 	 * At this point, the config exists, so we do not need the
diff --git a/builtin/init-db.c b/builtin/init-db.c
index edc40ff..064b919 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -294,12 +294,15 @@ static int create_default_files(const char *template_path)
 	return reinit;
 }
 
-int init_db(const char *template_dir, unsigned int flags)
+int init_db(const char *git_dir, const char *template_dir, unsigned int flags)
 {
 	const char *sha1_dir;
 	char *path;
 	int len, reinit;
 
+	set_git_dir(make_absolute_path(git_dir));
+	startup_info->have_repository = 1;
+
 	safe_create_dir(get_git_dir(), 0);
 
 	init_is_bare_repository = is_bare_repository();
@@ -509,7 +512,5 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 				   get_git_work_tree());
 	}
 
-	set_git_dir(make_absolute_path(git_dir));
-
-	return init_db(template_dir, flags);
+	return init_db(git_dir, template_dir, flags);
 }
diff --git a/cache.h b/cache.h
index 8e9d818..dd27e11 100644
--- a/cache.h
+++ b/cache.h
@@ -427,7 +427,7 @@ extern void verify_non_filename(const char *prefix, const char *name);
 
 #define INIT_DB_QUIET 0x0001
 
-extern int init_db(const char *template_dir, unsigned int flags);
+extern int init_db(const char *git_dir, const char *template_dir, unsigned int flags);
 
 #define alloc_nr(x) (((x)+16)*3/2)
 
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 09/19] git_config(): do not read .git/config if there is no repository
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (7 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 08/19] init/clone: turn on startup->have_repository properly Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 10/19] Do not read .git/info/exclude " Nguyễn Thái Ngọc Duy
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

If no repository is found, do not bother calling git_pathdup(). If a
command forgets to call setup_git_directory*() or enter_repo(),
$GIT_DIR/config will be missed, though.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 config.c               |    3 ++-
 t/t1300-repo-config.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/config.c b/config.c
index 2d38096..07d854a 100644
--- a/config.c
+++ b/config.c
@@ -737,7 +737,8 @@ int git_config(config_fn_t fn, void *data)
 	char *repo_config = NULL;
 	int ret;
 
-	repo_config = git_pathdup("config");
+	if (!startup_info || startup_info->have_repository)
+		repo_config = git_pathdup("config");
 	ret = git_config_early(fn, data, repo_config);
 	if (repo_config)
 		free(repo_config);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index f11f98c..cfb70e2 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -824,4 +824,17 @@ test_expect_success 'check split_cmdline return' "
 	test_must_fail git merge master
 	"
 
+test_expect_success 'skip .git/config if there is no repository' '
+	(
+		mkdir -p a/b/.git &&
+		cd a &&
+		GIT_CEILING_DIRECTORIES="`pwd`" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd b &&
+		echo "[core]" > .git/config &&
+		echo "wrong = true" >> .git/config &&
+		test -z "$(git var -l | grep core.wrong)"
+	)
+'
+
 test_done
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 10/19] Do not read .git/info/exclude if there is no repository
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (8 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 09/19] git_config(): do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 11/19] Do not read .git/info/attributes " Nguyễn Thái Ngọc Duy
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 dir.c           |    8 +++++---
 t/t7002-grep.sh |   24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 133c333..fee19fe 100644
--- a/dir.c
+++ b/dir.c
@@ -1024,9 +1024,11 @@ void setup_standard_excludes(struct dir_struct *dir)
 	const char *path;
 
 	dir->exclude_per_dir = ".gitignore";
-	path = git_path("info/exclude");
-	if (!access(path, R_OK))
-		add_excludes_from_file(dir, path);
+	if (!startup_info || startup_info->have_repository) {
+		path = git_path("info/exclude");
+		if (!access(path, R_OK))
+			add_excludes_from_file(dir, path);
+	}
 	if (excludes_file && !access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
 }
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index e249c3e..06ec4cb 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -527,4 +527,28 @@ test_expect_success 'grep -e -- -- path' '
 	test_cmp expected actual
 '
 
+test_expect_success 'Setup fake .git' '
+	cd t &&
+	GIT_CEILING_DIRECTORIES="`pwd`" &&
+	export GIT_CEILING_DIRECTORIES &&
+	cd a &&
+	mkdir -p .git/info &&
+	cd ../..
+
+'
+
+test_expect_success 'Ignore fake .git/info/exclude' '
+	(
+		cd t/a &&
+		echo v > .git/info/exclude &&
+		git grep --no-index vvv . &&
+		rm .git/info/exclude
+	)
+'
+
+test_expect_success 'Unsetup fake .git' '
+	rm -rf t/a &&
+	unset GIT_CEILING_DIRECTORIES
+'
+
 test_done
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 11/19] Do not read .git/info/attributes if there is no repository
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (9 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 10/19] Do not read .git/info/exclude " Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 12/19] apply: do not check sha1 when repository has not been found Nguyễn Thái Ngọc Duy
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This one is difficult to demonstrate by tests. So I'll describe the
call flow instead.

Say we do "git grep --no-index --show-function foo -- bar" in no
repository. The call flow would be:
 - ...
 - cmd_grep
 - grep_directory
 - grep_file
 - grep_buffer
 - grep_buffer_1
 - userdiff_find_by_path
 - git_checkattr("diff")
 - bootstrap_attr_stack
 - git_path("info/attributes")

Because no repository is found, git_dir in environment.c would be
NULL. When it reaches git_path("info/attributes"), it will
automatically set git_dir to ".git". If we have $(cwd)/.git/info/attributes
then that file will be read, no matter $(cwd)/.git is a valid
repository.

This bug is hard to be exposed, because after git_checkattr("diff")
finds something (wrong). It will look up for diff drivers from
.git/config. If we do thing correctly, .git/config will not be read,
thus no diff drivers, no visible impact. If .git/config is read,
git_dir must be set already, all this would not happen.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 attr.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/attr.c b/attr.c
index f5346ed..f1362de 100644
--- a/attr.c
+++ b/attr.c
@@ -480,7 +480,10 @@ static void bootstrap_attr_stack(void)
 			debug_push(elem);
 		}
 
-		elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);
+		if (!startup_info || startup_info->have_repository)
+			elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);
+		else
+			elem = NULL;
 		if (!elem)
 			elem = xcalloc(1, sizeof(*elem));
 		elem->origin = NULL;
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 12/19] apply: do not check sha1 when repository has not been found
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (10 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 11/19] Do not read .git/info/attributes " Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 13/19] config: do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/apply.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index d27aac6..ea7bf57 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2441,7 +2441,7 @@ static int apply_binary(struct image *img, struct patch *patch)
 		return 0; /* deletion patch */
 	}
 
-	if (has_sha1_file(sha1)) {
+	if (startup_info->have_repository && has_sha1_file(sha1)) {
 		/* We already have the postimage */
 		enum object_type type;
 		unsigned long size;
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 13/19] config: do not read .git/config if there is no repository
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (11 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 12/19] apply: do not check sha1 when repository has not been found Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 14/19] run_builtin(): save "-h" detection result for later use Nguyễn Thái Ngọc Duy
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/config.c        |    9 ++++++---
 t/t1300-repo-config.sh  |    3 ++-
 t/t1302-repo-version.sh |    2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index ecc8f87..3fca3b4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -159,7 +159,8 @@ static int get_value(const char *key_, const char *regex_)
 	local = config_exclusive_filename;
 	if (!local) {
 		const char *home = getenv("HOME");
-		local = repo_config = git_pathdup("config");
+		if (startup_info->have_repository)
+			local = repo_config = git_pathdup("config");
 		if (git_config_global() && home)
 			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
@@ -197,7 +198,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(show_config, system_wide, NULL);
 	if (do_all && global)
 		git_config_from_file(show_config, global, NULL);
-	git_config_from_file(show_config, local, NULL);
+	if (local)
+		git_config_from_file(show_config, local, NULL);
 	if (!do_all && !seen && global)
 		git_config_from_file(show_config, global, NULL);
 	if (!do_all && !seen && system_wide)
@@ -215,7 +217,8 @@ static int get_value(const char *key_, const char *regex_)
 		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
 
 free_strings:
-	free(repo_config);
+	if (repo_config)
+		free(repo_config);
 	free(global);
 	return ret;
 }
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index cfb70e2..b040f72 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -833,7 +833,8 @@ test_expect_success 'skip .git/config if there is no repository' '
 		cd b &&
 		echo "[core]" > .git/config &&
 		echo "wrong = true" >> .git/config &&
-		test -z "$(git var -l | grep core.wrong)"
+		test -z "$(git var -l | grep core.wrong)" &&
+		test -z "$(git config --bool core.wrong)"
 	)
 '
 
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index 8d305b4..74bf51f 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -29,7 +29,7 @@ test_expect_success 'gitdir selection on normal repos' '
 # Make sure it would stop at test2, not trash
 test_expect_success 'gitdir selection on unsupported repo' '
 	(cd test2 &&
-	test "$(git config core.repositoryformatversion)" = 99)'
+	test "$(git config --file=.git/config core.repositoryformatversion)" = 99)'
 
 test_expect_success 'gitdir not required mode' '
 	(git apply --stat test.patch &&
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 14/19] run_builtin(): save "-h" detection result for later use
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (12 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 13/19] config: do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 15/19] builtins: utilize startup_info->help where possible Nguyễn Thái Ngọc Duy
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

When run_builtin() sees "-h" as the first argument, it assumes that:
 - it's the call for help usage
 - the real git command will only print help usage then exit

So it skips all setup in this case. Unfortunately some commands may do
more things before calling parse_options(), which is where help usage
is printed. Some of those things may touch repository, which has yet
to be set up.

Until now, the only safe thing that can be called before
parse_options() is git_config(). This function is aware of repository
findind status, and will skip reading $GIT_DIR/config if no repository
is found.

Make real commands aware of this fast path so that they can handle it
properly (i.e. print help usage then exist immediately) if they do
more initialization than git_config().

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h |    1 +
 git.c   |    8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index dd27e11..e397e1d 100644
--- a/cache.h
+++ b/cache.h
@@ -1060,6 +1060,7 @@ int split_cmdline(char *cmdline, const char ***argv);
 struct startup_info {
 	const char *prefix;
 	int have_repository;
+	int help;
 };
 extern struct startup_info *startup_info;
 
diff --git a/git.c b/git.c
index 746470f..7e30498 100644
--- a/git.c
+++ b/git.c
@@ -237,13 +237,13 @@ struct cmd_struct {
 
 static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 {
-	int status, help;
+	int status;
 	struct stat st;
 
 	memset(&git_startup_info, 0, sizeof(git_startup_info));
 	startup_info = &git_startup_info;
-	help = argc == 2 && !strcmp(argv[1], "-h");
-	if (!help) {
+	startup_info->help = argc == 2 && !strcmp(argv[1], "-h");
+	if (!startup_info->help) {
 		if (p->option & RUN_SETUP)
 			setup_git_directory();
 		if (p->option & RUN_SETUP_GENTLY) {
@@ -261,7 +261,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	}
 	commit_pager_choice();
 
-	if (!help && p->option & NEED_WORK_TREE)
+	if (!startup_info->help && p->option & NEED_WORK_TREE)
 		setup_work_tree();
 
 	trace_argv_printf(argv, "trace: built-in: git");
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 15/19] builtins: utilize startup_info->help where possible
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (13 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 14/19] run_builtin(): save "-h" detection result for later use Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 16/19] builtins: check for startup_info->help, print and exit early Nguyễn Thái Ngọc Duy
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

It helps reduce false alarms while I'm looking for "git foo -h" code
path that accesses repository. Anyway it looks like a good thing to
do.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/check-ref-format.c |    2 +-
 builtin/grep.c             |    2 +-
 builtin/index-pack.c       |    2 +-
 builtin/log.c              |    7 ++-----
 builtin/merge-ours.c       |    2 +-
 builtin/pack-redundant.c   |    2 +-
 builtin/show-ref.c         |    2 +-
 7 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index b106c65..c13f90a 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -35,7 +35,7 @@ static void collapse_slashes(char *dst, const char *src)
 
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage(builtin_check_ref_format_usage);
 
 	if (argc == 3 && !strcmp(argv[1], "--branch")) {
diff --git a/builtin/grep.c b/builtin/grep.c
index fb82ff8..a7bdd7b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -857,7 +857,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	 * 'git grep -h', unlike 'git grep -h <pattern>', is a request
 	 * to show usage information and exit.
 	 */
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage_with_options(grep_usage, options);
 
 	memset(&opt, 0, sizeof(opt));
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 9aa6a13..44375a2 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -880,7 +880,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	struct pack_idx_entry **idx_objects;
 	unsigned char pack_sha1[20];
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage(index_pack_usage);
 
 	/*
diff --git a/builtin/log.c b/builtin/log.c
index e0d5caa..382d4a3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -50,12 +50,9 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	if (default_date_mode)
 		rev->date_mode = parse_date_format(default_date_mode);
 
-	/*
-	 * Check for -h before setup_revisions(), or "git log -h" will
-	 * fail when run without a git directory.
-	 */
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage(builtin_log_usage);
+
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 
 	if (!rev->show_notes_given && !rev->pretty_given)
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 6844116..8e0777b 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -20,7 +20,7 @@ static const char *diff_index_args[] = {
 
 int cmd_merge_ours(int argc, const char **argv, const char *prefix)
 {
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage(builtin_merge_ours_usage);
 
 	/*
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 41e1615..3f090b2 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -601,7 +601,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 	unsigned char *sha1;
 	char buf[42]; /* 40 byte sha1 + \n + \0 */
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage(pack_redundant_usage);
 
 	for (i = 1; i < argc; i++) {
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 17ada88..59f90df 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -204,7 +204,7 @@ static const struct option show_ref_options[] = {
 
 int cmd_show_ref(int argc, const char **argv, const char *prefix)
 {
-	if (argc == 2 && !strcmp(argv[1], "-h"))
+	if (startup_info->help)
 		usage_with_options(show_ref_usage, show_ref_options);
 
 	argc = parse_options(argc, argv, prefix, show_ref_options,
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 16/19] builtins: check for startup_info->help, print and exit early
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (14 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 15/19] builtins: utilize startup_info->help where possible Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 17/19] Allow to undo setup_git_directory_gently() gracefully (and fix alias code) Nguyễn Thái Ngọc Duy
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

These commands need more than just git_config() before parsing
commmand line arguments. Some of these activities will unconditionally
look into a repository. When startup_info->help is TRUE, no repository
is set up and the caller expects callees to print help usage and exit,
no more.

Do as expected.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c         |    3 +++
 builtin/checkout-index.c |    3 +++
 builtin/commit.c         |    6 ++++++
 builtin/gc.c             |    3 +++
 builtin/ls-files.c       |    3 +++
 builtin/merge.c          |    3 +++
 builtin/update-index.c   |    3 +++
 builtin/upload-archive.c |    7 ++++---
 8 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6cf7e72..cc0053b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -651,6 +651,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (startup_info->help)
+		usage_with_options(builtin_branch_usage, options);
+
 	git_config(git_branch_config, NULL);
 
 	if (branch_use_color == -1)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index a7a5ee1..7f25cd7 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -241,6 +241,9 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	if (startup_info->help)
+		usage_with_options(builtin_checkout_index_usage, builtin_checkout_index_options);
+
 	git_config(git_default_config, NULL);
 	state.base_dir = "";
 	prefix_length = prefix ? strlen(prefix) : 0;
diff --git a/builtin/commit.c b/builtin/commit.c
index f4c7344..b1a496e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1032,6 +1032,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (startup_info->help)
+		usage_with_options(builtin_status_usage, builtin_status_options);
+
 	if (null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
 
@@ -1172,6 +1175,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	int allow_fast_forward = 1;
 	struct wt_status s;
 
+	if (startup_info->help)
+		usage_with_options(builtin_commit_usage, builtin_commit_options);
+
 	wt_status_prepare(&s);
 	git_config(git_commit_config, &s);
 	in_merge = file_exists(git_path("MERGE_HEAD"));
diff --git a/builtin/gc.c b/builtin/gc.c
index c304638..f040171 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -191,6 +191,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 
 	git_config(gc_config, NULL);
 
+	if (startup_info->help)
+		usage_with_options(builtin_gc_usage, builtin_gc_options);
+
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b065061..5125560 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -518,6 +518,9 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	if (startup_info->help)
+		usage_with_options(ls_files_usage, builtin_ls_files_options);
+
 	memset(&dir, 0, sizeof(dir));
 	if (prefix)
 		prefix_offset = strlen(prefix);
diff --git a/builtin/merge.c b/builtin/merge.c
index 3aaec7b..11fadac 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -886,6 +886,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list **remotes = &remoteheads;
 
+	if (startup_info->help)
+		usage_with_options(builtin_merge_usage, builtin_merge_options);
+
 	if (read_cache_unmerged()) {
 		die_resolve_conflict("merge");
 	}
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3ab214d..46a53f5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -589,6 +589,9 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	int lock_error = 0;
 	struct lock_file *lock_file;
 
+	if (startup_info->help)
+		usage(update_index_usage);
+
 	git_config(git_default_config, NULL);
 
 	/* We can't free this memory, it becomes part of a linked list parsed atexit() */
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 73f788e..d4f4741 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -26,9 +26,6 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 	int sent_argc;
 	int len;
 
-	if (argc != 2)
-		usage(upload_archive_usage);
-
 	if (strlen(argv[1]) + 1 > sizeof(buf))
 		die("insanely long repository name");
 
@@ -98,6 +95,10 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
 	pid_t writer;
 	int fd1[2], fd2[2];
+
+	if (startup_info->help || argc != 2)
+		usage(upload_archive_usage);
+
 	/*
 	 * Set up sideband subprocess.
 	 *
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 17/19] Allow to undo setup_git_directory_gently() gracefully (and fix alias code)
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (15 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 16/19] builtins: check for startup_info->help, print and exit early Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 18/19] alias: keep repository found while collecting aliases as long as possible Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 19/19] Guard unallowed access to repository when it's not set up Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

unset_git_directory() can only clean up things as long as
set_git_dir() has not been called because set_git_dir() sets internal
state itself. Even worse, set_git_dir() may override $GIT_DIR env
variable.

Add unset_git_env() to undo set_git_dir(), allow unset_git_directory()
to undo a succesful setup_git_directory_gently().

While at there, fix alias handling code regarding git_dir {,un-}setup,
use unset_git_directory() instead of just chdir() back.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h       |    1 +
 environment.c |   20 ++++++++++++++++++++
 git.c         |   11 +++++------
 setup.c       |    2 ++
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/cache.h b/cache.h
index e397e1d..b1ed150 100644
--- a/cache.h
+++ b/cache.h
@@ -419,6 +419,7 @@ extern void setup_work_tree(void);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
 extern void unset_git_directory(const char *prefix);
+extern void unset_git_env();
 extern const char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern int check_filename(const char *prefix, const char *name);
diff --git a/environment.c b/environment.c
index c36c902..6127025 100644
--- a/environment.c
+++ b/environment.c
@@ -62,6 +62,7 @@ char *git_work_tree_cfg;
 static char *work_tree;
 
 static const char *git_dir;
+static const char *original_git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
 /*
@@ -81,6 +82,20 @@ const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = {
 	NULL
 };
 
+void unset_git_env(void)
+{
+	git_dir = NULL;
+	if (original_git_dir)
+		setenv(GIT_DIR_ENVIRONMENT, original_git_dir, 1);
+	else
+		unsetenv(GIT_DIR_ENVIRONMENT);
+	git_object_dir = NULL;
+	git_refs_dir = NULL;
+	git_index_file = NULL;
+	git_graft_file = NULL;
+	read_replace_refs = 1;
+}
+
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
@@ -184,6 +199,11 @@ char *get_graft_file(void)
 
 int set_git_dir(const char *path)
 {
+	static int original_git_dir_set = 0;
+	if (!original_git_dir_set) {
+		original_git_dir = getenv(GIT_DIR_ENVIRONMENT);
+		original_git_dir_set = 1;
+	}
 	if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
 		return error("Could not set GIT_DIR to '%s'", path);
 	setup_git_env();
diff --git a/git.c b/git.c
index 7e30498..db5bd95 100644
--- a/git.c
+++ b/git.c
@@ -146,14 +146,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 static int handle_alias(int *argcp, const char ***argv)
 {
 	int envchanged = 0, ret = 0, saved_errno = errno;
-	const char *subdir;
 	int count, option_count;
 	const char **new_argv;
 	const char *alias_command;
 	char *alias_string;
 	int unused_nongit;
 
-	subdir = setup_git_directory_gently(&unused_nongit);
+	setup_git_directory_gently(&unused_nongit);
 
 	alias_command = (*argv)[0];
 	alias_string = alias_lookup(alias_command);
@@ -210,8 +209,7 @@ static int handle_alias(int *argcp, const char ***argv)
 		ret = 1;
 	}
 
-	if (subdir && chdir(subdir))
-		die_errno("Cannot change to '%s'", subdir);
+	unset_git_directory(startup_info->prefix);
 
 	errno = saved_errno;
 
@@ -240,8 +238,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	int status;
 	struct stat st;
 
-	memset(&git_startup_info, 0, sizeof(git_startup_info));
-	startup_info = &git_startup_info;
 	startup_info->help = argc == 2 && !strcmp(argv[1], "-h");
 	if (!startup_info->help) {
 		if (p->option & RUN_SETUP)
@@ -486,6 +482,9 @@ int main(int argc, const char **argv)
 {
 	const char *cmd;
 
+	memset(&git_startup_info, 0, sizeof(git_startup_info));
+	startup_info = &git_startup_info;
+
 	cmd = git_extract_argv0_path(argv[0]);
 	if (!cmd)
 		cmd = "git-help";
diff --git a/setup.c b/setup.c
index 4de7bf0..1808ebe 100644
--- a/setup.c
+++ b/setup.c
@@ -336,6 +336,8 @@ void unset_git_directory(const char *prefix)
 		die("Cannot change to '%s'", prefix);
 
 	if (startup_info) {
+		if (startup_info->have_repository)
+			unset_git_env();
 		startup_info->prefix = NULL;
 		startup_info->have_repository = 0;
 	}
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 18/19] alias: keep repository found while collecting aliases as long as possible
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (16 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 17/19] Allow to undo setup_git_directory_gently() gracefully (and fix alias code) Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  2010-03-21 10:30 ` [PATCH v2 19/19] Guard unallowed access to repository when it's not set up Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Given that about 80% builtin commands will need to search for a
repository, we should keep the repository found in alias handling
code. Until we are clear there's no need for a repository, then we can
undo the setup with unset_git_directory().

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/git.c b/git.c
index db5bd95..88aaf13 100644
--- a/git.c
+++ b/git.c
@@ -152,7 +152,8 @@ static int handle_alias(int *argcp, const char ***argv)
 	char *alias_string;
 	int unused_nongit;
 
-	setup_git_directory_gently(&unused_nongit);
+	if (!startup_info->have_repository)
+		setup_git_directory_gently(&unused_nongit);
 
 	alias_command = (*argv)[0];
 	alias_string = alias_lookup(alias_command);
@@ -209,8 +210,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		ret = 1;
 	}
 
-	unset_git_directory(startup_info->prefix);
-
 	errno = saved_errno;
 
 	return ret;
@@ -240,12 +239,18 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	startup_info->help = argc == 2 && !strcmp(argv[1], "-h");
 	if (!startup_info->help) {
-		if (p->option & RUN_SETUP)
+		if ((p->option & RUN_SETUP) && !startup_info->have_repository)
 			setup_git_directory();
-		if (p->option & RUN_SETUP_GENTLY) {
+		else if ((p->option & RUN_SETUP_GENTLY) && !startup_info->have_repository) {
 			int nongit_ok;
 			setup_git_directory_gently(&nongit_ok);
 		}
+		else if (startup_info->have_repository) {
+			if (p->option & (RUN_SETUP_GENTLY | RUN_SETUP))
+				; /* done already */
+			else
+				unset_git_directory(startup_info->prefix);
+		}
 
 		if (use_pager == -1 && p->option & RUN_SETUP)
 			use_pager = check_pager_config(p->cmd);
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 19/19] Guard unallowed access to repository when it's not set up
  2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
                   ` (17 preceding siblings ...)
  2010-03-21 10:30 ` [PATCH v2 18/19] alias: keep repository found while collecting aliases as long as possible Nguyễn Thái Ngọc Duy
@ 2010-03-21 10:30 ` Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 20+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2010-03-21 10:30 UTC (permalink / raw
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Many code path will skip repo access if startup_info->have_repository
is false. This may be a fault if startup_info->have_repository has not
been properly initialized.

So the rule is one of the following commands must be run before any
repo access. And none of them can be called twice.

 - setup_git_directory*
 - enter_repo
 - init_db

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/init-db.c |    1 +
 cache.h           |    1 +
 config.c          |    2 ++
 environment.c     |   13 +++++++++++--
 git.c             |    3 +++
 setup.c           |   13 +++++++++++++
 6 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 064b919..d4c415c 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -302,6 +302,7 @@ int init_db(const char *git_dir, const char *template_dir, unsigned int flags)
 
 	set_git_dir(make_absolute_path(git_dir));
 	startup_info->have_repository = 1;
+	startup_info->have_run_setup_gitdir = 1;
 
 	safe_create_dir(get_git_dir(), 0);
 
diff --git a/cache.h b/cache.h
index b1ed150..417a744 100644
--- a/cache.h
+++ b/cache.h
@@ -1060,6 +1060,7 @@ int split_cmdline(char *cmdline, const char ***argv);
 /* git.c */
 struct startup_info {
 	const char *prefix;
+	int have_run_setup_gitdir;
 	int have_repository;
 	int help;
 };
diff --git a/config.c b/config.c
index 07d854a..9981b09 100644
--- a/config.c
+++ b/config.c
@@ -737,6 +737,8 @@ int git_config(config_fn_t fn, void *data)
 	char *repo_config = NULL;
 	int ret;
 
+	if (startup_info && !startup_info->have_run_setup_gitdir)
+		die("internal error: access to .git/config without repo setup");
 	if (!startup_info || startup_info->have_repository)
 		repo_config = git_pathdup("config");
 	ret = git_config_early(fn, data, repo_config);
diff --git a/environment.c b/environment.c
index 6127025..17f0cbe 100644
--- a/environment.c
+++ b/environment.c
@@ -98,9 +98,18 @@ void unset_git_env(void)
 
 static void setup_git_env(void)
 {
+	if (startup_info && startup_info->have_run_setup_gitdir)
+		die("internal error: setup_git_env can't be called twice");
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
-	if (!git_dir)
-		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+	if (!git_dir) {
+		/*
+		 * Repo detection should be done by setup_git_directory*
+		 * or enter_repo, not by this function
+		 */
+		 if (startup_info)
+			 die("internal error: $GIT_DIR is empty");
+		 git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+	}
 	if (!git_dir)
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 	git_object_dir = getenv(DB_ENVIRONMENT);
diff --git a/git.c b/git.c
index 88aaf13..5f7f410 100644
--- a/git.c
+++ b/git.c
@@ -260,6 +260,9 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 			use_pager = 1;
 		}
 	}
+	else
+		/* Stop git_config() from complaining that no repository found. */
+		startup_info->have_run_setup_gitdir = 1;
 	commit_pager_choice();
 
 	if (!startup_info->help && p->option & NEED_WORK_TREE)
diff --git a/setup.c b/setup.c
index 1808ebe..d9bb616 100644
--- a/setup.c
+++ b/setup.c
@@ -237,7 +237,17 @@ void setup_work_tree(void)
 		git_dir = make_absolute_path(git_dir);
 	if (!work_tree || chdir(work_tree))
 		die("This operation must be run in a work tree");
+
+	/*
+	 * have_run_setup_gitdir is unset in order to avoid die()ing
+	 * inside set_git_env(). We don't actually initialize
+	 * repo twice, we're just relative-izing gitdir
+	 */
+	if (startup_info)
+		startup_info->have_run_setup_gitdir = 0;
 	set_git_dir(make_relative_path(git_dir, work_tree));
+	if (startup_info)
+		startup_info->have_run_setup_gitdir = 1;
 	initialized = 1;
 }
 
@@ -340,6 +350,7 @@ void unset_git_directory(const char *prefix)
 			unset_git_env();
 		startup_info->prefix = NULL;
 		startup_info->have_repository = 0;
+		startup_info->have_run_setup_gitdir = 0;
 	}
 
 	/* Initialized in setup_git_directory_gently_1() */
@@ -506,6 +517,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	prefix = setup_git_directory_gently_1(nongit_ok);
 	if (startup_info) {
 		startup_info->prefix = prefix;
+		startup_info->have_run_setup_gitdir = 1;
 		startup_info->have_repository = !nongit_ok || !*nongit_ok;
 	}
 	return prefix;
@@ -600,6 +612,7 @@ char *enter_repo(char *path, int strict)
 		set_git_dir(".");
 		if (startup_info) {
 			startup_info->prefix = NULL;
+			startup_info->have_run_setup_gitdir = 1;
 			startup_info->have_repository = 1;
 		}
 		return path;
-- 
1.7.0.2.425.gb99f1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2010-03-21 10:36 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-21 10:30 [PATCH v2 00/19] nd/setup part two, second round Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 01/19] Move enter_repo() to setup.c Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 02/19] enter_repo(): initialize other variables as setup_git_directory_gently() does Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 03/19] rev-parse --git-dir: print relative gitdir correctly Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 04/19] worktree setup: call set_git_dir explicitly Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 05/19] Add git_config_early() Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 06/19] Use git_config_early() instead of git_config() during repo setup Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 07/19] worktree setup: restore original state when things go wrong Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 08/19] init/clone: turn on startup->have_repository properly Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 09/19] git_config(): do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 10/19] Do not read .git/info/exclude " Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 11/19] Do not read .git/info/attributes " Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 12/19] apply: do not check sha1 when repository has not been found Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 13/19] config: do not read .git/config if there is no repository Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 14/19] run_builtin(): save "-h" detection result for later use Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 15/19] builtins: utilize startup_info->help where possible Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 16/19] builtins: check for startup_info->help, print and exit early Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 17/19] Allow to undo setup_git_directory_gently() gracefully (and fix alias code) Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 18/19] alias: keep repository found while collecting aliases as long as possible Nguyễn Thái Ngọc Duy
2010-03-21 10:30 ` [PATCH v2 19/19] Guard unallowed access to repository when it's not set up Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).