On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Stefan Beller <sbeller@xxxxxxxxxx> writes: >> >>>> I wonder if the patches mentioned have something to do with the "git >>>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated >>>> repository in some way? > > The same functionality is added in 8745024 (parse_pathspec: support > stripping/checking submodule paths - 2013-07-14) so if it didn't fail > to notice that before 5a76aff1a6 and did after, it's a bug. The bug seems to have existed before. However in the bug we are talking about the nested repo is not a submodule yet. > >>> >>> Which is considered a feature now. Maybe we should add tests for that? >>> >>> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb >> >> That is a bug, plain and simple. Duy any ideas where we went wrong? > > I vaguely recall this symptom. It has something to do with the index, > the check we do requires a gitlink in the index, I think. So if the > gitlink entry is not in the index, our protection line fails. I think > doing all this at pathspec level is wrong. We should wait at least > after read_directory() is done, by then we have a lot more info to > decide. > >> I think we already have code to avoid adding beyond symlinks. >> "git add deep/in/the/tree" should refuse if deep/in is a symbolic >> link (and happens to point at a directory that has the/tree in it). >> We used not to catch that long time ago, but I think we fixed it. >> >> The logic and the places to do the checks for "no, that thing may be >> a directory but is an unrelated repository" should be the same. > -- > Duy Here are some tests to have a clear picture of what is happening: (diff against origin/master) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index f99f674..c9dfa11 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -37,6 +37,22 @@ test_expect_success 'setup - repository in init subdirectory' ' git add a && git commit -m "submodule commit 1" && git tag -a -m "rev-1" rev-1 + ) && + mkdir init_slash1 && + ( + cd init_slash1 && + git init && + echo a >a && + git add a && + git commit -m "first commit" + ) && + mkdir init_slash2 && + ( + cd init_slash2 && + git init && + echo a >a && + git add a && + git commit -m "first commit" ) ' @@ -44,7 +60,30 @@ test_expect_success 'setup - commit with gitlink' ' echo a >a && echo z >z && git add a init z && - git commit -m "super commit 1" + git commit -m "super commit 1" && + git ls-tree HEAD |grep init >actual && + grep "160000 commit" actual +' + +test_expect_success 'setup - commit with gitlink/ in between' ' + echo a >a && + echo z >z && + git add a init_slash1/ z && + git commit -m "super commit 1" && + git ls-tree HEAD |grep init_slash1 >actual && + grep "160000 commit" actual +' + +test_expect_failure 'setup - commit with gitlink/ only' ' + git add init_slash2/ && + git commit -m "super commit 1" && + git ls-tree HEAD |grep init_slash2 >actual && + grep "160000 commit" actual +' + +test_expect_success 'tear down slash tests' ' + rm -rf init_slash* && + git commit -a -m "removing init_slash*" ' test_expect_success 'setup - hide init subdirectory' ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html