Re: [RFC] require-work-tree wants more than what its name says

From: Jeff King <peff@peff.net>
Date: May 5, 2011, 04:23 UTC
Message-ID: <20110505042319.GA10243@sigill.intra.peff.net>
In-Reply-To: <7v62ppdhh5.fsf@alter.siamese.dyndns.org>

On Wed, May 04, 2011 at 07:11:18PM -0700, Junio C Hamano wrote:

Show 6 quoted lines

> >> I am Ok with renaming the thing to "require_work_tree_exists", and all the
> >> worry will go away.
> >
> > Yeah, that seems like a fine solution to me.
> 
> Ok, then here is a reroll.

Looks sane. A few comments:

Show 9 quoted lines

> But the implementation of require_work_tree we have today is quite
> different.  I don't have energy to dig the history, but currently it says:
> 
>  	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
>  	die "fatal: $0 cannot be used without a working tree."
> 
> Which is completely bogus.  Even though we may happen to be just outside
> of it right now, we may have a working tree that we can cd_to_toplevel
> back to.

AFAICT, it has been that way since 7ae3df8 (Use new semantics of is_bare/inside_git_dir/inside_work_tree, 2007-06-03), which is around the time GIT_WORK_TREE came into existence.

So I think it was a matter of being overly strict then, either accidentally, or because it matched the prior semantics (which is that there either was _no_ work tree, or you were in it).

> Add a function "require_work_tree_exists" that implements the check
> this function originally intended (this is so that third-party scripts
> that rely on the current behaviour do not have to get broken), and
> update all the in-tree users to use it.

Overall, I think this is a good thing to do. And for the existing working cases (i.e., when you are in the repo) it should be a no-op. It may be worth doing some basic sanity tests to make sure that the scripts properly work when GIT_DIR and GIT_WORK_TREE point elsewhere from the current working directory. But I confess I am not excited about doing the work to write those tests.

-Peff