[PATCH] allow setting GIT_WORK_TREE to "no work tree"

In setup_git_directory_gently, we have a special rule
that says "if GIT_DIR is set but GIT_WORK_TREE is not, then
use the current working directory as the work tree." This is
the intended behavior for the user perspective.

However, setup_git_directory_gently sets GIT_DIR itself,
meaning that further setups (either because we are executing
a command via alias, or in a subprocess) will see the
non-existent GIT_WORK_TREE and assume we fall into the
"current working directory is the working tree" codepath.

Instead, we now use a special value of GIT_WORK_TREE to
indicate that we have already checked for a worktree and
that there isn't one, setting it when we set GIT_DIR and
checking for it in the special case path.

The special value is a blank GIT_WORK_TREE; it could be any
value, but this should not conflict with any user values
(and as a bonus, you can now tell git "I don't have a work
tree" with "GIT_WORK_TREE= git", though I suspect the use
case for that is limited).

Signed-off-by: Jeff King <redacted>
---
On Sun, Feb 03, 2008 at 02:59:13AM -0800, Junio C Hamano wrote:

* "[alias] st = status" and "cd .git && git st" (Jeff King)

This turned out to be easier than expected. Note that this patch causes
t1500 to fail; however, the test is at fault, and is already fixed in
the series I just sent out.

 environment.c                 |    9 +++++++--
 setup.c                       |    1 +
 t/t1502-worktree-semantics.sh |   20 ++++++++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100755 t/t1502-worktree-semantics.sh

diff --git a/environment.c b/environment.c
index 18a1c4e..38e8dff 100644
--- a/environment.c
+++ b/environment.c

