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

Junio C Hamano venit, vidit, dixit 04.05.2011 01:33:

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 may have been that one that year or another...

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?

Same thoughts as Jeff. git-sh-setup is explicitly meant to be used by
scripters, and the users in git.git serve as an example how to use it.
If that usage changes it requires a long migration plan.

The only thing I can imagine doing right now is changing
require_work_tree() to actually cd to toplevel when possible, so that
(like before) on success we're really within. But that changes cwd, of
course. In summary, a require_work_tree() now can have three assumptions
when it returns with success:

- we have a worktree
- we are within worktree
- cwd has not changed

I'd rather break the last one than the second one, but breaking any may
be a problem, depending on the caller.

Michael