threads / rfc / 7454

RFC patchintroduce GIT_WORK_TREE environment variable

Subject: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

## tl;dr

12 messages between Mar 28, 2007 and Apr 11, 2007. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Matthias Lederhofer· Mar 28, 2007, 14:15 UTC · lore

GIT_WORK_TREE can be used with GIT_DIR to specify the working tree. As for GIT_DIR there is also the option `git --work-tree=GIT_WORK_TREE` which overrides the environment variable and a config setting core.worktree which is used as default value.

setup_git_directory_gently() is rewritten and does the following now:

  find the repository directory ($GIT_DIR, ".git" in parent
  directories, ".")
  read the configuration (core.bare and core.worktree are used)
  if core.bare is not set assume the repository is not bare if a
  working tree is specified or guess based on the name of the
  repository directory
  for a bare repository:
    set GIT_DIR if it is not set already and stop (this wont change
    the directory even if the repository was found as .git in a
    parent directory)
  for a non-bare repository:
    if GIT_DIR is specified:
      use GIT_WORK_TREE, core.worktree or "." as working tree
    if the repository was found as .git in a parent directory:
      use the parent directory of the .git directory as working tree
    if the repository was found in ".":
      use "." as working tree
    set inside_git_dir and inside_working_tree based on getcwd() and
    prefixcmp()

is_bare_repository() is also changed to return true if the working directory is outside of the working tree.

Signed-off-by: Matthias Lederhofer <matled@gmx.net>
---
This patch implements the outline for setup_git_directory_gently()
from <20070318211836.GA12456@moooo.ath.cx>.
setup_git_directory_gently() reads the whole configuration and gives
it to git_default_config() too.  I can't think of any downsides to do
this, if this should not be done setup_git_directory_gently() could
also do a git_config_from_file(${GIT_CONFIG:-$GIT_DIR/.config}) and
only get the value of core.bare and core.worktree.
---
 Documentation/config.txt |    4 +
 Documentation/git.txt    |   17 ++++-
 cache.h                  |    2 +
 environment.c            |   11 +++-
 git.c                    |   12 +++-
 setup.c                  |  178 +++++++++++++++++++++++++++++++++++----------
 t/test-lib.sh            |    1 +
 7 files changed, 181 insertions(+), 44 deletions(-)
Show changes to 7 files +181 −44

Documentation/config.txt, Documentation/git.txt, cache.h, environment.c, git.c, setup.c, t/test-lib.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cf1e040..52b1aa9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -162,6 +162,10 @@ repository that ends in "/.git" is assumed to be not bare (bare =
 false), while all other repositories are assumed to be bare (bare
 = true).
 
