Re: [PATCH 0/9] work-tree clean ups

Johannes Schindelin [off-list ref] writes:

There is not really much that can be done about step 6/9: if we are in a 
work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
this does not break git-clean, as a work tree is a work tree is a work 
tree.  If the user was stupid enough to specify the same directory as 
GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
tool, and you can harm yourself with it.  Tough.)

I think we might have a slight misunderstanding.  The "clean"
issue that was raised in an ancient thread was this sequence:

	$ git init
        $ cd .git
        $ git clean

It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
there).  The point was that not all subdirectories of the
toplevel (i.e. the directory on the filesystem that corresponds
to the root level of your index entries and trees contained in
your commits) were safe to answer yes when asked "are we safe to
perform this worktree oriented command here".  Because no trees
nor index would have .git/ subdirectory tracked, "git clean"
will happily remove everything under .git/ (which is $cwd in the
above sequence).

I personally feel that the above sequence is a pilot error and
not worth worrying about, but as people wanted to have that
extra safety, and as we added that (arguably stupid) safety way
before the WORK_TREE stuff, we should mention it if we are
changing the behaviour and lifting it with this patch series.

Note: if you are in a bare repository (a repository which either says 
"core.bare = false" in the config, or which is a direct ancestor 
directory, i.e. ../[...]/.. of the current working directory) there will 
_not_ be an automatic working directory assignment.  You will be operating 
_without_ any work tree, unless you specify one.

Sorry, I cannot interpret the condition part of the sentence,
nor "There will _not_ be an automatic assignment" part.

By the latter, do you mean to say your $cwd is assumed to be the
top of the working tree unless GIT_WORK_TREE or core.worktree,
if you are in a bare repository?  Or it is assumed that you do
not have a worktree and worktree oriented operations that
require a worktree such as "git diff-files" and "git status"
will fail?

I somehow feel that core.bare = true weighs more than core.worktree = 
/some/thing, and therefore I implemented it that way, but hey, if enough 
people disagree, then I'll change it.

Personally, I think

 [core]
     bare = true
     worktree = /some/where

is a configuration error, but probably I am missing a useful use
case for such a configuration?

IMHO we should (probably after 1.5.3) change setup_git_directory_gently() 
to call check_repository_format() in every return path, so that we 
ascertain that the current repository is recent enough.  Because that 
function now checks also if the repo is bare, and if it has a worktree 
set, in addition to ensuring a valid repository.

Agreed; gently() is there primarily because some commands do not
mind not having a git repository at all and if we do have a
repository to work against we probably should do the same checks
as setup_git_directory() would.

Hi,

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

Johannes Schindelin [off-list ref] writes:

quoted

There is not really much that can be done about step 6/9: if we are in a 
work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
this does not break git-clean, as a work tree is a work tree is a work 
tree.  If the user was stupid enough to specify the same directory as 
GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
tool, and you can harm yourself with it.  Tough.)

I think we might have a slight misunderstanding.  The "clean"
issue that was raised in an ancient thread was this sequence:

	$ git init
        $ cd .git
        $ git clean

It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
there).

I very much _did_ mean that case.  When "git clean" is run in ".git/", it 
should not say that it is in the working tree.  But I guess that my patch 
series is not really looking out for that;  I'll make that an add-on 
patch.  (But that _will_ have to wait until tomorrow afternoon.)

Ciao,
Dscho

Hi,

On Wed, 1 Aug 2007, Johannes Schindelin wrote:

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

quoted

Johannes Schindelin [off-list ref] writes:

quoted

There is not really much that can be done about step 6/9: if we are in a 
work tree: that does not mean that we are _not_ in the git_dir.  (And no, 
this does not break git-clean, as a work tree is a work tree is a work 
tree.  If the user was stupid enough to specify the same directory as 
GIT_DIR and GIT_WORK_TREE, then that is _her_ problem.  Git is a powerful 
tool, and you can harm yourself with it.  Tough.)

I think we might have a slight misunderstanding.  The "clean"
issue that was raised in an ancient thread was this sequence:

	$ git init
        $ cd .git
        $ git clean

It did not involve GIT_DIR (nor GIT_WORK_TREE as it was not even
there).

I very much _did_ mean that case.  When "git clean" is run in ".git/", it 
should not say that it is in the working tree.  But I guess that my patch 
series is not really looking out for that;  I'll make that an add-on 
patch.  (But that _will_ have to wait until tomorrow afternoon.)

I should not have answered so early.

In setup.c, I put in a comment that explains clearly where (in the absence 
of GIT_DIR) setup_git_directory_gently() looks for the git directory:

        /*
         * Test in the following order (relative to the cwd):
         * - .git/
         * - ./ (bare)
         * - ../.git/
         * - ../ (bare)
         * - ../../.git/
         *   etc.
         */

At least I hope that this explanation is clear.

So what happens in this case:

	$ git init
	$ cd .git
	$ git clean

In setup_git_directory_gently(), it is tested first if there is a 
subdirectory .git/.  No, none.  Then it is tested if "." is a git 
directory.  Yes!  So, work_tree is set to NULL tentatively (to be 
overridden by either core.worktree or GIT_WORK_TREE), and it is assumed to 
be bare (also subject to overriding).  So all is well!

You might have noticed that I left out --work-tree= handling; when 
--work-tree=<something> is specified, GIT_WORK_TREE is _forced_ to the new 
value, so it is literally handled by the same code as GIT_WORK_TREE.

Ciao,
Dscho