[PATCH 0/9] work-tree clean ups

[PATCH 0/9] work-tree clean ups

[Johannes Schindelin]

Hi,

this is the 3rd revision of the work-tree clean up series.  Unlike the 1st 
revision, this passes all the tests.  Unlike the 2nd revision, it has a 
concise and precise logic:

--work-tree=bla overrides GIT_WORK_TREE, which overrides core.bare = true,
which overrides core.worktree, which overrides GIT_DIR/.. when GIT_DIR
ends in /.git, which overrides the directory in which .git/ was found.

The distinction between git_dir and work_tree is much clearer now: you can 
have a work_tree which is inside a git_dir, and programs needing a 
work_tree will no longer complain.

The work tree information is no longer just thrown away.  Instead, you can 
run git from the git_dir and it will work on the work tree without you 
having to cd back and forth.

The wrong distinction between a non-bare repository and a repository with 
a work_tree is no longer there.  A repository is either bare, or it has a 
working directory.  There is no third option.

Ciao,
Dscho

[PATCH 1/9] Add is_absolute_path() and make_absolute_path()

[Johannes Schindelin]

This patch adds convenience functions to work with absolute paths.
The function is_absolute_path() should help the efforts to integrate
the MinGW fork.

Note that make_absolute_path() returns a pointer to a static buffer.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile             |    2 +-
 cache.h              |    5 ++++
 path.c               |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0000-basic.sh     |   16 ++++++++++++
 test-absolute-path.c |   11 ++++++++
 5 files changed, 98 insertions(+), 1 deletions(-)
 create mode 100644 test-absolute-path.c

diff --git a/Makefile b/Makefile
index d8100ad..546e008 100644
--- a/Makefile
+++ b/Makefile
@@ -936,7 +936,7 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X
+TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/cache.h b/cache.h
index 53801b8..98af530 100644
--- a/cache.h
+++ b/cache.h
@@ -358,6 +358,11 @@ int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
 char *enter_repo(char *path, int strict);
+static inline int is_absolute_path(const char *path)
+{
+	return path[0] == '/';
+}
+const char *make_absolute_path(const char *path);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index dfff41f..dfa6ce0 100644
--- a/path.c
+++ b/path.c
@@ -288,3 +288,68 @@ int adjust_shared_perm(const char *path)
 		return -2;
 	return 0;
 }
+
+/* We allow "recursive" symbolic links. Only within reason, though. */
+#define MAXDEPTH 5
+
+const char *make_absolute_path(const char *path)
+{
+	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
+	char cwd[1024] = "";
+	int buf_index = 1, len;
+
+	int depth = MAXDEPTH;
+	char *last_elem = NULL;
+	struct stat st;
+
+	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+		die ("Too long path: %.*s", 60, path);
+
+	while (depth--) {
+		if (stat(buf, &st) || !S_ISDIR(st.st_mode)) {
+			char *last_slash = strrchr(buf, '/');
+			if (last_slash) {
+				*last_slash = '\0';
+				last_elem = xstrdup(last_slash + 1);
+			} else
+				last_elem = xstrdup(buf);
+		}
+
+		if (*buf) {
+			if (!*cwd && getcwd(cwd, sizeof(cwd)) < 0)
+				die ("Could not get current working directory");
+
+			if (chdir(buf))
+				die ("Could not switch to '%s'", buf);
+		}
+		if (getcwd(buf, PATH_MAX) < 0)
+			die ("Could not get current working directory");
+
+		if (last_elem) {
+			int len = strlen(buf);
+			if (len + strlen(last_elem) + 2 > PATH_MAX)
+				die ("Too long path name: '%s/%s'",
+						buf, last_elem);
+			buf[len] = '/';
+			strcpy(buf + len + 1, last_elem);
+			free(last_elem);
+			last_elem = NULL;
+		}
+
+		if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {
+			len = readlink(buf, next_buf, PATH_MAX);
+			if (len < 0)
+				die ("Invalid symlink: %s", buf);
+			next_buf[len] = '\0';
+			buf = next_buf;
+			buf_index = 1 - buf_index;
+			next_buf = bufs[buf_index];
+		} else
+			break;
+	}
+
+	if (*cwd && chdir(cwd))
+		die ("Could not change back to '%s'", cwd);
+
+	return buf;
+}
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 4bba9c0..4e49d59 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -281,4 +281,20 @@ test_expect_success 'update-index D/F conflict' '
 	test $numpath0 = 1
 '
 
+test_expect_success 'absolute path works as expected' '
+	mkdir first &&
+	ln -s ../.git first/.git &&
+	mkdir second &&
+	ln -s ../first second/other &&
+	mkdir third &&
+	dir="$(cd .git; pwd -P)" &&
+	dir2=third/../second/other/.git &&
+	test "$dir" = "$(test-absolute-path $dir2)" &&
+	file="$dir"/index &&
+	test "$file" = "$(test-absolute-path $dir2/index)" &&
+	ln -s ../first/file .git/syml &&
+	sym="$(cd first; pwd -P)"/file &&
+	test "$sym" = "$(test-absolute-path $dir2/syml)"
+'
+
 test_done
diff --git a/test-absolute-path.c b/test-absolute-path.c
new file mode 100644
index 0000000..c959ea2
--- /dev/null
+++ b/test-absolute-path.c
@@ -0,0 +1,11 @@
+#include "cache.h"
+
+int main(int argc, char **argv)
+{
+	while (argc > 1) {
+		puts(make_absolute_path(argv[1]));
+		argc--;
+		argv++;
+	}
+	return 0;
+}
-- 
1.5.3.rc3.28.g1406

[PATCH 2/9] Add functions get_relative_cwd() and is_inside_dir()

[Johannes Schindelin]

The function get_relative_cwd() works just as getcwd(), only that it
takes an absolute path as additional parameter, returning the prefix
of the current working directory relative to the given path.  If the
cwd is no subdirectory of the given path, it returns NULL.

is_inside_dir() is just a trivial wrapper over get_relative_cwd().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c |   30 ++++++++++++++++++++++++++++++
 dir.h |    3 +++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index 8d8faf5..ef67d6f 100644
--- a/dir.c
+++ b/dir.c
@@ -642,3 +642,33 @@ file_exists(const char *f)
   struct stat sb;
   return stat(f, &sb) == 0;
 }
+
+char *get_relative_cwd(char *buffer, int size, const char *dir)
+{
+	char *cwd = buffer;
+
+	if (!dir)
+		return 0;
+
+	if (!getcwd(buffer, size))
+		return 0;
+
+	if (!is_absolute_path(dir))
+		dir = make_absolute_path(dir);
+
+	while (*dir && *dir == *cwd) {
+		dir++;
+		cwd++;
+	}
+	if (*dir)
+		return NULL;
+	if (*cwd == '/')
+		return cwd + 1;
+	return cwd;
+}
+
+int is_inside_dir(const char *dir)
+{
+	char buffer[PATH_MAX];
+	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
+}
diff --git a/dir.h b/dir.h
index ec0e8ab..f55a87b 100644
--- a/dir.h
+++ b/dir.h
@@ -61,4 +61,7 @@ extern void add_exclude(const char *string, const char *base,
 extern int file_exists(const char *);
 extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len);
 
+extern char *get_relative_cwd(char *buffer, int size, const char *dir);
+extern int is_inside_dir(const char *dir);
+
 #endif
-- 
1.5.3.rc3.28.g1406

[PATCH 3/9] white space fixes in setup.c

[Johannes Schindelin]

Some lines were not indented by tabs but by spaces.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Just to make the next patch look nicer.

 setup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/setup.c b/setup.c
index 7b07144..b54d65f 100644
--- a/setup.c
+++ b/setup.c
@@ -382,11 +382,11 @@ int git_config_perm(const char *var, const char *value)
 
 int check_repository_format_version(const char *var, const char *value)
 {
-       if (strcmp(var, "core.repositoryformatversion") == 0)
-               repository_format_version = git_config_int(var, value);
+	if (strcmp(var, "core.repositoryformatversion") == 0)
+		repository_format_version = git_config_int(var, value);
 	else if (strcmp(var, "core.sharedrepository") == 0)
 		shared_repository = git_config_perm(var, value);
-       return 0;
+	return 0;
 }
 
 int check_repository_format(void)
-- 
1.5.3.rc3.28.g1406

[PATCH 4/9] Clean up work-tree handling

[Johannes Schindelin]

The old version of work-tree support was an unholy mess, barely readable,
and not to the point.

For example, why do you have to provide a worktree, when it is not used?
As in "git status".  Now it works.

Another riddle was: if you can have work trees inside the git dir, why
are some programs complaining that they need a work tree?

IOW when inside repo.git/work, where GIT_DIR points to repo.git
and GIT_WORK_TREE to work, and cwd is work, --is-inside-git-dir _must_
return true, because it is _in the git dir_, but scripts _must_ test
for the right thing.

Also, GIT_DIR=../.git should behave the same as if no GIT_DIR was
specified, unless there is a repository in the current working directory.
It does now.

The logic to determine if a repository is bare, or has a work tree
(tertium non datur), is this:

--work-tree=bla overrides GIT_WORK_TREE, which overrides core.bare = true,
which overrides core.worktree, which overrides GIT_DIR/.. when GIT_DIR
ends in /.git, which overrides the directory in which .git/ was found.

In related news, a long standing bug was fixed: when in .git/bla/x.git/,
which is a bare repository, git formerly assumed ../.. to be the
appropriate git dir.  This problem was reported by Shawn Pearce to have
caused much pain, where a colleague mistakenly ran "git init" in "/" a
long time ago, and bare repositories just would not work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This version no longer removes more lines than it adds, but it
	adds a lot more documentation than it removes.  IMHO it is also
	easier to follow, implements a sane logic, and stands a chance
	to be fixed by somebody else than the original author.

 builtin-rev-parse.c |    7 ++
 cache.h             |    2 +
 environment.c       |   35 +++++--
 setup.c             |  279 +++++++++++++++++++++++----------------------------
 4 files changed, 162 insertions(+), 161 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 497903a..15bdb72 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -320,6 +320,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--show-cdup")) {
+				if (!is_inside_work_tree()) {
+					const char *work_tree =
+						get_git_work_tree();
+					if (work_tree)
+						printf("%s\n", work_tree);
+					continue;
+				}
 				const char *pfx = prefix;
 				while (pfx) {
 					pfx = strchr(pfx, '/');
diff --git a/cache.h b/cache.h
index 98af530..c0cab34 100644
--- a/cache.h
+++ b/cache.h
@@ -208,12 +208,14 @@ enum object_type {
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
 extern int is_inside_git_dir(void);
+extern char *git_work_tree_cfg;
 extern int is_inside_work_tree(void);
 extern const char *get_git_dir(void);
 extern char *get_object_directory(void);
 extern char *get_refs_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
+extern const char *get_git_work_tree(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index f83fb9e..26f41af 100644
--- a/environment.c
+++ b/environment.c
@@ -35,6 +35,10 @@ int pager_in_use;
 int pager_use_color = 1;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 
+/* This is set by setup_git_dir_gently() and/or git_default_config() */
+char *git_work_tree_cfg;
+static const char *work_tree;
+
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
@@ -62,15 +66,8 @@ 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;
-
-	dir = get_git_dir();
-	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
-		return 0;
-	s = strrchr(dir, '/');
-	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
+	/* if core.bare is not 'false', let's see if there is a work tree */
+	return is_bare_repository_cfg && !get_git_work_tree();
 }
 
 const char *get_git_dir(void)
@@ -80,6 +77,26 @@ const char *get_git_dir(void)
 	return git_dir;
 }
 
+const char *get_git_work_tree(void)
+{
+	static int initialized = 0;
+	if (!initialized) {
+		work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);
+		/* core.bare = true overrides implicit and config work tree */
+		if (!work_tree && is_bare_repository_cfg < 1) {
+			work_tree = git_work_tree_cfg;
+			/* make_absolute_path also normalizes the path */
+			if (work_tree && !is_absolute_path(work_tree))
+				work_tree = xstrdup(make_absolute_path(git_path(work_tree)));
+		} else if (work_tree)
+			work_tree = xstrdup(make_absolute_path(work_tree));
+		initialized = 1;
+		if (work_tree)
+			is_bare_repository_cfg = 0;
+	}
+	return work_tree;
+}
+
 char *get_object_directory(void)
 {
 	if (!git_object_dir)
diff --git a/setup.c b/setup.c
index b54d65f..8237fe3 100644
--- a/setup.c
+++ b/setup.c
@@ -1,4 +1,8 @@
 #include "cache.h"
+#include "dir.h"
+
+static int inside_git_dir = -1;
+static int inside_work_tree = -1;
 
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
@@ -170,100 +174,89 @@ static int is_git_directory(const char *suspect)
 	return 1;
 }
 
