git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC] introduce GIT_WORK_TREE environment variable
@ 2007-03-28 14:15 Matthias Lederhofer
  2007-03-28 15:45 ` Johannes Sixt
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Matthias Lederhofer @ 2007-03-28 14:15 UTC (permalink / raw)
  To: git

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(-)

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

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
@ 2007-03-28 15:45 ` Johannes Sixt
  2007-03-29  8:22 ` Matthias Lederhofer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Johannes Sixt @ 2007-03-28 15:45 UTC (permalink / raw)
  To: git

Matthias Lederhofer wrote:
> 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

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
  2007-03-28 15:45 ` Johannes Sixt
@ 2007-03-29  8:22 ` Matthias Lederhofer
  2007-04-04 14:08 ` Matthias Lederhofer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Matthias Lederhofer @ 2007-03-29  8:22 UTC (permalink / raw)
  To: git

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().

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
  2007-03-28 15:45 ` Johannes Sixt
  2007-03-29  8:22 ` Matthias Lederhofer
@ 2007-04-04 14:08 ` Matthias Lederhofer
  2007-04-04 16:59   ` Dana How
  2007-04-04 18:45   ` Junio C Hamano
  2007-04-04 20:09 ` [PATCH] export setup_git_env() Matthias Lederhofer
  2007-04-04 20:13 ` [PATCH(amend)] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
  4 siblings, 2 replies; 12+ messages in thread
From: Matthias Lederhofer @ 2007-04-04 14:08 UTC (permalink / raw)
  To: git

As 1.5.1 is released now: any comments on this patch?

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-04-04 14:08 ` Matthias Lederhofer
@ 2007-04-04 16:59   ` Dana How
  2007-04-04 18:45   ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Dana How @ 2007-04-04 16:59 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git, danahow

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

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-04-04 14:08 ` Matthias Lederhofer
  2007-04-04 16:59   ` Dana How
@ 2007-04-04 18:45   ` Junio C Hamano
  2007-04-04 21:04     ` Matthias Lederhofer
  1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2007-04-04 18:45 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git

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.

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

* [PATCH] export setup_git_env()
  2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
                   ` (2 preceding siblings ...)
  2007-04-04 14:08 ` Matthias Lederhofer
@ 2007-04-04 20:09 ` Matthias Lederhofer
  2007-04-04 20:13 ` [PATCH(amend)] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
  4 siblings, 0 replies; 12+ messages in thread
From: Matthias Lederhofer @ 2007-04-04 20:09 UTC (permalink / raw)
  To: git

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(-)

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

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

* [PATCH(amend)] introduce GIT_WORK_TREE environment variable
  2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
                   ` (3 preceding siblings ...)
  2007-04-04 20:09 ` [PATCH] export setup_git_env() Matthias Lederhofer
@ 2007-04-04 20:13 ` Matthias Lederhofer
  2007-04-04 22:34   ` Junio C Hamano
  4 siblings, 1 reply; 12+ messages in thread
From: Matthias Lederhofer @ 2007-04-04 20:13 UTC (permalink / raw)
  To: git

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(-)

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

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

* Re: [PATCH/RFC] introduce GIT_WORK_TREE environment variable
  2007-04-04 18:45   ` Junio C Hamano
@ 2007-04-04 21:04     ` Matthias Lederhofer
  0 siblings, 0 replies; 12+ messages in thread
From: Matthias Lederhofer @ 2007-04-04 21:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

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

* Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable
  2007-04-04 20:13 ` [PATCH(amend)] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
@ 2007-04-04 22:34   ` Junio C Hamano
  2007-04-06 13:21     ` Matthias Lederhofer
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2007-04-04 22:34 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git

Matthias Lederhofer <matled@gmx.net> writes:

> 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.

> 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?

>   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.

> 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?

> 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.

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

* Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable
  2007-04-04 22:34   ` Junio C Hamano
@ 2007-04-06 13:21     ` Matthias Lederhofer
  2007-04-11  1:29       ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Matthias Lederhofer @ 2007-04-06 13:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano <junkio@cox.net> wrote:
> 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?


> >   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.

> 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.


> >     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.

> >     - 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.

> > 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.

> > 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.

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

* Re: [PATCH(amend)] introduce GIT_WORK_TREE environment variable
  2007-04-06 13:21     ` Matthias Lederhofer
@ 2007-04-11  1:29       ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2007-04-11  1:29 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git

Matthias Lederhofer <matled@gmx.net> writes:

> 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".

> 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.

>> 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").

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

end of thread, other threads:[~2007-04-11  1:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-28 14:15 [PATCH/RFC] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
2007-03-28 15:45 ` Johannes Sixt
2007-03-29  8:22 ` Matthias Lederhofer
2007-04-04 14:08 ` Matthias Lederhofer
2007-04-04 16:59   ` Dana How
2007-04-04 18:45   ` Junio C Hamano
2007-04-04 21:04     ` Matthias Lederhofer
2007-04-04 20:09 ` [PATCH] export setup_git_env() Matthias Lederhofer
2007-04-04 20:13 ` [PATCH(amend)] introduce GIT_WORK_TREE environment variable Matthias Lederhofer
2007-04-04 22:34   ` Junio C Hamano
2007-04-06 13:21     ` Matthias Lederhofer
2007-04-11  1:29       ` Junio C Hamano

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 NNTP newsgroup(s).