+core.worktree::
+	Path to the working tree when GIT_DIR is set.  This can be
+	overriden by GIT_WORK_TREE.
+
 core.logAllRefUpdates::
 	Updates to a ref <ref> is logged to the file
 	"$GIT_DIR/logs/<ref>", by appending the new and old
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 31397dc..5ead10f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate]
-    [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]
+    [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
+    [--help] COMMAND [ARGS]
 
 DESCRIPTION
 -----------
@@ -83,6 +84,14 @@ OPTIONS
 	Set the path to the repository. This can also be controlled by
 	setting the GIT_DIR environment variable.
 
+--work-tree=<path>::
+	Set the path to the working tree.  The value will be used only
+	in combination with $GIT_DIR or '--git-dir'.  If GIT_DIR is
+	set but this option is not git will assume that the current
+	directory is also the working tree.
+	This can also be controlled by setting the GIT_WORK_TREE
+	environment variable.
+
 --bare::
 	Same as --git-dir=`pwd`.
 
@@ -327,6 +336,12 @@ git so take care if using Cogito etc.
 	specifies a path to use instead of the default `.git`
 	for the base of the repository.
 
+'GIT_WORK_TREE'::
+	Set the path to the working tree.  The value will be used only
+	in combination with $GIT_DIR or '--git-dir'.  If GIT_DIR is
+	set but this option is not git will assume that the current
+	directory is also the working tree.
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index 384b260..df47925 100644
--- a/cache.h
+++ b/cache.h
@@ -144,6 +144,7 @@ enum object_type {
 };
 
 #define GIT_DIR_ENVIRONMENT "GIT_DIR"
+#define GIT_WORKING_TREE_ENVIRONMENT "GIT_WORK_TREE"
 #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
 #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
 #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
@@ -164,6 +165,7 @@ extern char *get_graft_file(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
+extern int inside_working_tree;
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
diff --git a/environment.c b/environment.c
index 2231659..8db684a 100644
--- a/environment.c
+++ b/environment.c
@@ -60,8 +60,15 @@ static void setup_git_env(void)
 int is_bare_repository(void)
 {
 	const char *dir, *s;
-	if (0 <= is_bare_repository_cfg)
-		return is_bare_repository_cfg;
+	/* definitely bare */
+	if (is_bare_repository_cfg == 1)
+		return 1;
+	/* bare if cwd is outside of the working tree */
+	if (inside_working_tree >= 0)
+		return !inside_working_tree;
+	/* configuration says it is not bare */
+	if (is_bare_repository_cfg == 0)
+		return 0;
 
 	dir = get_git_dir();
 	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
diff --git a/git.c b/git.c
index 5b1bc2a..9c282cd 100644
--- a/git.c
+++ b/git.c
@@ -4,7 +4,7 @@
 #include "quote.h"
 
 const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]";
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
 static void prepend_to_path(const char *dir, int len)
 {
@@ -68,6 +68,16 @@ static int handle_options(const char*** argv, int* argc)
 			(*argc)--;
 		} else if (!prefixcmp(cmd, "--git-dir=")) {
 			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
+		} else if (!strcmp(cmd, "--work-tree")) {
+			if (*argc < 2) {
+				fprintf(stderr, "No directory given for --work-tree.\n" );
+				usage(git_usage_string);
+			}
+			setenv(GIT_WORKING_TREE_ENVIRONMENT, (*argv)[1], 1);
+			(*argv)++;
+			(*argc)--;
+		} else if (!prefixcmp(cmd, "--work-tree=")) {
+			setenv(GIT_WORKING_TREE_ENVIRONMENT, cmd + 11, 1);
 		} else if (!strcmp(cmd, "--bare")) {
 			static char git_dir[PATH_MAX+1];
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 1);
diff --git a/setup.c b/setup.c
index a45ea83..ddbe852 100644
--- a/setup.c
+++ b/setup.c
@@ -192,67 +192,165 @@ int is_inside_git_dir(void)
 	return inside_git_dir;
 }
 
+static char *git_work_tree;
+
+static int git_setup_config(const char *var, const char *value)
+{
+	if (git_work_tree && !strcmp(var, "core.worktree")) {
+		strlcpy(git_work_tree, value, PATH_MAX);
+	}
+	return git_default_config(var, value);
+}
+
+int inside_working_tree = -1;
+
 const char *setup_git_directory_gently(int *nongit_ok)
 {
 	static char cwd[PATH_MAX+1];
+	char worktree[PATH_MAX+1], gitdir[PATH_MAX+1];
 	const char *gitdirenv;
-	int len, offset;
+	const char *gitwt = NULL;
+	int len;
+	int dotgit = 0;
+	int wt_rel_cwd = 0;
 
-	/*
-	 * If GIT_DIR is set explicitly, we're not going
-	 * to do any discovery, but we still do repository
-	 * validation.
-	 */
 	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
 	if (gitdirenv) {
 		if (PATH_MAX - 40 < strlen(gitdirenv))
 			die("'$%s' too big", GIT_DIR_ENVIRONMENT);
-		if (is_git_directory(gitdirenv))
-			return NULL;
-		if (nongit_ok) {
-			*nongit_ok = 1;
-			return NULL;
+		if (!is_git_directory(gitdirenv)) {
+			if (nongit_ok) {
+				*nongit_ok = 1;
+				return NULL;
+			}
+			die("Not a git repository: '%s'", gitdirenv);
 		}
-		die("Not a git repository: '%s'", gitdirenv);
-	}
-
-	if (!getcwd(cwd, sizeof(cwd)-1) || cwd[0] != '/')
-		die("Unable to read current working directory");
-
-	offset = len = strlen(cwd);
-	for (;;) {
-		if (is_git_directory(".git"))
-			break;
-		chdir("..");
-		do {
-			if (!offset) {
-				if (is_git_directory(cwd)) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
-					setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
-					inside_git_dir = 1;
-					return NULL;
+	} else {
+		int offset;
+		/* keep 5 bytes for /.git and 40 for files in .git directory */
+		if (!getcwd(cwd, sizeof(cwd) - 5 - 40) || cwd[0] != '/')
+			die("Unable to read current working directory");
+		offset = strlen(cwd);
+		for (;;) {
+			strcat(cwd, "/.git");
+			if (is_git_directory(cwd)) {
+				dotgit = 1;
+				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
+				cwd[offset] = '\0';
+				strcpy(worktree, cwd);
+				gitwt = worktree;
+				break;
+			}
+			if (offset == 0) {
+				if (is_git_directory(".")) {
+					setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+					strcpy(worktree, ".");
+					gitwt = worktree;
+					break;
 				}
 				if (nongit_ok) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
 					*nongit_ok = 1;
 					return NULL;
 				}
 				die("Not a git repository");
 			}
-		} while (cwd[--offset] != '/');
+			cwd[offset] = '\0';
+			while (cwd[offset] != '/')
+				--offset;
+			cwd[offset] = '\0';
+		}
+		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
+	}
+
+	if (!gitwt)
+		gitwt = getenv(GIT_WORKING_TREE_ENVIRONMENT);
+	if (gitwt && gitwt[0] != '/')
+		wt_rel_cwd = 1;
+	if (!gitwt) {
+		git_work_tree = worktree;
+		worktree[0] = '\0';
+	}
+	git_config(git_setup_config);
+	if (!gitwt) {
+		git_work_tree = NULL;
+		if (worktree[0])
+			gitwt = worktree;
 	}
 
-	if (offset == len)
+	/*
+	 * try to figure out if this is a bare repository:
+	 * if core.bare is undefined and a worktree is specified we go on and
+	 * try the specified worktree, otherwise use is_bare_repository() to
+	 * decide
+	 */
+	if (!(is_bare_repository_cfg == -1 && gitwt) && is_bare_repository())
+		return NULL;
+
+	if (!gitwt) {
+		wt_rel_cwd = 1;
+		strcpy(worktree, ".");
+		gitwt = worktree;
+	}
+
+	/* run getcwd in cwd, GIT_DIR and GIT_WORK_TREE */
+	if (!getcwd(cwd, sizeof(cwd)-1) || cwd[0] != '/')
+		die("Unable to read current working directory");
+	if (chdir(gitdirenv))
+		die("Cannot change directory to '%s'", gitdirenv);
+	if (!getcwd(gitdir, sizeof(gitdir)-1) || gitdir[0] != '/')
+		die("Unable to read current working directory");
+	/* gitwt is a relative path from the old cwd, go back first */
+	if (wt_rel_cwd && chdir(cwd))
+		die("Cannot come back to cwd");
+	if (chdir(gitwt)) {
+		if (wt_rel_cwd || gitwt[0] == '/')
+			die("Cannot change directory to working tree '%s'",
+				gitwt);
+		else
+			die("Cannot change directory to working tree '%s'"
+				" from $GIT_DIR", gitwt);
+	}
+	if (!getcwd(worktree, sizeof(worktree)-1) || worktree[0] != '/')
+		die("Unable to read current working directory");
+
+	len = strlen(cwd);
+	cwd[len] = '/';
+	cwd[len+1] = '\0';
+
+	len = strlen(worktree);
+	worktree[len] = '/';
+	worktree[len+1] = '\0';
+	inside_working_tree = !prefixcmp(cwd, worktree);
+
+	len = strlen(gitdir);
+	gitdir[len] = '/';
+	gitdir[len+1] = '\0';
+	/*
+	 * if we are inside worktree, worktree is below gitdir and
+	 * worktree != gitdir then we are not really in gitdir
+	 */
+	if (inside_working_tree && !prefixcmp(worktree, gitdir) &&
+	    strcmp(worktree, gitdir)) {
+		inside_git_dir = 0;
+	} else {
+		inside_git_dir = !prefixcmp(cwd, gitdir);
+	}
+	gitdir[len] = '\0';
+
+	if (!inside_working_tree) {
+		if (chdir(cwd))
+			die("Cannot come back to cwd");
 		return NULL;
+	}
 
-	/* Make "offset" point to past the '/', and add a '/' at the end */
-	offset++;
-	cwd[len++] = '/';
-	cwd[len] = 0;
-	inside_git_dir = !prefixcmp(cwd + offset, ".git/");
-	return cwd + offset;
+	if (dotgit)
+		unsetenv(GIT_DIR_ENVIRONMENT);
+	else if (gitdirenv[0] != '/')
+		setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
+
+	if (!strcmp(cwd, worktree))
+		return NULL;
+	return cwd+strlen(worktree);
 }
 
 int git_config_perm(const char *var, const char *value)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c075474..77c6d23 100755
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -25,6 +25,7 @@ GIT_COMMITTER_EMAIL=committer@example.com
 GIT_COMMITTER_NAME='C O Mitter'
 unset GIT_DIFF_OPTS
 unset GIT_DIR
+unset GIT_WORK_TREE
 unset GIT_EXTERNAL_DIFF
 unset GIT_INDEX_FILE
 unset GIT_OBJECT_DIRECTORY
-- 
1.5.1.rc2.621.g1fee7
Johannes Sixt· Mar 28, 2007, 15:45 UTC · re: Matthias Lederhofer · lore

Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

Matthias Lederhofer wrote:
Show 12 quoted lines
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index c075474..77c6d23 100755
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -25,6 +25,7 @@ GIT_COMMITTER_EMAIL=committer@example.com
>  GIT_COMMITTER_NAME='C O Mitter'
>  unset GIT_DIFF_OPTS
>  unset GIT_DIR
> +unset GIT_WORK_TREE
>  unset GIT_EXTERNAL_DIFF
>  unset GIT_INDEX_FILE
>  unset GIT_OBJECT_DIRECTORY

connect.c:git_connect() unsets some environment variables, most prominently GIT_DIR; I think it should also unset GIT_WORK_TREE.

-- Hannes
Matthias Lederhofer· Mar 29, 2007, 08:22 UTC · re: Matthias Lederhofer · lore

Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

Matthias Lederhofer <matled@gmx.net> wrote:
> +	git_config(git_setup_config);
When $GIT_DIR was not set and the repository is found as a .git
directory $GIT_DIR will be the full path to the .git directory when
calling git_config().  git_config() calls git_path() which calls
get_git_dir() which calls getenv("GIT_DIR").
I'm not sure this is defined behaviour at all:
    
    char *foo = getenv("FOO");
    unsetenv("FOO");

Does foo still point to the old content of the FOO environment variable? If it does get_git_dir() will always return the full path to the repository directory.

I can think of two ways to solve this (in case this is a problem):
1. Add a function to environment.c which will cause setup_git_env to
   be called again (either directly or by setting all the pointers to
   NULL again).
2. Use git_config_from_file() instead of git_config().  This will
   probably duplicate code from git_config().
Dana How· Apr 4, 2007, 16:59 UTC · re: Matthias Lederhofer · lore

Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

On 4/4/07, Matthias Lederhofer <matled@gmx.net> wrote:
> As 1.5.1 is released now: any comments on this patch?

Ditto for "Handling large repositories". I guess I should resubmit it against 1.5.1.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell
Junio C Hamano· Apr 4, 2007, 18:45 UTC · re: Matthias Lederhofer · lore

Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

Matthias Lederhofer <matled@gmx.net> writes:
> As 1.5.1 is released now: any comments on this patch?

My impression was that you already received a couple responses which were not yet answered.

Matthias Lederhofer· Apr 4, 2007, 21:04 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable

Junio C Hamano <junkio@cox.net> wrote:
> My impression was that you already received a couple responses
> which were not yet answered.

I just fixed the two things noted in reply to the patch. I was waiting for feedback if it is desired to read the whole configuration from setup_git_directory_gently() and perhaps a comment on the rewrite of it.

Matthias Lederhofer· Apr 4, 2007, 20:09 UTC · re: Matthias Lederhofer · lore

[PATCH] export setup_git_env()

After changing environment variables used by environment.c we need to call getenv() again. setup_git_env() does this and can now be used from other files too.

Signed-off-by: Matthias Lederhofer <matled@gmx.net>
---
 cache.h       |    1 +
 environment.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)
Show changes to 2 files +2 −1

cache.h, environment.c

diff --git a/cache.h b/cache.h
index 384b260..7b49258 100644
--- a/cache.h
+++ b/cache.h
@@ -154,6 +154,7 @@ enum object_type {
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
 
 extern int is_bare_repository_cfg;
+extern void setup_git_env(void);
 extern int is_bare_repository(void);
 extern int is_inside_git_dir(void);
 extern const char *get_git_dir(void);
diff --git a/environment.c b/environment.c
index 2231659..713a011 100644
--- a/environment.c
+++ b/environment.c
@@ -35,7 +35,7 @@ int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
-static void setup_git_env(void)
+void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
 	if (!git_dir)
-- 
1.5.1.4.g446af-dirty
Matthias Lederhofer· Apr 4, 2007, 20:13 UTC · re: Matthias Lederhofer · lore

[PATCH(amend)] introduce GIT_WORK_TREE environment variable

GIT_WORK_TREE can be used with GIT_DIR to specify the working tree. As for GIT_DIR there is also the option `git --work-tree=GIT_WORK_TREE` which overrides the environment variable and a config setting core.worktree which is used as default value.

setup_git_directory_gently() is rewritten and does the following now:

  find the repository directory ($GIT_DIR, ".git" in parent
  directories, ".")
  read the configuration (core.bare and core.worktree are used)
  if core.bare is not set assume the repository is not bare if a
  working tree is specified or guess based on the name of the
  repository directory
  for a bare repository:
    set GIT_DIR if it is not set already and stop (this wont change
    the directory even if the repository was found as .git in a
    parent directory)
  for a non-bare repository:
    if GIT_DIR is specified:
      use GIT_WORK_TREE, core.worktree or "." as working tree
    if the repository was found as .git in a parent directory:
      use the parent directory of the .git directory as working tree
    if the repository was found in ".":
      use "." as working tree
    set inside_git_dir and inside_working_tree based on getcwd() and
    prefixcmp()

is_bare_repository() is also changed to return true if the working directory is outside of the working tree.

Signed-off-by: Matthias Lederhofer <matled@gmx.net>
---
Changes:
    - unsetenv(GIT_WORKING_TREE_ENVIRONMENT) in git_connect(), noted
      by Johannes Sixt
      I also looked at the other places using GIT_DIR_ENVIRONMENT to
      see if I missed something similar
    - call setup_git_env() from setup_git_directory_gently() if needed
---
 Documentation/config.txt |    4 +
 Documentation/git.txt    |   17 ++++-
 cache.h                  |    2 +
 connect.c                |    1 +
 environment.c            |   11 +++-
 git.c                    |   12 +++-
 setup.c                  |  181 ++++++++++++++++++++++++++++++++++++----------
 t/test-lib.sh            |    1 +
 8 files changed, 185 insertions(+), 44 deletions(-)
Show changes to 8 files +185 −44

Documentation/config.txt, Documentation/git.txt, cache.h, connect.c, environment.c, git.c, setup.c, t/test-lib.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cf1e040..52b1aa9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -162,6 +162,10 @@ repository that ends in "/.git" is assumed to be not bare (bare =
 false), while all other repositories are assumed to be bare (bare
 = true).
 
+core.worktree::
+	Path to the working tree when GIT_DIR is set.  This can be
+	overriden by GIT_WORK_TREE.
+
 core.logAllRefUpdates::
 	Updates to a ref <ref> is logged to the file
 	"$GIT_DIR/logs/<ref>", by appending the new and old
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9defc33..b37f9fd 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate]
-    [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]
+    [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]
+    [--help] COMMAND [ARGS]
 
 DESCRIPTION
 -----------
@@ -89,6 +90,14 @@ OPTIONS
 	Set the path to the repository. This can also be controlled by
 	setting the GIT_DIR environment variable.
 
+--work-tree=<path>::
+	Set the path to the working tree.  The value will be used only
+	in combination with $GIT_DIR or '--git-dir'.  If GIT_DIR is
+	set but this option is not git will assume that the current
+	directory is also the working tree.
+	This can also be controlled by setting the GIT_WORK_TREE
+	environment variable.
+
 --bare::
 	Same as --git-dir=`pwd`.
 
@@ -333,6 +342,12 @@ git so take care if using Cogito etc.
 	specifies a path to use instead of the default `.git`
 	for the base of the repository.
 
+'GIT_WORK_TREE'::
+	Set the path to the working tree.  The value will be used only
+	in combination with $GIT_DIR or '--git-dir'.  If GIT_DIR is
+	set but this option is not git will assume that the current
+	directory is also the working tree.
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index 7b49258..e163a76 100644
--- a/cache.h
+++ b/cache.h
@@ -144,6 +144,7 @@ enum object_type {
 };
 
 #define GIT_DIR_ENVIRONMENT "GIT_DIR"
+#define GIT_WORKING_TREE_ENVIRONMENT "GIT_WORK_TREE"
 #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
 #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
 #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
@@ -165,6 +166,7 @@ extern char *get_graft_file(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
+extern int inside_working_tree;
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
diff --git a/connect.c b/connect.c
index da89c9c..ff2c459 100644
--- a/connect.c
+++ b/connect.c
@@ -773,6 +773,7 @@ pid_t git_connect(int fd[2], char *url, const char *prog)
 			unsetenv(ALTERNATE_DB_ENVIRONMENT);
 			unsetenv(DB_ENVIRONMENT);
 			unsetenv(GIT_DIR_ENVIRONMENT);
+			unsetenv(GIT_WORKING_TREE_ENVIRONMENT);
 			unsetenv(GRAFT_ENVIRONMENT);
 			unsetenv(INDEX_ENVIRONMENT);
 			execlp("sh", "sh", "-c", command, NULL);
diff --git a/environment.c b/environment.c
index 713a011..769d409 100644
--- a/environment.c
+++ b/environment.c
@@ -60,8 +60,15 @@ void setup_git_env(void)
 int is_bare_repository(void)
 {
 	const char *dir, *s;
-	if (0 <= is_bare_repository_cfg)
-		return is_bare_repository_cfg;
+	/* definitely bare */
+	if (is_bare_repository_cfg == 1)
+		return 1;
+	/* bare if cwd is outside of the working tree */
+	if (inside_working_tree >= 0)
+		return !inside_working_tree;
+	/* configuration says it is not bare */
+	if (is_bare_repository_cfg == 0)
+		return 0;
 
 	dir = get_git_dir();
 	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
diff --git a/git.c b/git.c
index 5b1bc2a..9c282cd 100644
--- a/git.c
+++ b/git.c
@@ -4,7 +4,7 @@
 #include "quote.h"
 
 const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]";
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
 static void prepend_to_path(const char *dir, int len)
 {
@@ -68,6 +68,16 @@ static int handle_options(const char*** argv, int* argc)
 			(*argc)--;
 		} else if (!prefixcmp(cmd, "--git-dir=")) {
 			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
+		} else if (!strcmp(cmd, "--work-tree")) {
+			if (*argc < 2) {
+				fprintf(stderr, "No directory given for --work-tree.\n" );
+				usage(git_usage_string);
+			}
+			setenv(GIT_WORKING_TREE_ENVIRONMENT, (*argv)[1], 1);
+			(*argv)++;
+			(*argc)--;
+		} else if (!prefixcmp(cmd, "--work-tree=")) {
+			setenv(GIT_WORKING_TREE_ENVIRONMENT, cmd + 11, 1);
 		} else if (!strcmp(cmd, "--bare")) {
 			static char git_dir[PATH_MAX+1];
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 1);
diff --git a/setup.c b/setup.c
index a45ea83..794edcf 100644
--- a/setup.c
+++ b/setup.c
@@ -192,67 +192,168 @@ int is_inside_git_dir(void)
 	return inside_git_dir;
 }
 
+static char *git_work_tree;
+
+static int git_setup_config(const char *var, const char *value)
+{
+	if (git_work_tree && !strcmp(var, "core.worktree")) {
+		strlcpy(git_work_tree, value, PATH_MAX);
+	}
+	return git_default_config(var, value);
+}
+
+int inside_working_tree = -1;
+
 const char *setup_git_directory_gently(int *nongit_ok)
 {
 	static char cwd[PATH_MAX+1];
+	char worktree[PATH_MAX+1], gitdir[PATH_MAX+1];
 	const char *gitdirenv;
-	int len, offset;
+	const char *gitwt = NULL;
+	int len;
+	int dotgit = 0;
+	int wt_rel_cwd = 0;
 
-	/*
-	 * If GIT_DIR is set explicitly, we're not going
-	 * to do any discovery, but we still do repository
-	 * validation.
-	 */
 	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
 	if (gitdirenv) {
 		if (PATH_MAX - 40 < strlen(gitdirenv))
 			die("'$%s' too big", GIT_DIR_ENVIRONMENT);
-		if (is_git_directory(gitdirenv))
-			return NULL;
-		if (nongit_ok) {
-			*nongit_ok = 1;
-			return NULL;
+		if (!is_git_directory(gitdirenv)) {
+			if (nongit_ok) {
+				*nongit_ok = 1;
+				return NULL;
+			}
+			die("Not a git repository: '%s'", gitdirenv);
 		}
-		die("Not a git repository: '%s'", gitdirenv);
-	}
-
-	if (!getcwd(cwd, sizeof(cwd)-1) || cwd[0] != '/')
-		die("Unable to read current working directory");
-
-	offset = len = strlen(cwd);
-	for (;;) {
-		if (is_git_directory(".git"))
-			break;
-		chdir("..");
-		do {
-			if (!offset) {
-				if (is_git_directory(cwd)) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
-					setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
-					inside_git_dir = 1;
-					return NULL;
+	} else {
+		int offset;
+		/* keep 5 bytes for /.git and 40 for files in .git directory */
+		if (!getcwd(cwd, sizeof(cwd) - 5 - 40) || cwd[0] != '/')
+			die("Unable to read current working directory");
+		offset = strlen(cwd);
+		for (;;) {
+			strcat(cwd, "/.git");
+			if (is_git_directory(cwd)) {
+				dotgit = 1;
+				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
+				cwd[offset] = '\0';
+				strcpy(worktree, cwd);
+				gitwt = worktree;
+				break;
+			}
+			if (offset == 0) {
+				if (is_git_directory(".")) {
+					setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+					strcpy(worktree, ".");
+					gitwt = worktree;
+					break;
 				}
 				if (nongit_ok) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
 					*nongit_ok = 1;
 					return NULL;
 				}
 				die("Not a git repository");
 			}
-		} while (cwd[--offset] != '/');
+			cwd[offset] = '\0';
+			while (cwd[offset] != '/')
+				--offset;
+			cwd[offset] = '\0';
+		}
+		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
+	}
+
+	if (!gitwt)
+		gitwt = getenv(GIT_WORKING_TREE_ENVIRONMENT);
+	if (gitwt && gitwt[0] != '/')
+		wt_rel_cwd = 1;
+	if (!gitwt) {
+		git_work_tree = worktree;
+		worktree[0] = '\0';
+	}
+	git_config(git_setup_config);
+	if (!gitwt) {
+		git_work_tree = NULL;
+		if (worktree[0])
+			gitwt = worktree;
 	}
 
-	if (offset == len)
+	/*
+	 * try to figure out if this is a bare repository:
+	 * if core.bare is undefined and a worktree is specified we go on and
+	 * try the specified worktree, otherwise use is_bare_repository() to
+	 * decide
+	 */
+	if (!(is_bare_repository_cfg == -1 && gitwt) && is_bare_repository())
 		return NULL;
 
-	/* Make "offset" point to past the '/', and add a '/' at the end */
-	offset++;
-	cwd[len++] = '/';
-	cwd[len] = 0;
-	inside_git_dir = !prefixcmp(cwd + offset, ".git/");
-	return cwd + offset;
+	if (!gitwt) {
+		wt_rel_cwd = 1;
+		strcpy(worktree, ".");
+		gitwt = worktree;
+	}
+
+	/* run getcwd in cwd, GIT_DIR and GIT_WORK_TREE */
+	if (!getcwd(cwd, sizeof(cwd)-1) || cwd[0] != '/')
+		die("Unable to read current working directory");
+	if (chdir(gitdirenv))
+		die("Cannot change directory to '%s'", gitdirenv);
+	if (!getcwd(gitdir, sizeof(gitdir)-1) || gitdir[0] != '/')
+		die("Unable to read current working directory");
+	/* gitwt is a relative path from the old cwd, go back first */
+	if (wt_rel_cwd && chdir(cwd))
+		die("Cannot come back to cwd");
+	if (chdir(gitwt)) {
+		if (wt_rel_cwd || gitwt[0] == '/')
+			die("Cannot change directory to working tree '%s'",
+				gitwt);
+		else
+			die("Cannot change directory to working tree '%s'"
+				" from $GIT_DIR", gitwt);
+	}
+	if (!getcwd(worktree, sizeof(worktree)-1) || worktree[0] != '/')
+		die("Unable to read current working directory");
+
+	len = strlen(cwd);
+	cwd[len] = '/';
+	cwd[len+1] = '\0';
+
+	len = strlen(worktree);
+	worktree[len] = '/';
+	worktree[len+1] = '\0';
+	inside_working_tree = !prefixcmp(cwd, worktree);
+
+	len = strlen(gitdir);
+	gitdir[len] = '/';
+	gitdir[len+1] = '\0';
+	/*
+	 * if we are inside worktree, worktree is below gitdir and
+	 * worktree != gitdir then we are not really in gitdir
+	 */
+	if (inside_working_tree && !prefixcmp(worktree, gitdir) &&
+	    strcmp(worktree, gitdir)) {
+		inside_git_dir = 0;
+	} else {
+		inside_git_dir = !prefixcmp(cwd, gitdir);
+	}
+	gitdir[len] = '\0';
+
+	if (!inside_working_tree) {
+		if (chdir(cwd))
+			die("Cannot come back to cwd");
+		return NULL;
+	}
+
+	if (dotgit) {
+		unsetenv(GIT_DIR_ENVIRONMENT);
+		setup_git_env();
+	} else if (gitdirenv[0] != '/') {
+		setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
+		setup_git_env();
+	}
+
+	if (!strcmp(cwd, worktree))
+		return NULL;
+	return cwd+strlen(worktree);
 }
 
 int git_config_perm(const char *var, const char *value)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c075474..77c6d23 100755
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -25,6 +25,7 @@ GIT_COMMITTER_EMAIL=committer@example.com
 GIT_COMMITTER_NAME='C O Mitter'
 unset GIT_DIFF_OPTS
 unset GIT_DIR
+unset GIT_WORK_TREE
 unset GIT_EXTERNAL_DIFF
 unset GIT_INDEX_FILE
 unset GIT_OBJECT_DIRECTORY
-- 
1.5.1.4.g446af-dirty
Junio C Hamano· Apr 4, 2007, 22:34 UTC · re: Matthias Lederhofer · lore

Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable

Matthias Lederhofer <matled@gmx.net> writes:
Show 5 quoted lines
> GIT_WORK_TREE can be used with GIT_DIR to specify the working tree.
> As for GIT_DIR there is also the option `git
> --work-tree=GIT_WORK_TREE` which overrides the environment
> variable and a config setting core.worktree which is used as
> default value.

I haven't followed the latest code to see if it does what it claims to in the log message (although I looked at the last round briefly).

Most of my comments below are not objections but questions.
Show 11 quoted lines
> setup_git_directory_gently() is rewritten and does the
> following now:
>
>   find the repository directory ($GIT_DIR, ".git" in parent
>   directories, ".")
>
>   read the configuration (core.bare and core.worktree are used)
>
>   if core.bare is not set assume the repository is not bare if a
>   working tree is specified or guess based on the name of the
>   repository directory
Sounds sane so far.
>   for a bare repository:
>     set GIT_DIR if it is not set already and stop (this wont change
>     the directory even if the repository was found as .git in a
>     parent directory)

If you have a normal non-bare layout repository and have core.bare set (perhaps by mistake), currently we chdir(2) up to the working tree root, but the new code doesn't. Is it a problem in practice?

If you have a truly bare repository (perhaps a one that is serving the general public over gitweb), and you use GIT_DIR and GIT_WORK_TREE to have a working tree elsewhere so that you can hack on it, running a git command from a subdirectory would not chdir(2) up to the root of the working tree. I suspect that the callers of setup_git_directory() would expect to see the same "cd up to the root and return the prefix" behaviour. Is this a problem in practice?

Show 5 quoted lines
>   for a non-bare repository:
>     if GIT_DIR is specified:
>       use GIT_WORK_TREE, core.worktree or "." as working tree
>     if the repository was found as .git in a parent directory:
>       use the parent directory of the .git directory as working tree
Sounds sane so far.
>     if the repository was found in ".":
>       use "." as working tree
I am not sure about this.

Is there ever a case that the repository (i.e. the directory that has objects/, refs/, and HEAD) can also be a sane working tree root? Wouldn't it be saner to say this case is without any working tree and fail commands that require a working tree?

>     set inside_git_dir and inside_working_tree based on getcwd() and
>     prefixcmp()
> is_bare_repository() is also changed to return true if the working
> directory is outside of the working tree.

What does this mean from operational perspective? Suppose you are using GIT_DIR and GIT_WORK_TREE to have a working tree that is separate from your working area, and get interrupted and go somewhere else (say "pushd /var/tmp"). Does this suddenly allow "git fetch" into the repository to update the current branch tip? That does not sound right, but it might not be a good use case to begin with. I dunno, but I think is_bare_repository should mean "I am treating this repository as a bare repository". That means I do not want to have "no-working-tree" semantics applied to the repository operation, regardless of where my $cwd happens to be, once I say GIT_WORK_TREE to name which working tree I am using to work with that repository.

What problem are you solving with this is_bare_repository() thing? Could it be that you are working around problems with the current callers that behave inappropriately, based on is_bare_repository(), when they should really be checking inside_work_tree() instead, perhaps?

>     - call setup_git_env() from setup_git_directory_gently() if needed

Calling setup_git_env() again would leak memory for git_object_dir and friends, but I have a bigger worry about this change.

If calling setup_git_env() explicitly after resetting GIT_DIR and stuff in this code makes any difference, doesn't that mean somebody else already called a function in environment.c (say, get_refs_directory()) and also have already _acted_ on it (e.g. calling get_packed_refs() which populates the list of refs based on the old value of "$GIT_DIR/packed-refs")? Calling the function again would not undo/redo that, so maybe the calling sequence into setup_git_env() needs to be made safer.

I think the only thing you care about in your "where is the repo and where is the worktree" codepath are get_git_dir() and is_bare_repository(). As a side effect of calling these functions for your own purpose, you later have to call setup_git_env() again to clean up, which is fine. I would feel better if there is an assert in setup_git_env that catches the case where it is called for the second time even though the first caller was something other than this repository/worktree discovery code.

Show 22 quoted lines
> diff --git a/environment.c b/environment.c
> index 713a011..769d409 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -60,8 +60,15 @@ void setup_git_env(void)
>  int is_bare_repository(void)
>  {
>  	const char *dir, *s;
> -	if (0 <= is_bare_repository_cfg)
> -		return is_bare_repository_cfg;
> +	/* definitely bare */
> +	if (is_bare_repository_cfg == 1)
> +		return 1;
> +	/* bare if cwd is outside of the working tree */
> +	if (inside_working_tree >= 0)
> +		return !inside_working_tree;
> +	/* configuration says it is not bare */
> +	if (is_bare_repository_cfg == 0)
> +		return 0;
>  
>  	dir = get_git_dir();
>  	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))

For example, calling this function from programs before calling the repository/worktree discovery is an error, isn't it? Can we have a safety here to catch such a programming error?

Show 18 quoted lines
> diff --git a/setup.c b/setup.c
> index a45ea83..794edcf 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -192,67 +192,168 @@ int is_inside_git_dir(void)
>  	return inside_git_dir;
>  }
>  
> +static char *git_work_tree;
> +
> +static int git_setup_config(const char *var, const char *value)
> +{
> +	if (git_work_tree && !strcmp(var, "core.worktree")) {
> +		strlcpy(git_work_tree, value, PATH_MAX);
> +	}
> +	return git_default_config(var, value);
> +}
> +

