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

[Jeff King <peff@peff.net>]

On Tue, May 03, 2011 at 04:33:41PM -0700, Junio C Hamano wrote:

> 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.

Yeah, I ran into this just recently when converting merge-one-file to
use git-sh-setup. I was surprised that require_work_tree also required
one to be inside it. My solution was to call cd_to_toplevel first, as
you noted.

> I think the right solution would be to apply the attached patch; and then
> audit all the callers that call "require_work_tree" to see if any of them
> meant "No, it is not Ok just to have working tree somewhere! I want you to
> be IN that working tree when you call me", and convert them to call the
> new require_to_be_in_work_tree instead.

Your proposed semantics for require_work_tree make much more sense to
me, but I worry about compatibility. We can audit our in-tree scripts,
but git-sh-setup is part of the recommended API for external scripts,
no? This change might break those scripts, so we would need to do the
usual deprecation thing. I'm also concerned that the breakage might be
pretty severe. As in, not just "script doesn't work" but "script
silently produces incorrect results" or "script deletes data".

For example, the merge-one-file bug I fixed recently was silently
producing bogus merges because of a confusion over whether it was in the
workdir. Something like "git clean" would be even worse.

-Peff