Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE

From: Jeff King <peff@peff.net>
Date: Dec 29, 2009, 21:50 UTC
Message-ID: <20091229215015.GA1529@coredump.intra.peff.net>
In-Reply-To: <1a04eebf0912291309u7a222d9ch7e0926d30a5899b7@mail.gmail.com>

On Tue, Dec 29, 2009 at 01:09:20PM -0800, Fyn Fynn wrote:

Show 5 quoted lines

> But it is more likely that the breaking of the original action between
> 1.6.4 and 1.6.6 came about as an untended consequence of 952dfc6,
> which oversimplified by assuming that the worktree can only be found
> if we're inside it, ignoring the possibility that GIT_WORK_TREE was
> provided.

Yes, it is an unintended breakage. We discussed the possibility of automatically moving to the work tree when inside the repo, but decided it could be done later, as other builtins which use NEEDS_WORK_TREE (e.g., clean) already fail in that instance.

However, I did not take into account that those commands fail only with automatic repo detection; they _do_ work with GIT_WORK_TREE. So instead of just dying, we need to be emulating the code in git.c which sets up the work tree (and which will die itself if we run into an error).

So I think what we really want is this:

diff --git a/builtin-reset.c b/builtin-reset.c
index 11d1c6e..e4418bc 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -286,10 +286,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if ((reset_type == HARD || reset_type == MERGE)
-	    && !is_inside_work_tree())
-		die("%s reset requires a work tree",
-		    reset_type_names[reset_type]);
+	if (reset_type == HARD || reset_type == MERGE)
+		setup_work_tree();
 
 	/* Soft reset does not touch the index file nor the working tree
 	 * at all, but requires them in a good order.  Other resets reset

I'll try to write a few tests and make sure that's sane.

-Peff