On Wed, Jun 14, 2017 at 02:36:14AM -0400, Jeff King wrote: > > This patch looks good to me, apart from the perceived wording nits. > > Thanks. I'll re-roll with a few tweaks based on your feedback. Here it is. It just changes the wording and fixes the test as you suggested. [1/2]: add: warn when adding an embedded repository [2/2]: t: move "git add submodule" into test blocks I had some thoughts on adding a third patch to cover the case where .gitmodules has already been updated. But I couldn't figure out how to make it work. The patch I came up with is below: diff --git a/builtin/add.c b/builtin/add.c index d7cab1bd8..d20052462 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -322,6 +322,8 @@ static void check_embedded_repo(const char *path) return; if (!ends_with(path, "/")) return; + if (is_submodule_initialized(path)) + return; /* Drop trailing slash for aesthetics */ strbuf_addstr(&name, path); diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh index f2e7df59c..aee52b218 100755 --- a/t/t7414-submodule-mistakes.sh +++ b/t/t7414-submodule-mistakes.sh @@ -34,4 +34,14 @@ test_expect_success 'submodule add does not warn' ' test_i18ngrep ! warning stderr ' +test_expect_success 'making your own submodule does not warn' ' + test_when_finished "git rm --cached -f embed && + git rm -f .gitmodules" && + git config -f .gitmodules submodule.embed.path embed && + git config -f .gitmodules submodule.embed.url \ + git://example.com/embed.git && + git add embed 2>stderr && + test_i18ngrep ! warning stderr +' + test_done But that doesn't quite work. Because the submodule data is just in .gitmodules, it's not actually "initialized". I think that might work if instead it was: git config submodule.embed.url ... but I'm not clear on whether that should work, or if it's even something somebody would want to do. So I decided to punt on the whole thing and let somebody work on it who a) knows more about how submodules work or b) has some not-quite-submodule use case for gitlinks that they want to have work (without a warning). -Peff