Re: bug with .git file and aliases

[Geoffrey Irving <irving@naml.us>]

On Mon, Aug 10, 2009 at 7:05 PM, Johannes
Schindelin<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 10 Aug 2009, Geoffrey Irving wrote:
>
>> On Mon, Jul 20, 2009 at 11:21 AM, Jeff King<peff@peff.net> wrote:
>> > On Mon, Jul 20, 2009 at 09:54:12AM -0400, Geoffrey Irving wrote:
>> >
>> >> git 1.6.3.3 has a bug related to .git file support and aliases.
>> >> Specifically, if you make an alias for status and call it from a
>> >> subdirectory, git status chdirs into the true .git dir but then
>> >> chdir's back to the wrong place in order to run the lstats for status.
>> >>  The result is that git status thinks all files have disappeared.
>> >
>> > Yeah, this is a known problem. The problem is that the 'git' wrapper
>> > sets up the environment only partially when running aliases, and then
>> > the resulting command ends up confused about where the worktree is. I
>> > really don't remember the specifics, but you can probably find some
>> > discussion in the list archives.  Fixing it, IIRC, required some
>> > refactoring of the setup code (which I had hoped to get to at some
>> > point, but I am way behind on my git todo list).
>>
>> The attached patch fixes the bug for me.  I'll leave it to others to
>> determine whether this is a good way to fix the problem.
>
> Note that you made it particularly hard to comment on your patch by not
> granting us the wish stated in Documentation/SubmittingPatches, namely to
> inline your patch.
>
> I'll just forego inlining it myself, as I am way past my bed-time and
> cannot be bothered.

Oops.  Here's the inlined patch with offset fixed, for others:

From ec47aa09e5bc8d9a8c07cca9f8ef17a9898819c1 Mon Sep 17 00:00:00 2001
From: Geoffrey Irving <irving@naml.us>
Date: Mon, 10 Aug 2009 15:59:21 -0400
Subject: [PATCH] setup.c: fix work tree setup for .git-files and aliases

When .git-files and aliases are used together, the setup machinery
gets confused and ends up with the wrong work_tree.  Specifically,
git_work_tree_cfg is set to the correct value first, but set_work_tree
resets git_work_tree_cfg to the current directory, which (at least in
this case) is incorrect.

set_work_tree now detects this case by checking to see if
git_work_tree_cfg is already set.  If so, it leaves git_work_tree_cfg
unchanged and instead uses the current directory to compute and return
the correct prefix (where we are relative to the work tree).

Signed-off-by: Geoffrey Irving <irving@naml.us>
---
 setup.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index e3781b6..97f7eb1 100644
--- a/setup.c
+++ b/setup.c
@@ -198,13 +198,24 @@ int is_inside_work_tree(void)
 static const char *set_work_tree(const char *dir)
 {
 	char buffer[PATH_MAX + 1];

 	if (!getcwd(buffer, sizeof(buffer)))
 		die ("Could not get the current working directory");
-	git_work_tree_cfg = xstrdup(buffer);
 	inside_work_tree = 1;

-	return NULL;
+	if (!git_work_tree_cfg) {
+		git_work_tree_cfg = xstrdup(buffer);
+		return NULL;
+	} else {
+		size_t offset = strlen(git_work_tree_cfg);
+		if (memcmp(git_work_tree_cfg, buffer, offset)
+			|| (buffer[offset] && buffer[offset] != '/'))
+			die ("fatal: not inside work tree (should not happen)");
+		if (!buffer[offset] || !buffer[offset+1])
+			return NULL;
+		return xstrdup(strcat(buffer + offset + 1, "/"));
+	}
 }

 void setup_work_tree(void)
-- 
1.6.3.3

> However, I think that it is necessary to comment on your patch.
>
> There is a few style issues, such as declaring offset outside of the
> block that is the only user, and there is the issue that you go out of
> your way to append a slash if you're resetting the work tree, but not when
> not resetting it.
>
> But the bigger issue is that you now broke overriding the work tree via
> the command line.
>
> The proper fix, of course, is to avoid calling the function with the wrong
> path to begin with.

I'm happy that the correct fix is obvious, and apologize for missing it.

Geoffrey