Other config functions do not pass already handled variables to git_default_config(). You probably would care about core.bare in your own code, so falling back to git_default_config() is fine, although it may look wasteful.

Matthias Lederhofer· Apr 6, 2007, 13:21 UTC · re: Junio C Hamano · lore

Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable

Junio C Hamano <junkio@cox.net> wrote:
Show 23 quoted lines
> Matthias Lederhofer <matled@gmx.net> writes:
> 
> > is_bare_repository() is also changed to return true if the working
> > directory is outside of the working tree.
> 
> What does this mean from operational perspective?  Suppose you
> are using GIT_DIR and GIT_WORK_TREE to have a working tree that
> is separate from your working area, and get interrupted and go
> somewhere else (say "pushd /var/tmp").  Does this suddenly allow
> "git fetch" into the repository to update the current branch
> tip?  That does not sound right, but it might not be a good use
> case to begin with.  I dunno, but I think is_bare_repository
> should mean "I am treating this repository as a bare
> repository".  That means I do not want to have "no-working-tree"
> semantics applied to the repository operation, regardless of
> where my $cwd happens to be, once I say GIT_WORK_TREE to name
> which working tree I am using to work with that repository.
> 
> What problem are you solving with this is_bare_repository()
> thing?  Could it be that you are working around problems with
> the current callers that behave inappropriately, based on
> is_bare_repository(), when they should really be checking
> inside_work_tree() instead, perhaps?

