From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 02 Aug 2010 17:46:18 GMT
Subject: Re: [PATCH] require_work_tree: Look for top-level instead of is-inside-work-tree
Message-ID: <7v8w4onc0l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4C56D83F.3050507@drmicha.warpmail.net>

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

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