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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html