Right, I am using is_bare_repository() as check for "has this repository a usable working tree".

Currently:
    The core.bare setting is ignored during setup which leads to
    things like that:
        
        % git status
        fatal: git-status cannot be used without a working tree.
        % git runstatus
        # On branch master
        [..]
    So one command honors core.bare and another doesn't because even
    though runstatus has the NOT_BARE flag handle_internal_command()
    has not read the core.bare setting and just uses name based
    guessing.
    is_bare_repository() is used for two different checks at the
    moment:
    - should the user be protected from doing things with strange
      effects in repositories with a working tree (e.g. git fetch
      updating HEAD)
    - is there a working tree to be used (e.g. git add)
My patch:
    core.bare = true means that the repository may never be used with
    a working tree.
    is_bare_repository() checks for a usable working tree, this means
    it may return true for non-bare repositories when the cwd is not
    inside the working tree.  This allows a fetch to update HEAD when
    you're outside the working tree.
So the answer (you seem to suggest) to this problem is to split
is_bare_repository() into two functions, which sounds sane to me:
is_bare_repository():
    Check if it is allowed to do things which might cause problems if
    there is someone using this repository with a working tree.  This
    does not mean that there is a working tree available to use.
inside_work_tree():
    Check if there is a usable working tree (cwd is below the working
    tree).