-static int inside_git_dir = -1;
-
 int is_inside_git_dir(void)
 {
-	if (inside_git_dir >= 0)
-		return inside_git_dir;
-	die("BUG: is_inside_git_dir called before setup_git_directory");
+	if (inside_git_dir < 0)
+		inside_git_dir = is_inside_dir(get_git_dir());
+	return inside_git_dir;
 }
 
-static int inside_work_tree = -1;
-
 int is_inside_work_tree(void)
 {
-	if (inside_git_dir >= 0)
-		return inside_work_tree;
-	die("BUG: is_inside_work_tree called before setup_git_directory");
+	if (inside_work_tree < 0)
+		inside_work_tree = is_inside_dir(get_git_work_tree());
+	return inside_work_tree;
 }
 
-static char *gitworktree_config;
-
-static int git_setup_config(const char *var, const char *value)
+/*
+ * If no worktree was given, and we are outside of a default work tree,
+ * now is the time to set it.
+ *
+ * In other words, if the user calls git with something like
+ *
+ *	git --git-dir=/some/where/else/.git bla
+ *
+ * default to /some/where/else as working directory; if the specified
+ * git-dir does not end in "/.git", the cwd is used as working directory.
+ */
+const char* set_work_tree(const char *dir)
 {
-	if (!strcmp(var, "core.worktree")) {
-		if (gitworktree_config)
-			strlcpy(gitworktree_config, value, PATH_MAX);
-		return 0;
+	char dir_buffer[PATH_MAX];
+	static char buffer[PATH_MAX + 1], *rel = NULL;
+	int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
+
+	/* strip the variable 'dir' of the postfix "/.git" if it has it */
+	len = strlen(dir);
+	if (len > postfix_len && !strcmp(dir + len - postfix_len,
+				"/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
+			strncpy(dir_buffer, dir, len - postfix_len);
+
+		/* are we inside the default work tree? */
+		rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
+	}
+	/* if rel is set, the cwd is _not_ the current working tree */
+	if (rel && *rel) {
+		if (!is_absolute_path(dir))
+			set_git_dir(make_absolute_path(dir));
+		dir = dir_buffer;
+		chdir(dir);
+		strcat(rel, "/");
+		inside_git_dir = 0;
+	} else {
+		rel = NULL;
+		dir = getcwd(buffer, sizeof(buffer));
 	}
-	return git_default_config(var, value);
+	git_work_tree_cfg = xstrdup(dir);
+	inside_work_tree = 1;
+
+	return rel;
 }
 
+/*
+ * We cannot decide in this function whether we are in the work tree or
+ * not, since the config can only be read _after_ this function was called.
+ */
 const char *setup_git_directory_gently(int *nongit_ok)
 {
+	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
-	char worktree[PATH_MAX+1], gitdir[PATH_MAX+1];
-	const char *gitdirenv, *gitworktree;
-	int wt_rel_gitdir = 0;
+	const char *gitdirenv;
+	int len, offset;
 
+	/*
+	 * 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) {
-		int len, offset;
-
-		if (!getcwd(cwd, sizeof(cwd)-1))
-			die("Unable to read current working directory");
-
-		offset = len = strlen(cwd);
-		for (;;) {
-			if (is_git_directory(".git"))
-				break;
-			if (offset == 0) {
-				offset = -1;
-				break;
-			}
-			chdir("..");
-			while (cwd[--offset] != '/')
-				; /* do nothing */
-		}
-
-		if (offset >= 0) {
-			inside_work_tree = 1;
-			git_config(git_default_config);
-			if (offset == len) {
-				inside_git_dir = 0;
-				return NULL;
-			}
-
-			cwd[len++] = '/';
-			cwd[len] = '\0';
-			inside_git_dir = !prefixcmp(cwd + offset + 1, ".git/");
-			return cwd + offset + 1;
-		}
-
-		if (chdir(cwd))
-			die("Cannot come back to cwd");
-		if (!is_git_directory(".")) {
-			if (nongit_ok) {
-				*nongit_ok = 1;
-				return NULL;
-			}
-			die("Not a git repository");
-		}
-		setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
-		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-		if (!gitdirenv)
-			die("getenv after setenv failed");
-	}
-
-	if (PATH_MAX - 40 < strlen(gitdirenv)) {
-		if (nongit_ok) {
-			*nongit_ok = 1;
+	if (gitdirenv) {
+		if (PATH_MAX - 40 < strlen(gitdirenv))
+			die("'$%s' too big", GIT_DIR_ENVIRONMENT);
+		if (is_git_directory(gitdirenv)) {
+			if (!work_tree_env)
+				return set_work_tree(gitdirenv);
 			return NULL;
 		}
-		die("$%s too big", GIT_DIR_ENVIRONMENT);
-	}
-	if (!is_git_directory(gitdirenv)) {
 		if (nongit_ok) {
 			*nongit_ok = 1;
 			return NULL;
@@ -273,92 +266,53 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
-	if (chdir(gitdirenv)) {
-		if (nongit_ok) {
-			*nongit_ok = 1;
-			return NULL;
-		}
-		die("Cannot change directory to $%s '%s'",
-			GIT_DIR_ENVIRONMENT, gitdirenv);
-	}
-	if (!getcwd(gitdir, sizeof(gitdir)-1))
-		die("Unable to read current working directory");
-	if (chdir(cwd))
-		die("Cannot come back to cwd");
 
 	/*
-	 * In case there is a work tree we may change the directory,
-	 * therefore make GIT_DIR an absolute path.
+	 * Test in the following order (relative to the cwd):
+	 * - .git/
+	 * - ./ (bare)
+	 * - ../.git/
+	 * - ../ (bare)
+	 * - ../../.git/
+	 *   etc.
 	 */
-	if (gitdirenv[0] != '/') {
-		setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
-		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-		if (!gitdirenv)
-			die("getenv after setenv failed");
-		if (PATH_MAX - 40 < strlen(gitdirenv)) {
-			if (nongit_ok) {
-				*nongit_ok = 1;
-				return NULL;
-			}
-			die("$%s too big after expansion to absolute path",
-				GIT_DIR_ENVIRONMENT);
-		}
-	}
-
-	strcat(cwd, "/");
-	strcat(gitdir, "/");
-	inside_git_dir = !prefixcmp(cwd, gitdir);
-
-	gitworktree = getenv(GIT_WORK_TREE_ENVIRONMENT);
-	if (!gitworktree) {
-		gitworktree_config = worktree;
-		worktree[0] = '\0';
-	}
-	git_config(git_setup_config);
-	if (!gitworktree) {
-		gitworktree_config = NULL;
-		if (worktree[0])
-			gitworktree = worktree;
-		if (gitworktree && gitworktree[0] != '/')
-			wt_rel_gitdir = 1;
-	}
-
-	if (wt_rel_gitdir && chdir(gitdirenv))
-		die("Cannot change directory to $%s '%s'",
-			GIT_DIR_ENVIRONMENT, gitdirenv);
-	if (gitworktree && chdir(gitworktree)) {
-		if (nongit_ok) {
-			if (wt_rel_gitdir && chdir(cwd))
-				die("Cannot come back to cwd");
-			*nongit_ok = 1;
+	offset = len = strlen(cwd);
+	for (;;) {
+		if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
+			break;
+		if (is_git_directory(".")) {
+			inside_git_dir = 1;
+			if (!work_tree_env)
+				inside_work_tree = 0;
+			setenv(GIT_DIR_ENVIRONMENT, ".", 1);
 			return NULL;
 		}
-		if (wt_rel_gitdir)
-			die("Cannot change directory to working tree '%s'"
-				" from $%s", gitworktree, GIT_DIR_ENVIRONMENT);
-		else
-			die("Cannot change directory to working tree '%s'",
-				gitworktree);
-	}
-	if (!getcwd(worktree, sizeof(worktree)-1))
-		die("Unable to read current working directory");
-	strcat(worktree, "/");
-	inside_work_tree = !prefixcmp(cwd, worktree);
-
-	if (gitworktree && inside_work_tree && !prefixcmp(worktree, gitdir) &&
-	    strcmp(worktree, gitdir)) {
-		inside_git_dir = 0;
+		chdir("..");
+		do {
+			if (!offset) {
+				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] != '/');
 	}
 
-	if (!inside_work_tree) {
-		if (chdir(cwd))
-			die("Cannot come back to cwd");
+	inside_git_dir = 0;
+	if (!work_tree_env)
+		inside_work_tree = 1;
+	git_work_tree_cfg = xstrndup(cwd, offset);
+	if (offset == len)
 		return NULL;
-	}
 
-	if (!strcmp(cwd, worktree))
-		return NULL;
-	return cwd+strlen(worktree);
+	/* Make "offset" point to past the '/', and add a '/' at the end */
+	offset++;
+	cwd[len++] = '/';
+	cwd[len] = 0;
+	return cwd + offset;
 }
 
 int git_config_perm(const char *var, const char *value)
@@ -386,6 +340,16 @@ int check_repository_format_version(const char *var, const char *value)
 		repository_format_version = git_config_int(var, value);
 	else if (strcmp(var, "core.sharedrepository") == 0)
 		shared_repository = git_config_perm(var, value);
+	else if (strcmp(var, "core.bare") == 0) {
+		is_bare_repository_cfg = git_config_bool(var, value);
+		if (is_bare_repository_cfg == 1)
+			inside_work_tree = -1;
+	} else if (strcmp(var, "core.worktree") == 0) {
+		if (git_work_tree_cfg)
+			free(git_work_tree_cfg);
+		git_work_tree_cfg = xstrdup(value);
+		inside_work_tree = -1;
+	}
 	return 0;
 }
 
@@ -402,5 +366,16 @@ const char *setup_git_directory(void)
 {
 	const char *retval = setup_git_directory_gently(NULL);
 	check_repository_format();
+
+	/* If the work tree is not the default one, recompute prefix */
+	if (inside_work_tree < 0) {
+		static char buffer[PATH_MAX + 1];
+		if (retval && chdir(retval))
+			die ("Could not jump back into original cwd");
+		char *rel = get_relative_cwd(buffer, PATH_MAX,
+				get_git_work_tree());
+		return rel && *rel ? strcat(rel, "/") : NULL;
+	}
+
 	return retval;
 }
-- 
1.5.3.rc3.28.g1406

[PATCH 5/9] Add set_git_dir() function

[Johannes Schindelin]

With the function set_git_dir() you can reset the path that will
be used for git_path(), git_dir() and friends.

The responsibility to close files and throw away information from the
old git_dir lies with the caller.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h       |    1 +
 environment.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index c0cab34..36963f2 100644
--- a/cache.h
+++ b/cache.h
@@ -216,6 +216,7 @@ extern char *get_refs_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern const char *get_git_work_tree(void);
+extern int set_git_dir(const char *path);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index 26f41af..2af12fd 100644
--- a/environment.c
+++ b/environment.c
@@ -124,3 +124,11 @@ char *get_graft_file(void)
 		setup_git_env();
 	return git_graft_file;
 }
+
+int set_git_dir(const char *path)
+{
+	if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
+		return error("Could not set GIT_DIR to '%s'", path);
+	setup_git_env();
+	return 0;
+}
-- 
1.5.3.rc3.28.g1406

[PATCH 6/9] work-trees are allowed inside a git-dir

[Johannes Schindelin]

It is allowed to call

	$ git --git-dir=../ --work-tree=. bla

when you really want to.  In this case, you are both in the git directory
and in the working tree.

The earlier handling of this situation was seriously bogus.  For regular
working tree operations, it checked if inside git dir.  That makes no
sense, of course, since the check should be for a work tree, and nothing
else.

Fix that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-ls-files.c |    8 +++++---
 git-sh-setup.sh    |    3 +--
 git.c              |   11 ++++++++---
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 61577ea..d36181a 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -469,9 +469,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (require_work_tree &&
-			(!is_inside_work_tree() || is_inside_git_dir()))
-		die("This operation must be run in a work tree");
+	if (require_work_tree && !is_inside_work_tree()) {
+		const char *work_tree = get_git_work_tree();
+		if (!work_tree || chdir(work_tree))
+			die("This operation must be run in a work tree");
+	}
 
 	pathspec = get_pathspec(prefix, argv + i);
 
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c51985e..7bef43f 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -59,8 +59,7 @@ cd_to_toplevel () {
 }
 
 require_work_tree () {
-	test $(git rev-parse --is-inside-work-tree) = true &&
-	test $(git rev-parse --is-inside-git-dir) = false ||
+	test $(git rev-parse --is-inside-work-tree) = true ||
 	die "fatal: $0 cannot be used without a working tree."
 }
 
diff --git a/git.c b/git.c
index a647f9c..2433355 100644
--- a/git.c
+++ b/git.c
@@ -272,9 +272,14 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 		prefix = setup_git_directory();
 	if (p->option & USE_PAGER)
 		setup_pager();
-	if ((p->option & NEED_WORK_TREE) &&
-	    (!is_inside_work_tree() || is_inside_git_dir()))
-		die("%s must be run in a work tree", p->cmd);
+	if (p->option & NEED_WORK_TREE) {
+		const char *work_tree = get_git_work_tree();
+		const char *git_dir = get_git_dir();
+		if (!is_absolute_path(git_dir))
+			set_git_dir(make_absolute_path(git_dir));
+		if (!work_tree || chdir(work_tree))
+			die("%s must be run in a work tree", p->cmd);
+	}
 	trace_argv_printf(argv, argc, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
-- 
1.5.3.rc3.28.g1406

[PATCH 7/9] init: use get_git_work_tree() instead of rolling our own

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-init-db.c |   48 +++++++++++-------------------------------------
 1 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/builtin-init-db.c b/builtin-init-db.c
index 66ddaeb..0d9b1e0 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -174,36 +174,7 @@ static void copy_templates(const char *git_dir, int len, const char *template_di
 	closedir(dir);
 }
 
-/*
- * Get the full path to the working tree specified in $GIT_WORK_TREE
- * or NULL if no working tree is specified.
- */
-static const char *get_work_tree(void)
-{
-	const char *git_work_tree;
-	char cwd[PATH_MAX];
-	static char worktree[PATH_MAX];
-
-	git_work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);
-	if (!git_work_tree)
-		return NULL;
-	if (!getcwd(cwd, sizeof(cwd)))
-		die("Unable to read current working directory");
-	if (chdir(git_work_tree))
-		die("Cannot change directory to specified working tree '%s'",
-			git_work_tree);
-	if (git_work_tree[0] != '/') {
-		if (!getcwd(worktree, sizeof(worktree)))
-			die("Unable to read current working directory");
-		git_work_tree = worktree;
-	}
-	if (chdir(cwd))
-		die("Cannot come back to cwd");
-	return git_work_tree;
-}
-
-static int create_default_files(const char *git_dir, const char *git_work_tree,
-	const char *template_path)
+static int create_default_files(const char *git_dir, const char *template_path)
 {
 	unsigned len = strlen(git_dir);
 	static char path[PATH_MAX];
@@ -282,16 +253,16 @@ static int create_default_files(const char *git_dir, const char *git_work_tree,
 	}
 	git_config_set("core.filemode", filemode ? "true" : "false");
 
-	if (is_bare_repository() && !git_work_tree) {
+	if (is_bare_repository())
 		git_config_set("core.bare", "true");
-	}
 	else {
+		const char *work_tree = get_git_work_tree();
 		git_config_set("core.bare", "false");
 		/* allow template config file to override the default */
 		if (log_all_ref_updates == -1)
 		    git_config_set("core.logallrefupdates", "true");
-		if (git_work_tree)
-			git_config_set("core.worktree", git_work_tree);
+		if (work_tree != git_work_tree_cfg)
+			git_config_set("core.worktree", work_tree);
 	}
 	return reinit;
 }
@@ -308,7 +279,6 @@ static const char init_db_usage[] =
 int cmd_init_db(int argc, const char **argv, const char *prefix)
 {
 	const char *git_dir;
-	const char *git_work_tree;
 	const char *sha1_dir;
 	const char *template_dir = NULL;
 	char *path;
@@ -329,7 +299,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			usage(init_db_usage);
 	}
 
-	git_work_tree = get_work_tree();
+	git_work_tree_cfg = xcalloc(PATH_MAX, 1);
+	if (!getcwd(git_work_tree_cfg, PATH_MAX))
+		die ("Cannot access current working directory.");
+	if (access(get_git_work_tree(), X_OK))
+		die ("Cannot access work tree '%s'", get_git_work_tree());
 
 	/*
 	 * Set up the default .git directory contents
@@ -346,7 +320,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	 */
 	check_repository_format();
 
-	reinit = create_default_files(git_dir, git_work_tree, template_dir);
+	reinit = create_default_files(git_dir, template_dir);
 
 	/*
 	 * And set up the object store.
-- 
1.5.3.rc3.28.g1406

[PATCH 8/9] Fix t1501 for updated work-tree logic

[Johannes Schindelin]

t1501 is still a PITA to debug.  But I decided not to change it, so
that it is easier to see what the differences in the logic are.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1501-worktree.sh |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index aadeeab..7322161 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -33,17 +33,17 @@ mv .git repo.git || exit 1
 
 say "core.worktree = relative path"
 export GIT_DIR=repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 unset GIT_WORK_TREE
 git config core.worktree ../work
 test_rev_parse 'outside'      false false false
 cd work || exit 1
 export GIT_DIR=../repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 test_rev_parse 'inside'       false false true ''
 cd sub/dir || exit 1
 export GIT_DIR=../../../repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 test_rev_parse 'subdirectory' false false true sub/dir/
 cd ../../.. || exit 1
 
@@ -84,9 +84,23 @@ test_rev_parse 'in repo.git'              false true  false
 cd objects || exit 1
 test_rev_parse 'in repo.git/objects'      false true  false
 cd ../work || exit 1
-test_rev_parse 'in repo.git/work'         false false true ''
+test_rev_parse 'in repo.git/work'         false true true ''
 cd sub/dir || exit 1
-test_rev_parse 'in repo.git/sub/dir' false false true sub/dir/
+test_rev_parse 'in repo.git/sub/dir' false true true sub/dir/
 cd ../../../.. || exit 1
 
+test_expect_success 'repo finds its work tree' '
+	(cd repo.git &&
+	 : > work/sub/dir/untracked &&
+	 test sub/dir/untracked = "$(git ls-files --others)")
+'
+
+test_expect_success 'repo finds its work tree from work tree, too' '
+	(cd repo.git/work/sub/dir &&
+	 : > tracked &&
+	 git --git-dir=../../.. add tracked &&
+	 cd ../../.. &&
+	 test sub/dir/tracked = "$(git ls-files)")
+'
+
 test_done
-- 
1.5.3.rc3.28.g1406

[PATCH 9/9] Fix t1500 for sane work-tree behavior

[Johannes Schindelin]

When GIT_DIR=../.git, and no worktree is specified, it is reasonable
to assume that the repository is not bare, that the work tree is ".."
and that the prefix is the basename of the current directory.

This is the sane behavior.

t1500 tested for the old behavior, which was plain wrong.  And this
patch fixes it minimally.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1500-rev-parse.sh |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index ec49966..bea40cb 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -31,9 +31,9 @@ test_rev_parse() {
 test_rev_parse toplevel false false true ''
 
 cd .git || exit 1
-test_rev_parse .git/ false true true .git/
+test_rev_parse .git/ true true false ''
 cd objects || exit 1
-test_rev_parse .git/objects/ false true true .git/objects/
+test_rev_parse .git/objects/ true true false ''
 cd ../.. || exit 1
 
 mkdir -p sub/dir || exit 1
@@ -42,7 +42,7 @@ test_rev_parse subdirectory false false true sub/dir/
 cd ../.. || exit 1
 
 git config core.bare true
-test_rev_parse 'core.bare = true' true false true
+test_rev_parse 'core.bare = true' true false false
 
 git config --unset core.bare
 test_rev_parse 'core.bare undefined' false false true
@@ -50,28 +50,28 @@ test_rev_parse 'core.bare undefined' false false true
 mkdir work || exit 1
 cd work || exit 1
 export GIT_DIR=../.git
-export GIT_CONFIG="$GIT_DIR"/config
+export GIT_CONFIG="$(pwd)"/../.git/config
 
 git config core.bare false
-test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true work/
 
 git config core.bare true
-test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''
 
 git config --unset core.bare
-test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true work/
 
 mv ../.git ../repo.git || exit 1
 export GIT_DIR=../repo.git
-export GIT_CONFIG="$GIT_DIR"/config
+export GIT_CONFIG="$(pwd)"/../repo.git/config
 
 git config core.bare false
 test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true ''
 
 git config core.bare true
-test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false true ''
+test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''
 
 git config --unset core.bare
-test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' true false true ''
+test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
 
 test_done
-- 
1.5.3.rc3.28.g1406

[UNWANTED PATCH] Die if core.bare = true and core.worktree is set

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This patch is just here to demonstrate how a core.bare = true / 
	core.worktree conflict could be caught.

	Personally, I do not deem it worth the effort, as can be seen by 
	the lack of adjustments to t1501, which fails utterly with this 
	patch.

 environment.c |    3 +--
 setup.c       |    8 ++++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/environment.c b/environment.c
index 2af12fd..4c5db32 100644
--- a/environment.c
+++ b/environment.c
@@ -82,8 +82,7 @@ const char *get_git_work_tree(void)
 	static int initialized = 0;
 	if (!initialized) {
 		work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);
-		/* core.bare = true overrides implicit and config work tree */
-		if (!work_tree && is_bare_repository_cfg < 1) {
+		if (!work_tree) {
 			work_tree = git_work_tree_cfg;
 			/* make_absolute_path also normalizes the path */
 			if (work_tree && !is_absolute_path(work_tree))
diff --git a/setup.c b/setup.c
index 8237fe3..b0febed 100644
--- a/setup.c
+++ b/setup.c
@@ -342,13 +342,17 @@ int check_repository_format_version(const char *var, const char *value)
 		shared_repository = git_config_perm(var, value);
 	else if (strcmp(var, "core.bare") == 0) {
 		is_bare_repository_cfg = git_config_bool(var, value);
-		if (is_bare_repository_cfg == 1)
-			inside_work_tree = -1;
+		if (is_bare_repository_cfg == 1 && inside_work_tree < 0)
+			die ("Contradicting config settings for "
+				"bare and worktree");
 	} else if (strcmp(var, "core.worktree") == 0) {
 		if (git_work_tree_cfg)
 			free(git_work_tree_cfg);
 		git_work_tree_cfg = xstrdup(value);
 		inside_work_tree = -1;
+		if (is_bare_repository_cfg == 1)
+			die ("Contradicting config settings for "
+				"bare and worktree");
 	}
 	return 0;
 }
-- 
1.5.3.rc3.28.g1406

Re: [PATCH 0/9] work-tree clean ups

[Johannes Schindelin]

Hi,

so this is yet another revision of the work-tree clean ups (sorry to all 
those who grow tired of it; I feel with you: I am tired of it, too).

Junio rightfully pointed out that the tests do not succeed after each 
single step of the series.

Alas, after thinking about it for quite some time, I do not think there is 
any way around squashing all the earlier steps 4,6--9 into one step.

There is not really much that can be done about step 6/9: if we are in a 
work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
this does not break git-clean, as a work tree is a work tree is a work 
tree.  If the user was stupid enough to specify the same directory as 
GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
tool, and you can harm yourself with it.  Tough.)

Patch 7/9 is needed, because the old logic in git-init was wrong, and only 
hidden by the fact that the work-tree logic was implemented wrongly to 
begin with.

Patches 8 and 9/9 only updated the tests to ensure a sane logic, instead 
of an unsane one.  So they are needed, too.

Note: if you are in a bare repository (a repository which either says 
"core.bare = false" in the config, or which is a direct ancestor 
directory, i.e. ../[...]/.. of the current working directory) there will 
_not_ be an automatic working directory assignment.  You will be operating 
_without_ any work tree, unless you specify one.

I somehow feel that core.bare = true weighs more than core.worktree = 
/some/thing, and therefore I implemented it that way, but hey, if enough 
people disagree, then I'll change it.

Maybe I should add two comments?

Namely that setup_git_directory_gently() does _not_ check the config if 
the repository version is right, and where the working directory is, and 
if the repository is bare.  setup_git_directory() does...

And that setup_git_directory_gently() _does_ try to be smart about 
get_git_work_tree(), is_inside_git_dir() and is_inside_work_tree() by 
assigning their return values, and only if core.bare or core.worktree (or 
--work-tree=<something> or GIT_WORK_TREE) are set, get_git_work_tree() and 
is_inside_work_tree() are reset to recalculate what is happening...  
(actually, that is not completely true: if we _know_ that GIT_WORK_TREE is 
set, or --work-tree=<something> which is almost the same, we will defer 
the calculation until one of the functions get_git_work_tree() and 
is_inside_work_tree() is called.)

IMHO we should (probably after 1.5.3) change setup_git_directory_gently() 
to call check_repository_format() in every return path, so that we 
ascertain that the current repository is recent enough.  Because that 
function now checks also if the repo is bare, and if it has a worktree 
set, in addition to ensuring a valid repository.

In hindsight, I should have separated the "check .git/, then ./, and if no 
git_dir was found, continue with the parent directory" into a separate 
patch, but frankly, I am sick and tired of the work-tree series.  It was 
not done right in the first place, and it used hard-to-understand code to 
hide the fact.

Ciao,
Dscho

P.S.: After reading my patch to the tests, I have to disagree strongly 
with my notion that _not_ cleaning up the tests to use some sane syntax 
would make them clearer.  Nevertheless, I think I'll let them stand as an 
example how _not_ to write tests.

[PATCH 1/4] Add is_absolute_path() and make_absolute_path()

[Johannes Schindelin]

This patch adds convenience functions to work with absolute paths.
The function is_absolute_path() should help the efforts to integrate
the MinGW fork.

Note that make_absolute_path() returns a pointer to a static buffer.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile             |    2 +-
 cache.h              |    5 ++++
 path.c               |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0000-basic.sh     |   16 ++++++++++++
 test-absolute-path.c |   11 ++++++++
 5 files changed, 98 insertions(+), 1 deletions(-)
 create mode 100644 test-absolute-path.c

diff --git a/Makefile b/Makefile
index 149df1b..41c4de0 100644
--- a/Makefile
+++ b/Makefile
@@ -937,7 +937,7 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X
+TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/cache.h b/cache.h
index 53801b8..98af530 100644
--- a/cache.h
+++ b/cache.h
@@ -358,6 +358,11 @@ int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
 char *enter_repo(char *path, int strict);
+static inline int is_absolute_path(const char *path)
+{
+	return path[0] == '/';
+}
+const char *make_absolute_path(const char *path);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index dfff41f..27b4ac9 100644
--- a/path.c
+++ b/path.c
@@ -288,3 +288,68 @@ int adjust_shared_perm(const char *path)
 		return -2;
 	return 0;
 }
+
+/* We allow "recursive" symbolic links. Only within reason, though. */
+#define MAXDEPTH 5
+
+const char *make_absolute_path(const char *path)
+{
+	static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
+	char cwd[1024] = "";
+	int buf_index = 1, len;
+
+	int depth = MAXDEPTH;
+	char *last_elem = NULL;
+	struct stat st;
+
+	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+		die ("Too long path: %.*s", 60, path);
+
+	while (depth--) {
+		if (stat(buf, &st) || !S_ISDIR(st.st_mode)) {
+			char *last_slash = strrchr(buf, '/');
+			if (last_slash) {
+				*last_slash = '\0';
+				last_elem = xstrdup(last_slash + 1);
+			} else
+				last_elem = xstrdup(buf);
+		}
+
+		if (*buf) {
+			if (!*cwd && !getcwd(cwd, sizeof(cwd)))
+				die ("Could not get current working directory");
+
+			if (chdir(buf))
+				die ("Could not switch to '%s'", buf);
+		}
+		if (getcwd(buf, PATH_MAX) < 0)
+			die ("Could not get current working directory");
+
+		if (last_elem) {
+			int len = strlen(buf);
+			if (len + strlen(last_elem) + 2 > PATH_MAX)
+				die ("Too long path name: '%s/%s'",
+						buf, last_elem);
+			buf[len] = '/';
+			strcpy(buf + len + 1, last_elem);
+			free(last_elem);
+			last_elem = NULL;
+		}
+
+		if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {
+			len = readlink(buf, next_buf, PATH_MAX);
+			if (len < 0)
+				die ("Invalid symlink: %s", buf);
+			next_buf[len] = '\0';
+			buf = next_buf;
+			buf_index = 1 - buf_index;
+			next_buf = bufs[buf_index];
+		} else
+			break;
+	}
+
+	if (*cwd && chdir(cwd))
+		die ("Could not change back to '%s'", cwd);
+
+	return buf;
+}
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 4bba9c0..4e49d59 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -281,4 +281,20 @@ test_expect_success 'update-index D/F conflict' '
 	test $numpath0 = 1
 '
 
+test_expect_success 'absolute path works as expected' '
+	mkdir first &&
+	ln -s ../.git first/.git &&
+	mkdir second &&
+	ln -s ../first second/other &&
+	mkdir third &&
+	dir="$(cd .git; pwd -P)" &&
+	dir2=third/../second/other/.git &&
+	test "$dir" = "$(test-absolute-path $dir2)" &&
+	file="$dir"/index &&
+	test "$file" = "$(test-absolute-path $dir2/index)" &&
+	ln -s ../first/file .git/syml &&
+	sym="$(cd first; pwd -P)"/file &&
+	test "$sym" = "$(test-absolute-path $dir2/syml)"
+'
+
 test_done
diff --git a/test-absolute-path.c b/test-absolute-path.c
new file mode 100644
index 0000000..c959ea2
--- /dev/null
+++ b/test-absolute-path.c
@@ -0,0 +1,11 @@
+#include "cache.h"
+
+int main(int argc, char **argv)
+{
+	while (argc > 1) {
+		puts(make_absolute_path(argv[1]));
+		argc--;
+		argv++;
+	}
+	return 0;
+}
-- 
1.5.3.rc3.94.g3024

[PATCH 2/4] Add functions get_relative_cwd() and is_inside_dir()

[Johannes Schindelin]

The function get_relative_cwd() works just as getcwd(), only that it
takes an absolute path as additional parameter, returning the prefix
of the current working directory relative to the given path.  If the
cwd is no subdirectory of the given path, it returns NULL.

is_inside_dir() is just a trivial wrapper over get_relative_cwd().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c |   32 ++++++++++++++++++++++++++++++++
 dir.h |    3 +++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index 8d8faf5..5ea269a 100644
--- a/dir.c
+++ b/dir.c
@@ -642,3 +642,35 @@ file_exists(const char *f)
   struct stat sb;
   return stat(f, &sb) == 0;
 }
+
+/*
+ * get_relative_cwd() gets the prefix of the current working directory
+ * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
+ * As a convenience, it also returns NULL if 'dir' is already NULL.
+ */
+char *get_relative_cwd(char *buffer, int size, const char *dir)
+{
+	char *cwd = buffer;
+
+	if (!dir || !getcwd(buffer, size))
+		return NULL;
+
+	if (!is_absolute_path(dir))
+		dir = make_absolute_path(dir);
+
+	while (*dir && *dir == *cwd) {
+		dir++;
+		cwd++;
+	}
+	if (*dir)
+		return NULL;
+	if (*cwd == '/')
+		return cwd + 1;
+	return cwd;
+}
+
+int is_inside_dir(const char *dir)
+{
+	char buffer[PATH_MAX];
+	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
+}
diff --git a/dir.h b/dir.h
index ec0e8ab..f55a87b 100644
--- a/dir.h
+++ b/dir.h
@@ -61,4 +61,7 @@ extern void add_exclude(const char *string, const char *base,
 extern int file_exists(const char *);
 extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len);
 
+extern char *get_relative_cwd(char *buffer, int size, const char *dir);
+extern int is_inside_dir(const char *dir);
+
 #endif
-- 
1.5.3.rc3.94.g3024

#

Re: [PATCH 2/4] Add functions get_relative_cwd() and is_inside_dir()

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The function get_relative_cwd() works just as getcwd(), only that it
> takes an absolute path as additional parameter, returning the prefix
> of the current working directory relative to the given path.  If the
> cwd is no subdirectory of the given path, it returns NULL.
> ...
> +/*
> + * get_relative_cwd() gets the prefix of the current working directory
> + * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
> + * As a convenience, it also returns NULL if 'dir' is already NULL.
> + */
> +char *get_relative_cwd(char *buffer, int size, const char *dir)
> +{
> +	char *cwd = buffer;
> +
> +	if (!dir || !getcwd(buffer, size))
> +		return NULL;

When is it not a fatal error if get_relative_cwd() is called
with a NULL dir parameter, or getcwd() fails?

If there is no valid such cases, I would rather have this
die(), former with "BUG" and the latter with strerror(errno).

Re: [PATCH 2/4] Add functions get_relative_cwd() and is_inside_dir()

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> The function get_relative_cwd() works just as getcwd(), only that it
>> takes an absolute path as additional parameter, returning the prefix
>> of the current working directory relative to the given path.  If the
>> cwd is no subdirectory of the given path, it returns NULL.
>> ...
>> +/*
>> + * get_relative_cwd() gets the prefix of the current working directory
>> + * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
>> + * As a convenience, it also returns NULL if 'dir' is already NULL.
>> + */
>> +char *get_relative_cwd(char *buffer, int size, const char *dir)
>> +{
>> +	char *cwd = buffer;
>> +
>> +	if (!dir || !getcwd(buffer, size))
>> +		return NULL;
>
> When is it not a fatal error if get_relative_cwd() is called
> with a NULL dir parameter, or getcwd() fails?
>
> If there is no valid such cases, I would rather have this
> die(), former with "BUG" and the latter with strerror(errno).

Heh, it turns out that there is this lazy or clever (depending
on the viewpoint) caller that passes the return value of
get_git_work_tree() to this function and expect this to return
NULL when no work tree is found.

The callers of the is_* functions are much cleaner and in that
sense the series is a definite improvement, but this one
particular obscurity makes me wonder if it is replacing one
unholy mess with a smaller but still unholy mess.

Will apply on "master" and will be part of -rc4, but we probably
would want to have a longer pre-final freeze than usual to
really make sure this one is good.

Re: [PATCH 2/4] Add functions get_relative_cwd() and is_inside_dir()

[Johannes Schindelin]

Hi,

On Tue, 31 Jul 2007, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >> The function get_relative_cwd() works just as getcwd(), only that it
> >> takes an absolute path as additional parameter, returning the prefix
> >> of the current working directory relative to the given path.  If the
> >> cwd is no subdirectory of the given path, it returns NULL.
> >> ...
> >> +/*
> >> + * get_relative_cwd() gets the prefix of the current working directory
> >> + * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
> >> + * As a convenience, it also returns NULL if 'dir' is already NULL.
> >> + */
> >> +char *get_relative_cwd(char *buffer, int size, const char *dir)
> >> +{
> >> +	char *cwd = buffer;
> >> +
> >> +	if (!dir || !getcwd(buffer, size))
> >> +		return NULL;
> >
> > When is it not a fatal error if get_relative_cwd() is called
> > with a NULL dir parameter, or getcwd() fails?
> >
> > If there is no valid such cases, I would rather have this
> > die(), former with "BUG" and the latter with strerror(errno).
> 
> Heh, it turns out that there is this lazy or clever (depending
> on the viewpoint) caller that passes the return value of
> get_git_work_tree() to this function and expect this to return
> NULL when no work tree is found.

Right.  I thought I had said that (something along the lines: it is more 
convenient not having to check the directory), but I probably did not.

> The callers of the is_* functions are much cleaner and in that sense the 
> series is a definite improvement, but this one particular obscurity 
> makes me wonder if it is replacing one unholy mess with a smaller but 
> still unholy mess.
> 
> Will apply on "master" and will be part of -rc4, but we probably would 
> want to have a longer pre-final freeze than usual to really make sure 
> this one is good.

I'll provide a patch which makes the callers of get_relative_cwd() holy, 
and skip the check in get_relative_cwd(), okay?

Ciao,
Dscho

[NOT-SERIOUS PATCH] Make get_relative_cwd() not accept NULL for a directory

[Johannes Schindelin]

Earlier, get_relative_cwd() interpreted "dir == NULL" as "outside of the dir",
and therefore returned NULL.  Be more strict now.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	As promised.

	Okay, I made up my mind.  Allowing "dir == NULL" is not only a matter of
	convenience.  It is the most natural way to say that "dir" is an invalid
	or non-existing directory.

	Besides, this patch adds 14.286% more lines than it removes ;-)

	But ultimately, it is your decision, Junio, and I am d'accord with 
	what you choose.

 dir.c   |    3 ---
 setup.c |   10 +++++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index b3329f4..cfcde13 100644
--- a/dir.c
+++ b/dir.c
@@ -646,7 +646,6 @@ file_exists(const char *f)
 /*
  * get_relative_cwd() gets the prefix of the current working directory
  * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
- * As a convenience, it also returns NULL if 'dir' is already NULL.
  */
 char *get_relative_cwd(char *buffer, int size, const char *dir)
 {
@@ -656,8 +655,6 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
 	 * a lazy caller can pass a NULL returned from get_git_work_tree()
 	 * and rely on this function to return NULL.
 	 */
-	if (!dir)
-		return NULL;
 	if (!getcwd(buffer, size))
 		die("can't find the current directory: %s", strerror(errno));
 
diff --git a/setup.c b/setup.c
index 3653092..2f720f8 100644
--- a/setup.c
+++ b/setup.c
@@ -183,8 +183,10 @@ int is_inside_git_dir(void)
 
 int is_inside_work_tree(void)
 {
-	if (inside_work_tree < 0)
-		inside_work_tree = is_inside_dir(get_git_work_tree());
+	if (inside_work_tree < 0) {
+		const char *work_tree = get_git_work_tree();
+		inside_work_tree = work_tree ? is_inside_dir(work_tree) : 0;
+	}
 	return inside_work_tree;
 }
 
@@ -370,10 +372,12 @@ const char *setup_git_directory(void)
 	/* If the work tree is not the default one, recompute prefix */
 	if (inside_work_tree < 0) {
 		static char buffer[PATH_MAX + 1];
+		const char *work_tree;
 		char *rel;
 		if (retval && chdir(retval))
 			die ("Could not jump back into original cwd");
-		rel = get_relative_cwd(buffer, PATH_MAX, get_git_work_tree());
+		work_tree = get_git_work_tree();
+		rel = work_tree ? get_relative_cwd(buffer, PATH_MAX, work_tree) : NULL;
 		return rel && *rel ? strcat(rel, "/") : NULL;
 	}
 
-- 
1.5.3.rc3.112.gf60b6

Re: [NOT-SERIOUS PATCH] Make get_relative_cwd() not accept NULL for a directory

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	Okay, I made up my mind.  Allowing "dir == NULL" is not
> 	only a matter of convenience.  It is the most natural
> 	way to say that "dir" is an invalid or non-existing
> 	directory.

That is also fine; it only needs to be clarified somehow to
people who might wonder what get_relative_cwd() function is used
for and how to use it in their programs.  The comment that says
"As a convenience" may need to become a bit more elaborate to
say why it is convenient for the callers to do so (e.g. "The
caller may have called another function that returns a directory
to obtain the 'dir', which may have returned NULL as a way to
say 'there is nothing applicable in your case', and in such a
case, your $(cwd) relative to that 'dir' is also something that
cannot be used, hence a NULL is returned").

[PATCH] get_relative_cwd(): clarify why it handles dir == NULL

[Johannes Schindelin]

The comment did not make a good case why it makes sense.  It does so now.

Pointed out by Junio Hamano.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index b3329f4..791c566 100644
--- a/dir.c
+++ b/dir.c
@@ -646,7 +646,16 @@ file_exists(const char *f)
 /*
  * get_relative_cwd() gets the prefix of the current working directory
  * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
- * As a convenience, it also returns NULL if 'dir' is already NULL.
+ *
+ * As a convenience, it also returns NULL if 'dir' is already NULL.  The
+ * reason for this behaviour is that it is natural for functions returning
+ * directory names to return NULL to say "this directory does not exist"
+ * or "this directory is invalid".  These cases are usually handled the
+ * same as if the cwd is not inside 'dir' at all, so get_relative_cwd()
+ * returns NULL for both of them.
+ *
+ * Most notably, get_relative_cwd(buffer, size, get_git_work_tree())
+ * unifies the handling of "outside work tree" with "no work tree at all".
  */
 char *get_relative_cwd(char *buffer, int size, const char *dir)
 {
-- 
1.5.3.rc3.112.gf60b6

[PATCH 3/4] Add set_git_dir() function

[Johannes Schindelin]

With the function set_git_dir() you can reset the path that will
be used for git_path(), git_dir() and friends.

The responsibility to close files and throw away information from the
old git_dir lies with the caller.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h       |    1 +
 environment.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 98af530..e1f94cb 100644
--- a/cache.h
+++ b/cache.h
@@ -214,6 +214,7 @@ extern char *get_object_directory(void);
 extern char *get_refs_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
+extern int set_git_dir(const char *path);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index f83fb9e..a571fae 100644
--- a/environment.c
+++ b/environment.c
@@ -107,3 +107,11 @@ char *get_graft_file(void)
 		setup_git_env();
 	return git_graft_file;
 }
+
+int set_git_dir(const char *path)
+{
+	if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
+		return error("Could not set GIT_DIR to '%s'", path);
+	setup_git_env();
+	return 0;
+}
-- 
1.5.3.rc3.94.g3024

[PATCH 4/4] Clean up work-tree handling

[Johannes Schindelin]

The old version of work-tree support was an unholy mess, barely readable,
and not to the point.

For example, why do you have to provide a worktree, when it is not used?
As in "git status".  Now it works.

Another riddle was: if you can have work trees inside the git dir, why
are some programs complaining that they need a work tree?

IOW it is allowed to call

	$ git --git-dir=../ --work-tree=. bla

when you really want to.  In this case, you are both in the git directory
and in the working tree.  So, programs have to actually test for the right
thing, namely if they are inside a working tree, and not if they are
inside a git directory.

Also, GIT_DIR=../.git should behave the same as if no GIT_DIR was
specified, unless there is a repository in the current working directory.
It does now.

The logic to determine if a repository is bare, or has a work tree
(tertium non datur), is this:

--work-tree=bla overrides GIT_WORK_TREE, which overrides core.bare = true,
which overrides core.worktree, which overrides GIT_DIR/.. when GIT_DIR
ends in /.git, which overrides the directory in which .git/ was found.

In related news, a long standing bug was fixed: when in .git/bla/x.git/,
which is a bare repository, git formerly assumed ../.. to be the
appropriate git dir.  This problem was reported by Shawn Pearce to have
caused much pain, where a colleague mistakenly ran "git init" in "/" a
long time ago, and bare repositories just would not work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-init-db.c    |   48 ++-------
 builtin-ls-files.c   |    8 +-
 builtin-rev-parse.c  |    7 ++
 cache.h              |    2 +
 environment.c        |   35 +++++--
 git-sh-setup.sh      |    3 +-
 git.c                |   11 ++-
 setup.c              |  279 +++++++++++++++++++++++---------------------------
 t/t1500-rev-parse.sh |   20 ++--
 t/t1501-worktree.sh  |   24 ++++-
 10 files changed, 216 insertions(+), 221 deletions(-)

diff --git a/builtin-init-db.c b/builtin-init-db.c
index 66ddaeb..0d9b1e0 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -174,36 +174,7 @@ static void copy_templates(const char *git_dir, int len, const char *template_di
 	closedir(dir);
 }
 
-/*
- * Get the full path to the working tree specified in $GIT_WORK_TREE
- * or NULL if no working tree is specified.
- */
-static const char *get_work_tree(void)
-{
-	const char *git_work_tree;
-	char cwd[PATH_MAX];
-	static char worktree[PATH_MAX];
-
-	git_work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);
-	if (!git_work_tree)
-		return NULL;
-	if (!getcwd(cwd, sizeof(cwd)))
-		die("Unable to read current working directory");
-	if (chdir(git_work_tree))
-		die("Cannot change directory to specified working tree '%s'",
-			git_work_tree);
-	if (git_work_tree[0] != '/') {
-		if (!getcwd(worktree, sizeof(worktree)))
-			die("Unable to read current working directory");
-		git_work_tree = worktree;
-	}
-	if (chdir(cwd))
-		die("Cannot come back to cwd");
-	return git_work_tree;
-}
-
-static int create_default_files(const char *git_dir, const char *git_work_tree,
-	const char *template_path)
+static int create_default_files(const char *git_dir, const char *template_path)
 {
 	unsigned len = strlen(git_dir);
 	static char path[PATH_MAX];
@@ -282,16 +253,16 @@ static int create_default_files(const char *git_dir, const char *git_work_tree,
 	}
 	git_config_set("core.filemode", filemode ? "true" : "false");
 
-	if (is_bare_repository() && !git_work_tree) {
+	if (is_bare_repository())
 		git_config_set("core.bare", "true");
-	}
 	else {
+		const char *work_tree = get_git_work_tree();
 		git_config_set("core.bare", "false");
 		/* allow template config file to override the default */
 		if (log_all_ref_updates == -1)
 		    git_config_set("core.logallrefupdates", "true");
-		if (git_work_tree)
-			git_config_set("core.worktree", git_work_tree);
+		if (work_tree != git_work_tree_cfg)
+			git_config_set("core.worktree", work_tree);
 	}
 	return reinit;
 }
@@ -308,7 +279,6 @@ static const char init_db_usage[] =
 int cmd_init_db(int argc, const char **argv, const char *prefix)
 {
 	const char *git_dir;
-	const char *git_work_tree;
 	const char *sha1_dir;
 	const char *template_dir = NULL;
 	char *path;
@@ -329,7 +299,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			usage(init_db_usage);
 	}
 
-	git_work_tree = get_work_tree();
+	git_work_tree_cfg = xcalloc(PATH_MAX, 1);
+	if (!getcwd(git_work_tree_cfg, PATH_MAX))
+		die ("Cannot access current working directory.");
+	if (access(get_git_work_tree(), X_OK))
+		die ("Cannot access work tree '%s'", get_git_work_tree());
 
 	/*
 	 * Set up the default .git directory contents
@@ -346,7 +320,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	 */
 	check_repository_format();
 
-	reinit = create_default_files(git_dir, git_work_tree, template_dir);
+	reinit = create_default_files(git_dir, template_dir);
 
 	/*
 	 * And set up the object store.
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 61577ea..d36181a 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -469,9 +469,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (require_work_tree &&
-			(!is_inside_work_tree() || is_inside_git_dir()))
-		die("This operation must be run in a work tree");
+	if (require_work_tree && !is_inside_work_tree()) {
+		const char *work_tree = get_git_work_tree();
+		if (!work_tree || chdir(work_tree))
+			die("This operation must be run in a work tree");
+	}
 
 	pathspec = get_pathspec(prefix, argv + i);
 
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 497903a..15bdb72 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -320,6 +320,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--show-cdup")) {
+				if (!is_inside_work_tree()) {
+					const char *work_tree =
+						get_git_work_tree();
+					if (work_tree)
+						printf("%s\n", work_tree);
+					continue;
+				}
 				const char *pfx = prefix;
 				while (pfx) {
 					pfx = strchr(pfx, '/');
diff --git a/cache.h b/cache.h
index e1f94cb..e97af18 100644
--- a/cache.h
+++ b/cache.h
@@ -208,6 +208,7 @@ enum object_type {
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
 extern int is_inside_git_dir(void);
+extern char *git_work_tree_cfg;
 extern int is_inside_work_tree(void);
 extern const char *get_git_dir(void);
 extern char *get_object_directory(void);
@@ -215,6 +216,7 @@ extern char *get_refs_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
+extern const char *get_git_work_tree(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index a571fae..2af12fd 100644
--- a/environment.c
+++ b/environment.c
@@ -35,6 +35,10 @@ int pager_in_use;
 int pager_use_color = 1;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 
+/* This is set by setup_git_dir_gently() and/or git_default_config() */
+char *git_work_tree_cfg;
+static const char *work_tree;
+
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
@@ -62,15 +66,8 @@ 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;
-
-	dir = get_git_dir();
-	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
-		return 0;
-	s = strrchr(dir, '/');
-	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
+	/* if core.bare is not 'false', let's see if there is a work tree */
+	return is_bare_repository_cfg && !get_git_work_tree();
 }
 
 const char *get_git_dir(void)
@@ -80,6 +77,26 @@ const char *get_git_dir(void)
 	return git_dir;
 }
 
+const char *get_git_work_tree(void)
+{
+	static int initialized = 0;
+	if (!initialized) {
+		work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);
+		/* core.bare = true overrides implicit and config work tree */
+		if (!work_tree && is_bare_repository_cfg < 1) {
+			work_tree = git_work_tree_cfg;
+			/* make_absolute_path also normalizes the path */
+			if (work_tree && !is_absolute_path(work_tree))
+				work_tree = xstrdup(make_absolute_path(git_path(work_tree)));
+		} else if (work_tree)
+			work_tree = xstrdup(make_absolute_path(work_tree));
+		initialized = 1;
+		if (work_tree)
+			is_bare_repository_cfg = 0;
+	}
+	return work_tree;
+}
+
 char *get_object_directory(void)
 {
 	if (!git_object_dir)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c51985e..7bef43f 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -59,8 +59,7 @@ cd_to_toplevel () {
 }
 
 require_work_tree () {
-	test $(git rev-parse --is-inside-work-tree) = true &&
-	test $(git rev-parse --is-inside-git-dir) = false ||
+	test $(git rev-parse --is-inside-work-tree) = true ||
 	die "fatal: $0 cannot be used without a working tree."
 }
 
diff --git a/git.c b/git.c
index a647f9c..2433355 100644
--- a/git.c
+++ b/git.c
@@ -272,9 +272,14 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 		prefix = setup_git_directory();
 	if (p->option & USE_PAGER)
 		setup_pager();
-	if ((p->option & NEED_WORK_TREE) &&
-	    (!is_inside_work_tree() || is_inside_git_dir()))
-		die("%s must be run in a work tree", p->cmd);
+	if (p->option & NEED_WORK_TREE) {
+		const char *work_tree = get_git_work_tree();
+		const char *git_dir = get_git_dir();
+		if (!is_absolute_path(git_dir))
+			set_git_dir(make_absolute_path(git_dir));
+		if (!work_tree || chdir(work_tree))
+			die("%s must be run in a work tree", p->cmd);
+	}
 	trace_argv_printf(argv, argc, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
diff --git a/setup.c b/setup.c
index b54d65f..8237fe3 100644
--- a/setup.c
+++ b/setup.c
@@ -1,4 +1,8 @@
 #include "cache.h"
+#include "dir.h"
+
+static int inside_git_dir = -1;
+static int inside_work_tree = -1;
 
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
@@ -170,100 +174,89 @@ static int is_git_directory(const char *suspect)
 	return 1;
 }
 
-static int inside_git_dir = -1;
-
 int is_inside_git_dir(void)
 {
-	if (inside_git_dir >= 0)
-		return inside_git_dir;
-	die("BUG: is_inside_git_dir called before setup_git_directory");
+	if (inside_git_dir < 0)
+		inside_git_dir = is_inside_dir(get_git_dir());
+	return inside_git_dir;
 }
 
-static int inside_work_tree = -1;
-
 int is_inside_work_tree(void)
 {
-	if (inside_git_dir >= 0)
-		return inside_work_tree;
-	die("BUG: is_inside_work_tree called before setup_git_directory");
+	if (inside_work_tree < 0)
+		inside_work_tree = is_inside_dir(get_git_work_tree());
+	return inside_work_tree;
 }
 
-static char *gitworktree_config;
-
-static int git_setup_config(const char *var, const char *value)
+/*
+ * If no worktree was given, and we are outside of a default work tree,
+ * now is the time to set it.
+ *
+ * In other words, if the user calls git with something like
+ *
+ *	git --git-dir=/some/where/else/.git bla
+ *
+ * default to /some/where/else as working directory; if the specified
+ * git-dir does not end in "/.git", the cwd is used as working directory.
+ */
+const char* set_work_tree(const char *dir)
 {
-	if (!strcmp(var, "core.worktree")) {
-		if (gitworktree_config)
-			strlcpy(gitworktree_config, value, PATH_MAX);
-		return 0;
+	char dir_buffer[PATH_MAX];
+	static char buffer[PATH_MAX + 1], *rel = NULL;
+	int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
+
+	/* strip the variable 'dir' of the postfix "/.git" if it has it */
+	len = strlen(dir);
+	if (len > postfix_len && !strcmp(dir + len - postfix_len,
+				"/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
+			strncpy(dir_buffer, dir, len - postfix_len);
+
+		/* are we inside the default work tree? */
+		rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
+	}
+	/* if rel is set, the cwd is _not_ the current working tree */
+	if (rel && *rel) {
+		if (!is_absolute_path(dir))
+			set_git_dir(make_absolute_path(dir));
+		dir = dir_buffer;
+		chdir(dir);
+		strcat(rel, "/");
+		inside_git_dir = 0;
+	} else {
+		rel = NULL;
+		dir = getcwd(buffer, sizeof(buffer));
 	}
-	return git_default_config(var, value);
+	git_work_tree_cfg = xstrdup(dir);
+	inside_work_tree = 1;
+
+	return rel;
 }
 
+/*
+ * We cannot decide in this function whether we are in the work tree or
+ * not, since the config can only be read _after_ this function was called.
+ */
 const char *setup_git_directory_gently(int *nongit_ok)
 {
+	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
-	char worktree[PATH_MAX+1], gitdir[PATH_MAX+1];
-	const char *gitdirenv, *gitworktree;
-	int wt_rel_gitdir = 0;
+	const char *gitdirenv;
+	int len, offset;
 
+	/*
+	 * 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) {
-		int len, offset;
-
-		if (!getcwd(cwd, sizeof(cwd)-1))
-			die("Unable to read current working directory");
-
-		offset = len = strlen(cwd);
-		for (;;) {
-			if (is_git_directory(".git"))
-				break;
-			if (offset == 0) {
-				offset = -1;
-				break;
-			}
-			chdir("..");
-			while (cwd[--offset] != '/')
-				; /* do nothing */
-		}
-
-		if (offset >= 0) {
-			inside_work_tree = 1;
-			git_config(git_default_config);
-			if (offset == len) {
-				inside_git_dir = 0;
-				return NULL;
-			}
-
-			cwd[len++] = '/';
-			cwd[len] = '\0';
-			inside_git_dir = !prefixcmp(cwd + offset + 1, ".git/");
-			return cwd + offset + 1;
-		}
-
-		if (chdir(cwd))
-			die("Cannot come back to cwd");
-		if (!is_git_directory(".")) {
-			if (nongit_ok) {
-				*nongit_ok = 1;
-				return NULL;
-			}
-			die("Not a git repository");
-		}
-		setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
-		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-		if (!gitdirenv)
-			die("getenv after setenv failed");
-	}
-
-	if (PATH_MAX - 40 < strlen(gitdirenv)) {
-		if (nongit_ok) {
-			*nongit_ok = 1;
+	if (gitdirenv) {
+		if (PATH_MAX - 40 < strlen(gitdirenv))
+			die("'$%s' too big", GIT_DIR_ENVIRONMENT);
+		if (is_git_directory(gitdirenv)) {
+			if (!work_tree_env)
+				return set_work_tree(gitdirenv);
 			return NULL;
 		}
-		die("$%s too big", GIT_DIR_ENVIRONMENT);
-	}
-	if (!is_git_directory(gitdirenv)) {
 		if (nongit_ok) {
 			*nongit_ok = 1;
 			return NULL;
@@ -273,92 +266,53 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
-	if (chdir(gitdirenv)) {
-		if (nongit_ok) {
-			*nongit_ok = 1;
-			return NULL;
-		}
-		die("Cannot change directory to $%s '%s'",
-			GIT_DIR_ENVIRONMENT, gitdirenv);
-	}
-	if (!getcwd(gitdir, sizeof(gitdir)-1))
-		die("Unable to read current working directory");
-	if (chdir(cwd))
-		die("Cannot come back to cwd");
 
 	/*
-	 * In case there is a work tree we may change the directory,
-	 * therefore make GIT_DIR an absolute path.
+	 * Test in the following order (relative to the cwd):
+	 * - .git/
+	 * - ./ (bare)
+	 * - ../.git/
+	 * - ../ (bare)
+	 * - ../../.git/
+	 *   etc.
 	 */
-	if (gitdirenv[0] != '/') {
-		setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
-		gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-		if (!gitdirenv)
-			die("getenv after setenv failed");
-		if (PATH_MAX - 40 < strlen(gitdirenv)) {
-			if (nongit_ok) {
-				*nongit_ok = 1;
-				return NULL;
-			}
-			die("$%s too big after expansion to absolute path",
-				GIT_DIR_ENVIRONMENT);
-		}
-	}
-
-	strcat(cwd, "/");
-	strcat(gitdir, "/");
-	inside_git_dir = !prefixcmp(cwd, gitdir);
-
-	gitworktree = getenv(GIT_WORK_TREE_ENVIRONMENT);
-	if (!gitworktree) {
-		gitworktree_config = worktree;
-		worktree[0] = '\0';
-	}
-	git_config(git_setup_config);
-	if (!gitworktree) {
-		gitworktree_config = NULL;
-		if (worktree[0])
-			gitworktree = worktree;
-		if (gitworktree && gitworktree[0] != '/')
-			wt_rel_gitdir = 1;
-	}
-
-	if (wt_rel_gitdir && chdir(gitdirenv))
-		die("Cannot change directory to $%s '%s'",
-			GIT_DIR_ENVIRONMENT, gitdirenv);
-	if (gitworktree && chdir(gitworktree)) {
-		if (nongit_ok) {
-			if (wt_rel_gitdir && chdir(cwd))
-				die("Cannot come back to cwd");
-			*nongit_ok = 1;
+	offset = len = strlen(cwd);
+	for (;;) {
+		if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
+			break;
+		if (is_git_directory(".")) {
+			inside_git_dir = 1;
+			if (!work_tree_env)
+				inside_work_tree = 0;
+			setenv(GIT_DIR_ENVIRONMENT, ".", 1);
 			return NULL;
 		}
-		if (wt_rel_gitdir)
-			die("Cannot change directory to working tree '%s'"
-				" from $%s", gitworktree, GIT_DIR_ENVIRONMENT);
-		else
-			die("Cannot change directory to working tree '%s'",
-				gitworktree);
-	}
-	if (!getcwd(worktree, sizeof(worktree)-1))
-		die("Unable to read current working directory");
-	strcat(worktree, "/");
-	inside_work_tree = !prefixcmp(cwd, worktree);
-
-	if (gitworktree && inside_work_tree && !prefixcmp(worktree, gitdir) &&
-	    strcmp(worktree, gitdir)) {
-		inside_git_dir = 0;
+		chdir("..");
+		do {
+			if (!offset) {
+				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] != '/');
 	}
 
-	if (!inside_work_tree) {
-		if (chdir(cwd))
-			die("Cannot come back to cwd");
+	inside_git_dir = 0;
+	if (!work_tree_env)
+		inside_work_tree = 1;
+	git_work_tree_cfg = xstrndup(cwd, offset);
+	if (offset == len)
 		return NULL;
-	}
 
-	if (!strcmp(cwd, worktree))
-		return NULL;
-	return cwd+strlen(worktree);
+	/* Make "offset" point to past the '/', and add a '/' at the end */
+	offset++;
+	cwd[len++] = '/';
+	cwd[len] = 0;
+	return cwd + offset;
 }
 
 int git_config_perm(const char *var, const char *value)
@@ -386,6 +340,16 @@ int check_repository_format_version(const char *var, const char *value)
 		repository_format_version = git_config_int(var, value);
 	else if (strcmp(var, "core.sharedrepository") == 0)
 		shared_repository = git_config_perm(var, value);
+	else if (strcmp(var, "core.bare") == 0) {
+		is_bare_repository_cfg = git_config_bool(var, value);
+		if (is_bare_repository_cfg == 1)
+			inside_work_tree = -1;
+	} else if (strcmp(var, "core.worktree") == 0) {
+		if (git_work_tree_cfg)
+			free(git_work_tree_cfg);
+		git_work_tree_cfg = xstrdup(value);
+		inside_work_tree = -1;
+	}
 	return 0;
 }
 
@@ -402,5 +366,16 @@ const char *setup_git_directory(void)
 {
 	const char *retval = setup_git_directory_gently(NULL);
 	check_repository_format();
+
+	/* If the work tree is not the default one, recompute prefix */
+	if (inside_work_tree < 0) {
+		static char buffer[PATH_MAX + 1];
+		if (retval && chdir(retval))
+			die ("Could not jump back into original cwd");
+		char *rel = get_relative_cwd(buffer, PATH_MAX,
+				get_git_work_tree());
+		return rel && *rel ? strcat(rel, "/") : NULL;
+	}
+
 	return retval;
 }
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index ec49966..bea40cb 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -31,9 +31,9 @@ test_rev_parse() {
 test_rev_parse toplevel false false true ''
 
 cd .git || exit 1
-test_rev_parse .git/ false true true .git/
+test_rev_parse .git/ true true false ''
 cd objects || exit 1
-test_rev_parse .git/objects/ false true true .git/objects/
+test_rev_parse .git/objects/ true true false ''
 cd ../.. || exit 1
 
 mkdir -p sub/dir || exit 1
@@ -42,7 +42,7 @@ test_rev_parse subdirectory false false true sub/dir/
 cd ../.. || exit 1
 
 git config core.bare true
-test_rev_parse 'core.bare = true' true false true
+test_rev_parse 'core.bare = true' true false false
 
 git config --unset core.bare
 test_rev_parse 'core.bare undefined' false false true
@@ -50,28 +50,28 @@ test_rev_parse 'core.bare undefined' false false true
 mkdir work || exit 1
 cd work || exit 1
 export GIT_DIR=../.git
-export GIT_CONFIG="$GIT_DIR"/config
+export GIT_CONFIG="$(pwd)"/../.git/config
 
 git config core.bare false
-test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true work/
 
 git config core.bare true
-test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''
 
 git config --unset core.bare
-test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''
+test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true work/
 
 mv ../.git ../repo.git || exit 1
 export GIT_DIR=../repo.git
-export GIT_CONFIG="$GIT_DIR"/config
+export GIT_CONFIG="$(pwd)"/../repo.git/config
 
 git config core.bare false
 test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true ''
 
 git config core.bare true
-test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false true ''
+test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''
 
 git config --unset core.bare
-test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' true false true ''
+test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
 
 test_done
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index aadeeab..7322161 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -33,17 +33,17 @@ mv .git repo.git || exit 1
 
 say "core.worktree = relative path"
 export GIT_DIR=repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 unset GIT_WORK_TREE
 git config core.worktree ../work
 test_rev_parse 'outside'      false false false
 cd work || exit 1
 export GIT_DIR=../repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 test_rev_parse 'inside'       false false true ''
 cd sub/dir || exit 1
 export GIT_DIR=../../../repo.git
-export GIT_CONFIG=$GIT_DIR/config
+export GIT_CONFIG="$(pwd)"/$GIT_DIR/config
 test_rev_parse 'subdirectory' false false true sub/dir/
 cd ../../.. || exit 1
 
@@ -84,9 +84,23 @@ test_rev_parse 'in repo.git'              false true  false
 cd objects || exit 1
 test_rev_parse 'in repo.git/objects'      false true  false
 cd ../work || exit 1
-test_rev_parse 'in repo.git/work'         false false true ''
+test_rev_parse 'in repo.git/work'         false true true ''
 cd sub/dir || exit 1
-test_rev_parse 'in repo.git/sub/dir' false false true sub/dir/
+test_rev_parse 'in repo.git/sub/dir' false true true sub/dir/
 cd ../../../.. || exit 1
 
+test_expect_success 'repo finds its work tree' '
+	(cd repo.git &&
+	 : > work/sub/dir/untracked &&
+	 test sub/dir/untracked = "$(git ls-files --others)")
+'
+
+test_expect_success 'repo finds its work tree from work tree, too' '
+	(cd repo.git/work/sub/dir &&
+	 : > tracked &&
+	 git --git-dir=../../.. add tracked &&
+	 cd ../../.. &&
+	 test sub/dir/tracked = "$(git ls-files)")
+'
+
 test_done
-- 
1.5.3.rc3.94.g3024

Re: [PATCH 4/4] Clean up work-tree handling

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The old version of work-tree support was an unholy mess, barely readable,
> and not to the point.
>
> For example, why do you have to provide a worktree, when it is not used?
> As in "git status".  Now it works.
> ...

Without continuing with negatives, let's try to define the new,
corrected world order.

I do not think the following is exactly what your cleaned-up
version tries to perform, but I am writing this down primarily
to demonstrate the style and the level of detail I expect to
accompany a clean-up patch like this.

----------------------------------------------------------------

Definitions:

 - You can have "checked out" files on the filesystem, and such
   files are said to be in your "work tree".  The directory
   on the filesystem that corresponds to the toplevel entries of
   the index and the tree objects directly contained in the
   commit objects is called "the toplevel of the work tree",
   or simply "work tree" if it is not ambiguous from the
   context.

 - The directory that holds git repository information is called
   "git directory".  This is typically .git directory at the
   toplevel of your work tree, but not necessarily so.

 - You can perform many git operations without a work tree, but
   some operations fundamentally require you to have one
   (e.g. checkout and diff, unless two tree-ishes are given, do
   not make sense without a work tree).

There are four predicates, two interrogators, and two
manipulators:

 - is_inside_git_dir(): this returns true if the $cwd is the git
   directory or its subdirectory. [IS THIS STILL NEEDED???]

 - is_inside_work_tree(): this returns true if the $cwd is
   inside work tree (i.e. either at the toplevel of the work
   tree or its subdirectory).  [NEEDSHELP: is .git in the usual
   layout considered "is_inside_work_tree()"?  Should it?]

 - is_bare_repository(): this returns true if no work tree is
   found.  There is a corresponding function usable from the
   scripts.

 - require_work_tree (shell): this is called by scripts that
   needs to have a work tree to operate, and barfs otherwise.

 - get_git_dir(): this returns the location of the git
   directory.  With GIT_DIR environment variable, or --git-dir
   command line option, you can tell git to use a specific
   directory as the git directory.  Otherwise a directory that
   looks like a git directory and whose name is .git is looked
   for, in the $cwd or its parent directory.  If there is no
   such directory, and if the $cwd looks like a git directory,
   $cwd is the git directory.

 - get_git_work_tree(): this returns the location of the work
   tree; it returns NULL if there is none.  The command line
   option --work-tree, or the environment variable GIT_WORK_TREE
   can specify the location; otherwise, git directory is looked
   for so that its configuration file can be read.  If
   core.worktree is there, that specifies the location.
   Otherwise, if the basename of the git directory is .git, it
   is the parent directory of the git directory.  Otherwise, you
   do not have work tree.

 - set_git_dir(): used to set the git directory, internally to
   handle --git-dir option;

 - set_work_tree(): used to set the git directory, internally to
   handle --work-tree option;

----------------------------------------------------------------

After writing the above down, it strikes me odd that we do not
have a predicate that says "we know the work tree is there".

If a command wants a work tree, and if you are outside the work
tree, then is_inside_work_tree() returns false and
get_git_work_tree() returns non NULL, so that is a good pair of
interface that can be mixed and matched (e.g. you can chdir to
the former to perform the whole tree operation, or refuse to
perform, based on is_inside_work_tree being false, cwd relative
operations).

Re: [PATCH 4/4] Clean up work-tree handling

[Johannes Schindelin]

Hi,

On Tue, 31 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The old version of work-tree support was an unholy mess, barely readable,
> > and not to the point.
> >
> > For example, why do you have to provide a worktree, when it is not used?
> > As in "git status".  Now it works.
> > ...
> 
> Without continuing with negatives, let's try to define the new,
> corrected world order.
> 
> I do not think the following is exactly what your cleaned-up
> version tries to perform, but I am writing this down primarily
> to demonstrate the style and the level of detail I expect to
> accompany a clean-up patch like this.

After reading your description I sink into the ground in shame.  I really 
like the style this has, and agree that something as nice as this should 
have been there.

>  - is_inside_git_dir(): this returns true if the $cwd is the git
>    directory or its subdirectory. [IS THIS STILL NEEDED???]

Hmmm.

>  - is_inside_work_tree(): this returns true if the $cwd is
>    inside work tree (i.e. either at the toplevel of the work
>    tree or its subdirectory).  [NEEDSHELP: is .git in the usual
>    layout considered "is_inside_work_tree()"?  Should it?]

.git/ is not considered part of the work tree, even if it is _physically_ 
there.

> After writing the above down, it strikes me odd that we do not
> have a predicate that says "we know the work tree is there".
>
> If a command wants a work tree, and if you are outside the work
> tree, then is_inside_work_tree() returns false and
> get_git_work_tree() returns non NULL, so that is a good pair of
> interface that can be mixed and matched (e.g. you can chdir to
> the former to perform the whole tree operation, or refuse to
> perform, based on is_inside_work_tree being false, cwd relative
> operations).

Yes.  Builtins which need a working tree expect to start at the toplevel 
of the work tree (which I like to call "working directory", because it is 
described as such in the glossary AFAIR), and therefore they chdir() to 
the toplevel in any case.

I'll be running "master"+worktree+branch-newdir for the remainder of the 
1.5.3-rc period, to be sure that all works as intended.

Ciao,
Dscho

Re: [PATCH 4/4] Clean up work-tree handling

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> I do not think the following is exactly what your cleaned-up
>> version tries to perform, but I am writing this down primarily
>> to demonstrate the style and the level of detail I expect to
>> accompany a clean-up patch like this.
>
> After reading your description I sink into the ground in shame.  I really 
> like the style this has, and agree that something as nice as this should 
> have been there.

Not too late.  There always is a room for Documentation/
improvements ;-)

Seriously, I think config.txt mentions GIT_WORK_TREE and GIT_DIR
and git.txt has separate sections for environment and config,
but I do not see a good section that gives a comprehensive view
of how they work together to affect your git experience.
Perhaps between File/Directory Structure and Terminology
sections we would want to have a description of how repository
and worktree relate to each other.

Re: [PATCH 4/4] Clean up work-tree handling

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> diff --git a/builtin-ls-files.c b/builtin-ls-files.c
> index 61577ea..d36181a 100644
> --- a/builtin-ls-files.c
> +++ b/builtin-ls-files.c
> @@ -469,9 +469,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
>  		break;
>  	}
>  
> -	if (require_work_tree &&
> -			(!is_inside_work_tree() || is_inside_git_dir()))
> -		die("This operation must be run in a work tree");
> +	if (require_work_tree && !is_inside_work_tree()) {
> +		const char *work_tree = get_git_work_tree();
> +		if (!work_tree || chdir(work_tree))
> +			die("This operation must be run in a work tree");
> +	}
>  
>  	pathspec = get_pathspec(prefix, argv + i);
>  

Similarly to this change, I am wondering if we would want to fix
verify_non_filename() in setup.c, which does this:

/*
 * Verify a filename that we got as an argument for a pathspec
 * entry. Note that a filename that begins with "-" never verifies
 * as true, because even if such a filename were to exist, we want
 * it to be preceded by the "--" marker (or we want the user to
 * use a format like "./-filename")
 */
void verify_filename(const char *prefix, const char *arg)
{
...
}

/*
 * Opposite of the above: the command line did not have -- marker
 * and we parsed the arg as a refname.  It should not be interpretable
 * as a filename.
 */
void verify_non_filename(const char *prefix, const char *arg)
{
        const char *name;
        struct stat st;

        if (!is_inside_work_tree() || is_inside_git_dir())
                return;
        if (*arg == '-')
                return; /* flag */
        name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
        if (!lstat(name, &st))
                die("ambiguous argument '%s': both revision and filename\n"
                    "Use '--' to separate filenames from revisions", arg);
        if (errno != ENOENT)
                die("'%s': %s", arg, strerror(errno));
}

At this point, we are given an ambiguous parameter, that could
be naming a path in the work tree.  If we are not in the work
tree, then it is understandable that we do not have to barf.
The other check (i.e. "|| is_inside_git_dir()") does not hurt
(iow, it is not an incorrect check per-se), because if you did
"cd .git && git log HEAD" then the HEAD parameter cannot be
naming the path ".git/HEAD" in the work tree, but (1) that is
already covered by .git/ being "outside of work tree", and (2)
it is not something this function wants to check anyway
(i.e. "can the parameter be naming a file in the work tree?").

Am I mistaken and/or confused?

Re: [PATCH 4/4] Clean up work-tree handling

[Johannes Schindelin]

Hi,

On Wed, 1 Aug 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/builtin-ls-files.c b/builtin-ls-files.c
> > index 61577ea..d36181a 100644
> > --- a/builtin-ls-files.c
> > +++ b/builtin-ls-files.c
> > @@ -469,9 +469,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
> >  		break;
> >  	}
> >  
> > -	if (require_work_tree &&
> > -			(!is_inside_work_tree() || is_inside_git_dir()))
> > -		die("This operation must be run in a work tree");
> > +	if (require_work_tree && !is_inside_work_tree()) {
> > +		const char *work_tree = get_git_work_tree();
> > +		if (!work_tree || chdir(work_tree))
> > +			die("This operation must be run in a work tree");
> > +	}
> >  
> >  	pathspec = get_pathspec(prefix, argv + i);
> >  
> 
> Similarly to this change, I am wondering if we would want to fix
> verify_non_filename() in setup.c, which does this:
> 
> /*
>  * Verify a filename that we got as an argument for a pathspec
>  * entry. Note that a filename that begins with "-" never verifies
>  * as true, because even if such a filename were to exist, we want
>  * it to be preceded by the "--" marker (or we want the user to
>  * use a format like "./-filename")
>  */
> void verify_filename(const char *prefix, const char *arg)
> {
> ...
> }
> 
> /*
>  * Opposite of the above: the command line did not have -- marker
>  * and we parsed the arg as a refname.  It should not be interpretable
>  * as a filename.
>  */
> void verify_non_filename(const char *prefix, const char *arg)
> {
>         const char *name;
>         struct stat st;
> 
>         if (!is_inside_work_tree() || is_inside_git_dir())
>                 return;
>         if (*arg == '-')
>                 return; /* flag */
>         name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
>         if (!lstat(name, &st))
>                 die("ambiguous argument '%s': both revision and filename\n"
>                     "Use '--' to separate filenames from revisions", arg);
>         if (errno != ENOENT)
>                 die("'%s': %s", arg, strerror(errno));
> }
> 
> At this point, we are given an ambiguous parameter, that could
> be naming a path in the work tree.  If we are not in the work
> tree, then it is understandable that we do not have to barf.
> The other check (i.e. "|| is_inside_git_dir()") does not hurt
> (iow, it is not an incorrect check per-se), because if you did
> "cd .git && git log HEAD" then the HEAD parameter cannot be
> naming the path ".git/HEAD" in the work tree, but (1) that is
> already covered by .git/ being "outside of work tree", and (2)
> it is not something this function wants to check anyway
> (i.e. "can the parameter be naming a file in the work tree?").
> 
> Am I mistaken and/or confused?

I think you are completely right.  Inside a bare repository, "git log 
FETCH_HEAD" should not need to complain.  And I think that the 
"is_inside_git_dir()" could be _replaced_ by "!is_inside_work_tree()", 
since that is the intent of that call.

Ciao,
Dscho

Re: [PATCH 0/9] work-tree clean ups

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> There is not really much that can be done about step 6/9: if we are in a 
> work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
> this does not break git-clean, as a work tree is a work tree is a work 
> tree.  If the user was stupid enough to specify the same directory as 
> GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
> tool, and you can harm yourself with it.  Tough.)

I think we might have a slight misunderstanding.  The "clean"
issue that was raised in an ancient thread was this sequence:

	$ git init
        $ cd .git
        $ git clean

It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
there).  The point was that not all subdirectories of the
toplevel (i.e. the directory on the filesystem that corresponds
to the root level of your index entries and trees contained in
your commits) were safe to answer yes when asked "are we safe to
perform this worktree oriented command here".  Because no trees
nor index would have .git/ subdirectory tracked, "git clean"
will happily remove everything under .git/ (which is $cwd in the
above sequence).

I personally feel that the above sequence is a pilot error and
not worth worrying about, but as people wanted to have that
extra safety, and as we added that (arguably stupid) safety way
before the WORK_TREE stuff, we should mention it if we are
changing the behaviour and lifting it with this patch series.

> Note: if you are in a bare repository (a repository which either says 
> "core.bare = false" in the config, or which is a direct ancestor 
> directory, i.e. ../[...]/.. of the current working directory) there will 
> _not_ be an automatic working directory assignment.  You will be operating 
> _without_ any work tree, unless you specify one.

Sorry, I cannot interpret the condition part of the sentence,
nor "There will _not_ be an automatic assignment" part.

By the latter, do you mean to say your $cwd is assumed to be the
top of the working tree unless GIT_WORK_TREE or core.worktree,
if you are in a bare repository?  Or it is assumed that you do
not have a worktree and worktree oriented operations that
require a worktree such as "git diff-files" and "git status"
will fail?

> I somehow feel that core.bare = true weighs more than core.worktree = 
> /some/thing, and therefore I implemented it that way, but hey, if enough 
> people disagree, then I'll change it.

Personally, I think

 [core]
     bare = true
     worktree = /some/where

is a configuration error, but probably I am missing a useful use
case for such a configuration?

> IMHO we should (probably after 1.5.3) change setup_git_directory_gently() 
> to call check_repository_format() in every return path, so that we 
> ascertain that the current repository is recent enough.  Because that 
> function now checks also if the repo is bare, and if it has a worktree 
> set, in addition to ensuring a valid repository.

Agreed; gently() is there primarily because some commands do not
mind not having a git repository at all and if we do have a
repository to work against we probably should do the same checks
as setup_git_directory() would.

Re: [PATCH 0/9] work-tree clean ups

[Johannes Schindelin]

Hi,

On Tue, 31 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > There is not really much that can be done about step 6/9: if we are in a 
> > work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
> > this does not break git-clean, as a work tree is a work tree is a work 
> > tree.  If the user was stupid enough to specify the same directory as 
> > GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
> > tool, and you can harm yourself with it.  Tough.)
> 
> I think we might have a slight misunderstanding.  The "clean"
> issue that was raised in an ancient thread was this sequence:
> 
> 	$ git init
>         $ cd .git
>         $ git clean
> 
> It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
> there).

I very much _did_ mean that case.  When "git clean" is run in ".git/", it 
should not say that it is in the working tree.  But I guess that my patch 
series is not really looking out for that;  I'll make that an add-on 
patch.  (But that _will_ have to wait until tomorrow afternoon.)

Ciao,
Dscho

Re: [PATCH 0/9] work-tree clean ups

[Johannes Schindelin]

Hi,

On Wed, 1 Aug 2007, Johannes Schindelin wrote:

> On Tue, 31 Jul 2007, Junio C Hamano wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > There is not really much that can be done about step 6/9: if we are in a 
> > > work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
> > > this does not break git-clean, as a work tree is a work tree is a work 
> > > tree.  If the user was stupid enough to specify the same directory as 
> > > GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
> > > tool, and you can harm yourself with it.  Tough.)
> > 
> > I think we might have a slight misunderstanding.  The "clean"
> > issue that was raised in an ancient thread was this sequence:
> > 
> > 	$ git init
> >         $ cd .git
> >         $ git clean
> > 
> > It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
> > there).
> 
> I very much _did_ mean that case.  When "git clean" is run in ".git/", it 
> should not say that it is in the working tree.  But I guess that my patch 
> series is not really looking out for that;  I'll make that an add-on 
> patch.  (But that _will_ have to wait until tomorrow afternoon.)

I should not have answered so early.

In setup.c, I put in a comment that explains clearly where (in the absence 
of GIT_DIR) setup_git_directory_gently() looks for the git directory:

        /*
         * Test in the following order (relative to the cwd):
         * - .git/
         * - ./ (bare)
         * - ../.git/
         * - ../ (bare)
         * - ../../.git/
         *   etc.
         */

At least I hope that this explanation is clear.

So what happens in this case:

	$ git init
	$ cd .git
	$ git clean

In setup_git_directory_gently(), it is tested first if there is a 
subdirectory .git/.  No, none.  Then it is tested if "." is a git 
directory.  Yes!  So, work_tree is set to NULL tentatively (to be 
overridden by either core.worktree or GIT_WORK_TREE), and it is assumed to 
be bare (also subject to overriding).  So all is well!

You might have noticed that I left out --work-tree= handling; when 
--work-tree=<something> is specified, GIT_WORK_TREE is _forced_ to the new 
value, so it is literally handled by the same code as GIT_WORK_TREE.

Ciao,
Dscho