Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists

Nguyen Thai Ngoc Duy [off-list ref] writes:

Note that this has nothing to do with read_directory() discussion we
had in the notes-merge patch...

I think we are in agreement on that point.

Going back to your example...

$ GIT_DIR=clone2/.git git add clone2/2 3

$ GIT_DIR=clone2/.git git ls-files --stage
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       1
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       3

You probably found a bug here. It is simply wrong to choose not to add
clone2/2, especially without telling the caller anything.

	Side note. I just did this and I am not getting what you saw above.

        $ mkdir -p /var/tmp/j/y && cd /var/tmp/j/y
        $ git init; git init clone2
        $ : >3; : >clone2/2
        $ GIT_DIR=clone2/.git git add clone2/2 3
        $ GIT_DIR=clone2/.git git ls-files
        3
	clone2/2

	The behavour is different when clone2/.git already has commit, and
        whatever codepath that gives these two different behaviour needs
        to be fixed.

By the way, I think I know where you are coming from.

If we think clone2/ and everything underneath belongs to a repository that
is _not_ governed by our GIT_DIR (which usually is .git), it may be nicer
when the user attempts to add clone2/2 (which would normally belong to
clone2/.git) to at least warn about it, or even error out. I would not be
entirely opposed to a change in the behaviour if the above example were
done without GIT_DIR and produced an error, like this:

    $ git add clone2/2 3; echo $?
    error: clone2/2 is outside our repository, possibly governed by clone2/.git
    1
    $ git ls-files
    1

After all, if clone2 were a submodule of our repository, we do notice and
error out an attempt to add clone2/2 to our repository, so if we changed
the way how "git add" behaves to do the above, I can buy an argument that
calls it a bugfix.

When GIT_DIR=clone2/.git is given, however, the caller explicitly declines
the repository discovery. We do not know how the repository we are dealing
with (which we were explicitly told with $GIT_DIR) and a directory whose
name is ".git" under "clone2" we happened to find in read_directory()
relates to each other, especially when our index does not have clone2 as
our submodule.

We however *do* know that our working tree is our current directory, so
it would be wrong to do this:

    $ GIT_DIR=clone2/.git git add clone2/2 3; echo $?
    error: 3 is outside our repository, possibly goverened by .git
    1

The command should just add clone2/2 and 3 as it was told to.

On Sun, Oct 30, 2011 at 2:08 PM, Junio C Hamano [off-list ref] wrote:

       Side note. I just did this and I am not getting what you saw above.

       $ mkdir -p /var/tmp/j/y && cd /var/tmp/j/y
       $ git init; git init clone2
       $ : >3; : >clone2/2
       $ GIT_DIR=clone2/.git git add clone2/2 3
       $ GIT_DIR=clone2/.git git ls-files
       3
       clone2/2

       The behavour is different when clone2/.git already has commit, and
       whatever codepath that gives these two different behaviour needs
       to be fixed.

It's resolve_gitlink_ref() in treat_directory(). I think replacing
that call() with is_git_directory() would fix this problem. We may
want to do the same with remove_dir_recursively().

When GIT_DIR=clone2/.git is given, however, the caller explicitly declines
the repository discovery. We do not know how the repository we are dealing
with (which we were explicitly told with $GIT_DIR) and a directory whose
name is ".git" under "clone2" we happened to find in read_directory()
relates to each other, especially when our index does not have clone2 as
our submodule.

We however *do* know that our working tree is our current directory, so
it would be wrong to do this:

   $ GIT_DIR=clone2/.git git add clone2/2 3; echo $?
   error: 3 is outside our repository, possibly goverened by .git
   1

The command should just add clone2/2 and 3 as it was told to.

I am concerned about clone2/2 in this case, not 3. I guess we can
check if clone2/.git is the repo we are using. If it is, skip it.
-- 
Duy