With splitting is_bare_repository() it is no longer abused to check
for a working tree.  This raises the question what 'bare' is actually
supposed to mean:
(1) git may do operations on the repository which have strange effects
    if I'm using the repository with a working tree (e.g. update HEAD
    during a fetch)
(2) git may do (1) but also disallows to use the repository with a
working tree

With (1) it would be ok to let a user work with a repository that has core.bare = true if she really wants to (e.g. by setting GIT_WORK_TREE explicitly, details to be figured out later). With (2) git should disallow the use of a working tree if core.bare = true.

One option to solve this is to make core.bare have three possible
values, so the user can decide if he wants (1) or (2).
    core.bare = true: no working tree allowed
    core.bare = false: don't allow git-fetch to update HEAD etc.
    core.bare = mixed: allow both
In the third case is_bare_repository() would be true and
inside_work_tree() would change depending on cwd and GIT_WORK_TREE
setting.  This would allow git fetch to update the current branch even
while inside the working tree.
But I'm not really sure about this yet, any comments?
Show 9 quoted lines
> >   for a bare repository:
> >     set GIT_DIR if it is not set already and stop (this wont change
> >     the directory even if the repository was found as .git in a
> >     parent directory)
> 
> If you have a normal non-bare layout repository and have
> core.bare set (perhaps by mistake), currently we chdir(2) up to
> the working tree root, but the new code doesn't.  Is it a
> problem in practice?
If you refer to the user: see above.

