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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]