@@ -90,8 +90,13 @@ const char *get_git_work_tree(void)
 			/* 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));
+		} else if (work_tree) {
+			if (*work_tree)
+				work_tree = xstrdup(
+						make_absolute_path(work_tree));
+			else
+				work_tree = NULL;
+		}
 		initialized = 1;
 		if (work_tree)
 			is_bare_repository_cfg = 0;

diff --git a/setup.c b/setup.c
index adede16..85c0b48 100644
--- a/setup.c
+++ b/setup.c

@@ -309,6 +309,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env)
 				inside_work_tree = 0;
 			setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+			setenv(GIT_WORK_TREE_ENVIRONMENT, "", 1);
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}

diff --git a/t/t1502-worktree-semantics.sh b/t/t1502-worktree-semantics.sh
new file mode 100755
index 0000000..fbc355b
--- /dev/null
+++ b/t/t1502-worktree-semantics.sh

@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='semantics of GIT_WORK_TREE variable'
+. ./test-lib.sh
+
+test_expect_success 'setup working tree' 'touch file'
+
+test_expect_success 'setup alias' 'git config alias.a add'
+
+test_expect_success 'blank GIT_WORK_TREE disallows work-tree commands' '
+	! GIT_WORK_TREE= git add .
+'
+
+test_expect_success 'alias inside working tree works' 'git a .'
+
+test_expect_success 'alias inside .git complains about working tree' '
+	cd .git && ! GIT_CONFIG=config git a .
+'
+
+test_done

-- 
1.5.4.26.g301fb

Jeff King schrieb:

In setup_git_directory_gently, we have a special rule
that says "if GIT_DIR is set but GIT_WORK_TREE is not, then
use the current working directory as the work tree." This is
the intended behavior for the user perspective.

However, setup_git_directory_gently sets GIT_DIR itself,
meaning that further setups (either because we are executing
a command via alias, or in a subprocess) will see the
non-existent GIT_WORK_TREE and assume we fall into the
"current working directory is the working tree" codepath.

Instead, we now use a special value of GIT_WORK_TREE to
indicate that we have already checked for a worktree and
that there isn't one, setting it when we set GIT_DIR and
checking for it in the special case path.

The special value is a blank GIT_WORK_TREE; it could be any
value, but this should not conflict with any user values
(and as a bonus, you can now tell git "I don't have a work
tree" with "GIT_WORK_TREE= git", though I suspect the use
case for that is limited).

Hrm. Unfortunately, on Windows there is no such thing as an empty
environment string. setenv(x, "") *removes* the environment variable.

-- Hannes

On Wed, Feb 06, 2008 at 11:42:15AM +0100, Johannes Sixt wrote:

quoted

The special value is a blank GIT_WORK_TREE; it could be any
value, but this should not conflict with any user values
(and as a bonus, you can now tell git "I don't have a work
tree" with "GIT_WORK_TREE= git", though I suspect the use
case for that is limited).

Hrm. Unfortunately, on Windows there is no such thing as an empty
environment string. setenv(x, "") *removes* the environment variable.

Bleh. Maybe "GIT_WORK_TREE=:"? It doesn't make sense by itself since we
don't try to execute the contents of GIT_WORK_TREE, but it's unlikely to
be used by a user, and I believe there was recent talk of making
"GIT_EDITOR=:" work.

The other option is setting

  GIT_MAGICALLY_SET_GIT_DIR_SO_DONT_ACT_LIKE_THE_USER_DID=1

but I was hoping to avoid that.

-Peff

Hi,

On Wed, 6 Feb 2008, Johannes Sixt wrote:

Jeff King schrieb:

quoted

In setup_git_directory_gently, we have a special rule that says "if 
GIT_DIR is set but GIT_WORK_TREE is not, then use the current working 
directory as the work tree." This is the intended behavior for the 
user perspective.

However, setup_git_directory_gently sets GIT_DIR itself, meaning that 
further setups (either because we are executing a command via alias, 
or in a subprocess) will see the non-existent GIT_WORK_TREE and assume 
we fall into the "current working directory is the working tree" 
codepath.

Instead, we now use a special value of GIT_WORK_TREE to indicate that 
we have already checked for a worktree and that there isn't one, 
setting it when we set GIT_DIR and checking for it in the special case 
path.

The special value is a blank GIT_WORK_TREE; it could be any value, but 
this should not conflict with any user values (and as a bonus, you can 
now tell git "I don't have a work tree" with "GIT_WORK_TREE= git", 
though I suspect the use case for that is limited).

Hrm. Unfortunately, on Windows there is no such thing as an empty 
environment string. setenv(x, "") *removes* the environment variable.

That might be a shortcoming of our implementation of setenv():

-- snip --
cd /git

cat > a1.c << EOF
#include <stdio.h>
#include "compat/setenv.c"
#include "compat/unsetenv.c"

static void p()
{
	const char *abc = getenv("ABC");
	printf("env ABC: %s\n", abc ? abc : "(null)");
}

int main()
{
	p();
	gitsetenv("ABC", "Hello", 1);
	p();
	gitsetenv("ABC", "", 1);
	p();
	gitunsetenv("ABC");
	p();
	return 0;
}
EOF

gcc -DNO_MMAP=1 -I. -Icompat -o a1.exe a1.c

ABC="" ./a1.exe 
-- snap --

This will show

env ABC: 
env ABC: Hello
env ABC: (null)
env ABC: (null)

So it seems that environment variables _can_ be empty.  Just our 
relatively stupid implementation of setenv() does not do it.

Maybe something like compat/unsetenv.c is needed in setenv(), too.

Ciao,
Dscho

Johannes Schindelin schrieb:

Hi,

On Wed, 6 Feb 2008, Johannes Sixt wrote:

quoted

Jeff King schrieb:

quoted

In setup_git_directory_gently, we have a special rule that says "if 
GIT_DIR is set but GIT_WORK_TREE is not, then use the current working 
directory as the work tree." This is the intended behavior for the 
user perspective.

However, setup_git_directory_gently sets GIT_DIR itself, meaning that 
further setups (either because we are executing a command via alias, 
or in a subprocess) will see the non-existent GIT_WORK_TREE and assume 
we fall into the "current working directory is the working tree" 
codepath.

Instead, we now use a special value of GIT_WORK_TREE to indicate that 
we have already checked for a worktree and that there isn't one, 
setting it when we set GIT_DIR and checking for it in the special case 
path.

The special value is a blank GIT_WORK_TREE; it could be any value, but 
this should not conflict with any user values (and as a bonus, you can 
now tell git "I don't have a work tree" with "GIT_WORK_TREE= git", 
though I suspect the use case for that is limited).

Hrm. Unfortunately, on Windows there is no such thing as an empty 
environment string. setenv(x, "") *removes* the environment variable.

That might be a shortcoming of our implementation of setenv():

No, it is not. It's Windows's putenv(), and it's even documented.

-- snip --
cd /git

cat > a1.c << EOF
#include <stdio.h>
#include "compat/setenv.c"
#include "compat/unsetenv.c"

static void p()
{
	const char *abc = getenv("ABC");
	printf("env ABC: %s\n", abc ? abc : "(null)");
}

int main()
{
	p();
	gitsetenv("ABC", "Hello", 1);
	p();
	gitsetenv("ABC", "", 1);
	p();
	gitunsetenv("ABC");
	p();
	return 0;
}
EOF

gcc -DNO_MMAP=1 -I. -Icompat -o a1.exe a1.c

ABC="" ./a1.exe 
-- snap --

This will show

env ABC: 
env ABC: Hello
env ABC: (null)
env ABC: (null)

So it seems that environment variables _can_ be empty.  Just our 
relatively stupid implementation of setenv() does not do it.

Maybe something like compat/unsetenv.c is needed in setenv(), too.

This only shows that, yes, variables _can_ be empty - if the setenv/putenv
implementation is "sane", like MSYS/bash's.

That said, we probably should modify environ directly in gitsetenv().

-- Hannes

Hi,

On Fri, 8 Feb 2008, Johannes Sixt wrote:

Johannes Schindelin schrieb:

quoted

On Wed, 6 Feb 2008, Johannes Sixt wrote:

quoted

Jeff King schrieb:

quoted

In setup_git_directory_gently, we have a special rule that says "if 
GIT_DIR is set but GIT_WORK_TREE is not, then use the current 
working directory as the work tree." This is the intended behavior 
for the user perspective.

However, setup_git_directory_gently sets GIT_DIR itself, meaning 
that further setups (either because we are executing a command via 
alias, or in a subprocess) will see the non-existent GIT_WORK_TREE 
and assume we fall into the "current working directory is the 
working tree" codepath.

Instead, we now use a special value of GIT_WORK_TREE to indicate 
that we have already checked for a worktree and that there isn't 
one, setting it when we set GIT_DIR and checking for it in the 
special case path.

The special value is a blank GIT_WORK_TREE; it could be any value, 
but this should not conflict with any user values (and as a bonus, 
you can now tell git "I don't have a work tree" with "GIT_WORK_TREE= 
git", though I suspect the use case for that is limited).

Hrm. Unfortunately, on Windows there is no such thing as an empty 
environment string. setenv(x, "") *removes* the environment variable.

That might be a shortcoming of our implementation of setenv():

No, it is not. It's Windows's putenv(), and it's even documented.

Yes, that's what I said: our setenv() implementation uses putenv(), which 
introduces that bug (setenv() is not supposed to unset variables, 
unsetenv() is).

That said, we probably should modify environ directly in gitsetenv().

Exactly.

Ciao,
Dscho