For git itself: The caller can only note a difference if it tries to figure out if the repository is bare or not before calling setup_gdg (setup_git_directory_gently) or using something else than is_bare_repository(). After calling setup_gdg everything is the same as for any other bare repository with the current code: GIT_DIR is exported and cwd has not been changed.

Show 8 quoted lines
> If you have a truly bare repository (perhaps a one that is
> serving the general public over gitweb), and you use GIT_DIR and
> GIT_WORK_TREE to have a working tree elsewhere so that you can
> hack on it, running a git command from a subdirectory would not
> chdir(2) up to the root of the working tree.  I suspect that the
> callers of setup_git_directory() would expect to see the same
> "cd up to the root and return the prefix" behaviour.  Is this a
> problem in practice?

Callers of setup_git_directory() should not check if GIT_WORK_TREE is set and if the cwd is below this directory (which is required to use the tree) but use is_bare_repository() and inside_work_tree(). The former should return true, the latter should return false, so the caller clearly knows that there is no working tree to use and therefore there is especially no prefix.

Show 9 quoted lines
> >     if the repository was found in ".":
> >       use "." as working tree
> 
> I am not sure about this.
> 
> Is there ever a case that the repository (i.e. the directory
> that has objects/, refs/, and HEAD) can also be a sane working
> tree root?  Wouldn't it be saner to say this case is without any
> working tree and fail commands that require a working tree?

