Re: [PATCH] require_work_tree: Look for top-level instead of is-inside-work-tree

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 2, 2010, 17:46 UTC
Message-ID: <7v8w4onc0l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4C56D83F.3050507@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 8 quoted lines

> An alternative which does not change the established behavior of
> require_work_tree would be changing the order of require_work_tree and
> cd_to_top_level in the callers where possible along the lines of
>
> http://mid.gmane.org/96abf622ca2cf92998ce4ed393ccaa75d95dd9a8.1279112025.git.git@drmicha.warpmail.net
>
> which got lost somehow. (The other callers, as mentioned by Junio, would
> need to be changed differently, e.g. by moving cd_to... earlier.)

Doesn't it sound stupid to "cd-to-toplevel" and then "require-work-tree"?

If you can go to the top-level, and once you successfully got there, you already _know_ that you have a work tree (and also you already know at that point you are in the work tree). The reason why "require-work-tree" has been placed before "cd-to-toplevel" is exactly for that purpose, I think. It is possible that some callers wanted to "require-work-tree" to mean "I want you to not just _have_ a work tree, but actually be _in_ it", but I somehow doubt it. It is more like "I am going to ask you to go to the top, but let's make sure that you do have a top before doing so", I think.

I on the other hand do not think it is wrong to lose the existing calls to require-work-tree if you know that you are going to call cd-to-toplevel before doing any git operation that needs to have a work-tree, though.

> Another problem I noticed back then (I was away since) was that a
> relative GIT_WORK_TREE is left in place after a cd_to_top_level and
> messes things up completely - it does not seem to be relative to
> GIT_DIR. So, there seems to be more to fix in this area.

I agree; I don't think GIT_WORK_TREE was designed to be anything but an absolute path to begin with. If a command chdir's around and exports the environment to its hooks and subcommands, it should be prepared to adjust it before doing so.