Re: [RFC PATCH 6/6] add: reject nested repositories

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

Jeff King <peff@peff.net> writes:

>> If we are keeping the escape hatch, it would make sense to actually
>> use that escape hatch to protect existing "git add" with that,
>> instead of turning them into "git submodule add" and then adjust the
>> tests for the consequences (i.e. "submodule add" does more than what
>> "git add [--no-warn-embedded-repo]" would), at least for these tests
>> in [3,4,5/6].
>
> Good point. I did not really look at the test modifications, but
> anywhere that is triggering the current warning is arguably a good spot
> to be using --no-warn-embedded-repo already. It is simply that the test
> did not bother to look at their noisy stderr. And such a modification is
> obviously correct, as there are no further implications for the test.

I did not mean that no "git add" that create a gitlink in existing
tests should be made into "git submodule add".  The ones that
clearly wanted to set up tests to see what happens in a top-level
with a subproject may become more realistic tests by switching to
"git submodule add" and updating the expected "git diff HEAD" output
to include a newly created .gitmodules file.  But some of the tests
are merely to see what happens with an index with a gitlink in it,
and "add --no-warn" would be more appropriate for them.

>> Also I do not think it is too late for a more natural UI, e.g.
>> "--allow-embedded-repo=[yes/no/warn]", to deprecate the
>> "--[no-]warn-*" option.
>
> True. We have to keep the existing form for backwards compatibility, but
> we can certainly add a new one.
>
> I kind of doubt that --allow-embedded-repo=warn is useful, though. If a
> caller knows what it is doing is OK, then it would say "yes". And
> otherwise, you'd want "no". There is no situation where a caller is
> unsure.

Yeah, if the default becomes "no", then there isn't much point,
other than just for completeness, to have "warn" as a choice.

Thanks.