Right, I can't think of any case where this helps. I think I tried to follow the old code but missed that if the directory is named .git it is found as .git directory in .. and if it is named otherwise it is bare because its name is not ending in /.git.

Show 24 quoted lines
> >     - call setup_git_env() from setup_git_directory_gently() if needed
> 
> Calling setup_git_env() again would leak memory for
> git_object_dir and friends, but I have a bigger worry about this
> change.
> 
> If calling setup_git_env() explicitly after resetting GIT_DIR
> and stuff in this code makes any difference, doesn't that mean
> somebody else already called a function in environment.c (say,
> get_refs_directory()) and also have already _acted_ on it
> (e.g. calling get_packed_refs() which populates the list of refs
> based on the old value of "$GIT_DIR/packed-refs")?  Calling the
> function again would not undo/redo that, so maybe the calling
> sequence into setup_git_env() needs to be made safer.
> 
> I think the only thing you care about in your "where is the repo
> and where is the worktree" codepath are get_git_dir() and
> is_bare_repository().  As a side effect of calling these
> functions for your own purpose, you later have to call
> setup_git_env() again to clean up, which is fine.  I would feel
> better if there is an assert in setup_git_env that catches the
> case where it is called for the second time even though the
> first caller was something other than this repository/worktree
> discovery code.

You mean for the third time (first: git_get_dir() from git_config(), second: setup_gdg, third: someone who should not call setup_git_env)?

I think we could also work around to use setup_git_env() at all: the guessing part of is_bare_repository() could be moved out of is_bare_repository() and there could be an additional git_config() function taking the path to the repository as argument.

Show 26 quoted lines
> > diff --git a/environment.c b/environment.c
> > index 713a011..769d409 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -60,8 +60,15 @@ void setup_git_env(void)
> >  int is_bare_repository(void)
> >  {
> >  	const char *dir, *s;
> > -	if (0 <= is_bare_repository_cfg)
> > -		return is_bare_repository_cfg;
> > +	/* definitely bare */
> > +	if (is_bare_repository_cfg == 1)
> > +		return 1;
> > +	/* bare if cwd is outside of the working tree */
> > +	if (inside_working_tree >= 0)
> > +		return !inside_working_tree;
> > +	/* configuration says it is not bare */
> > +	if (is_bare_repository_cfg == 0)
> > +		return 0;
> >  
> >  	dir = get_git_dir();
> >  	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
> 
> For example, calling this function from programs before calling
> the repository/worktree discovery is an error, isn't it?  Can we
> have a safety here to catch such a programming error?

Right, before setup_gdg is_bare_repository() will only guess based on the name, with core.bare being used by setup_gdg this guessing should not be used if there is more accurate information after calling setup_gdg.

You mean something like assert(inside_work_tree >= 0); to check if setup_gdg was called yet? I didn't see such checks in the git source yet but I like it, it is better to have an error message pointing at the bug instead of strange behaviour which may result in a bug.

Show 23 quoted lines
> > diff --git a/setup.c b/setup.c
> > index a45ea83..794edcf 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -192,67 +192,168 @@ int is_inside_git_dir(void)
> >  	return inside_git_dir;
> >  }
> >  
> > +static char *git_work_tree;
> > +
> > +static int git_setup_config(const char *var, const char *value)
> > +{
> > +	if (git_work_tree && !strcmp(var, "core.worktree")) {
> > +		strlcpy(git_work_tree, value, PATH_MAX);
> > +	}
> > +	return git_default_config(var, value);
> > +}
> > +
> 
> Other config functions do not pass already handled variables to
> git_default_config().  You probably would care about core.bare
> in your own code, so falling back to git_default_config() is
> fine, although it may look wasteful.

