Re: [PATCH 4/4] Clean up work-tree handling

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 1, 2007, 11:46 UTC
Message-ID: <Pine.LNX.4.64.0708011239090.14781@racer.site>
In-Reply-To: <7v3az3deac.fsf@assigned-by-dhcp.cox.net>

Hi,

On Tue, 31 Jul 2007, Junio C Hamano wrote:

Show 16 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The old version of work-tree support was an unholy mess, barely readable,
> > and not to the point.
> >
> > For example, why do you have to provide a worktree, when it is not used?
> > As in "git status".  Now it works.
> > ...
> 
> Without continuing with negatives, let's try to define the new,
> corrected world order.
> 
> I do not think the following is exactly what your cleaned-up
> version tries to perform, but I am writing this down primarily
> to demonstrate the style and the level of detail I expect to
> accompany a clean-up patch like this.

After reading your description I sink into the ground in shame. I really like the style this has, and agree that something as nice as this should have been there.

>  - is_inside_git_dir(): this returns true if the $cwd is the git
>    directory or its subdirectory. [IS THIS STILL NEEDED???]

Hmmm.

>  - is_inside_work_tree(): this returns true if the $cwd is
>    inside work tree (i.e. either at the toplevel of the work
>    tree or its subdirectory).  [NEEDSHELP: is .git in the usual
>    layout considered "is_inside_work_tree()"?  Should it?]

.git/ is not considered part of the work tree, even if it is _physically_ there.

Show 10 quoted lines

> After writing the above down, it strikes me odd that we do not
> have a predicate that says "we know the work tree is there".
>
> If a command wants a work tree, and if you are outside the work
> tree, then is_inside_work_tree() returns false and
> get_git_work_tree() returns non NULL, so that is a good pair of
> interface that can be mixed and matched (e.g. you can chdir to
> the former to perform the whole tree operation, or refuse to
> perform, based on is_inside_work_tree being false, cwd relative
> operations).

Yes. Builtins which need a working tree expect to start at the toplevel of the work tree (which I like to call "working directory", because it is described as such in the glossary AFAIR), and therefore they chdir() to the toplevel in any case.

I'll be running "master"+worktree+branch-newdir for the remainder of the 1.5.3-rc period, to be sure that all works as intended.

Ciao, Dscho