On Wed, May 04, 2011 at 07:11:18PM -0700, Junio C Hamano wrote: > >> 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: > 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 -- 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