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
