Re: [PATCH] t3700 (add): add failing test for add with submodules

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Mon, Apr 08, 2013 at 03:56:49PM +0530, Ramkumar Ramachandra wrote:
>
>> git add currently goes past submodule boundaries.  Document this bug.
>
> It's not just submodules, but we should not recurse into any
> sub-repository. If I have an unrelated Meta/ repository, I should not be
> able to "git add Meta/foo", whether I have used "git submodule" or not.
>
> This topic came about 2 years ago, and I had a quick-and-dirty patch:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/170937/focus=171040
>
> I do not recall anything about the patch at this point (i.e., whether it
> was the right thing), but maybe it is a good starting point for somebody
> to look into it.
>
>> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
>> index 874b3a6..a1ea050 100755
>> --- a/t/t3700-add.sh
>> +++ b/t/t3700-add.sh
>> @@ -310,4 +310,18 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
>>  	test_i18ncmp expect.err actual.err
>>  '
>>  
>> +test_expect_failure 'git add should not go past submodule boundaries' '
>> +	mkdir submodule_dir &&
>> +	(
>> +		cd submodule_dir &&
>> +		git init &&
>> +		cat >foo <<-\EOF &&
>> +		Some content
>> +		EOF
>> +		git add foo &&
>> +		git commit -a -m "Add foo"
>> +	) &&
>> +	git add submodule_dir/foo
>> +'
>
> That is not actually a submodule, but rather just a repo that happens to
> be inside our working tree. 

I think we should treat it as a submodule-to-be, waiting for the
user to run "git add submodule_dir".

If it is a file in the working tree of an unrelated and separate
repository, it still is wrong to allow it to be added to our
repository, no?

If we had "git add submodule_dir" before the last "git add", as you
pointed out, we should already error out.

> I know the distinction is subtle, but according to the thread I
> linked to above, we may actually treat paths with gitlinked index
> entries separately already (I did not try it, though).