[RFC] require-work-tree wants more than what its name says
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:09
Subsystem:
the rest · Maintainer:
Linus Torvalds
Somebody tried "git pull" from a random place completely outside the work
tree, while exporting GIT_DIR and GIT_WORK_TREE that are set to correct
places, e.g.
GIT_WORK_TREE=$HOME/git.git
GIT_DIR=$GIT_WORK_TREE/.git
export GIT_WORK_TREE GIT_DIR
cd /tmp
git pull
At the beginning of git-pull, we check "require-work-tree" and then
"cd-to-toplevel". I _think_ the original intention when I wrote the
command was "we MUST have a work tree, our $(cwd) might not be at the
top-level directory of it", and no stronger than that. That check is a
very sensible thing to do before doing cd-to-toplevel. We check that the
place we would want to go exists, and then go there.
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.
I recall there was a discussion sometime last year about this topic, and
vaguely recall somebody proposed to swap the order of cd-to-toplevel and
require-work-tree. While I agree that would sweep the issue under the rug,
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.
Thoughts?
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index aa16b83..0b25f12 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh@@ -141,6 +141,13 @@ cd_to_toplevel () { } require_work_tree () { + if test "z$(git rev-parse --is-bare-repository)" != zfalse + then + die "fatal: $0 cannot be used without a working tree." + fi +} + +require_to_be_in_work_tree () { test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true || die "fatal: $0 cannot be used without a working tree." }