Right, this should be: + if (!strcmp(var, "core.worktree")) { + if (git_work_tree) + strlcpy(git_work_tree, value, PATH_MAX); + return 0; + }

setup_gdg has to be called before calling git_config(), I think we
should do one of
(1) setup_gdg calls git_default_config()
    we can probably get rid of all those
      git_config(git_default_config);
    and
      return git_default_config(var, value);
(2) setup_gdg extracts only core.{bare,worktree} and does not call
    git_default_config()

I'm in favor of the first one: all commands can rely on the default configuration values to be loaded and I don't see a disadvantage yet.

Junio C Hamano· Apr 11, 2007, 01:29 UTC · re: Matthias Lederhofer · lore

Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable

Matthias Lederhofer <matled@gmx.net> writes:
Show 13 quoted lines
> With splitting is_bare_repository() it is no longer abused to check
> for a working tree.  This raises the question what 'bare' is actually
> supposed to mean:
> (1) git may do operations on the repository which have strange effects
>     if I'm using the repository with a working tree (e.g. update HEAD
>     during a fetch)
> (2) git may do (1) but also disallows to use the repository with a
> working tree
>
> With (1) it would be ok to let a user work with a repository that has
> core.bare = true if she really wants to (e.g. by setting GIT_WORK_TREE
> explicitly, details to be figured out later).
> With (2) git should disallow the use of a working tree if core.bare = true.

Well, core.bare was designed without any support for something like GIT_WORK_TREE in mind, so I do not think we should be tied too much to whatever the current implementation happens to do.

If you are using GIT_WORK_TREE and GIT_DIR to name two directories that are not connected in the usual way (i.e. $GIT_DIR != $GIT_WORK_TREE/.git) to work in, I *think* your intention is "I want to checkout, diff, commit and all the usual git operations in GIT_WORK_TREE to grow branches in GIT_DIR". I do not think core.bare should even be looked at in this case.

A quick "git grep is_bare" reveals that:
 - "git-branch foo $commit" refuses if the repository is not
   bare and foo is the current branch.  The intention here is if
   we allow it then it makes the HEAD and index+working tree out
   of sync, so this clearly is about "does it have a working
   tree, and is it used with one"?
 - "git gc" runs pack-refs only if the repository is not bare.
   This is to prevent public repositories from getting
   pack-ref-pruned (which would make older dumb clients not to
   be able to fetch from it).  This _is_ about is_bare; it does
   not matter if somebody else happens to use it with a working
   tree.
 - reflog update code in refs.c defaults log_all_ref_updates
   value to true when !is-bare; the intent is if the repository
   is used to actively build history with working tree we would
   want the reflog, otherwise if it is primarily used to track
   other repositories (either pushing into it, or fetching from
   elsewhere) reflog is often clutter.  So this is not about
   bare, but "does it have a working tree?".
 - "git ls-files" refuses if "is-bare" or "is-inside-git-dir" is
   true.  The intention is we want ls-files to be run inside the
   working tree.
 - Any other commands that says NOT_BARE does the same as
   ls-files, and the error message is "must be run in a work
   tree".
Show 9 quoted lines
> One option to solve this is to make core.bare have three possible
> values, so the user can decide if he wants (1) or (2).
>     core.bare = true: no working tree allowed
>     core.bare = false: don't allow git-fetch to update HEAD etc.
>     core.bare = mixed: allow both
> In the third case is_bare_repository() would be true and
> inside_work_tree() would change depending on cwd and GIT_WORK_TREE
> setting.  This would allow git fetch to update the current branch even
> while inside the working tree.

I am not sure if you even want the "no working tree allowed" case. Doesn't it directly contradict with the purpose of the whole GIT_WORK_TREE idea?

I did not look at the Porcelain-ish scripts, but for example in the case of git-fetch, the reason it does not allow updating the current branch is the same as "git-branch" above.

So I suspect that most of the existing use of is_bare is just an implementation detail of "do we have a working tree?", and it has been a good implementation primarily because we did not even allow doing something like GIT_WORK_TREE to begin with. They need to be adjusted to work well with GIT_WORK_TREE.

Show 12 quoted lines
>> I think the only thing you care about in your "where is the repo
>> and where is the worktree" codepath are get_git_dir() and
>> is_bare_repository().  As a side effect of calling these
>> functions for your own purpose, you later have to call
>> setup_git_env() again to clean up, which is fine.  I would feel
>> better if there is an assert in setup_git_env that catches the
>> case where it is called for the second time even though the
>> first caller was something other than this repository/worktree
>> discovery code.
>
> You mean for the third time (first: git_get_dir() from git_config(),
> second: setup_gdg, third: someone who should not call setup_git_env)?
I am more worried about a case where
 (1) a program calls some function A (perhaps in environment.c);
 (2) both of us are forgetting right now that A ends up calling
    setup_git_env();
 (3) the result from A is used to make decision by the calling
     program, and...
 (4) then the program calls setup_git_directory().

Currently the damange is limited as A cannot be outside environment.c because setup_git_env() is static there, but whatever the program decides in step (3) may need to be rolled back and recomputed if setup_git_directory() ends up moving GIT_DIR and friends around.

> You mean something like assert(inside_work_tree >= 0); to check if
> setup_gdg was called yet?  I didn't see such checks in the git source
> yet but I like it, it is better to have an error message pointing at
> the bug instead of strange behaviour which may result in a bug.

Yes, but I would rather have something stroner than assert() that cannot be compiled away. An explicit if (...) die("Gaah").

← back to recent threads