git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/2] warn when adding an embedded repository

From
Jeff King <peff@peff.net>
Date
Jun 14, 2017, 10:54 UTC
Message-ID
<20170614105437.cgdlykmtocdj5vzw@sigill.intra.peff.net>
In-Reply-To
<20170614063614.a34ovimjpz2g24qe@sigill.intra.peff.net>
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
Previous: Jeff KingNext: Jeff King
Message 15 of 23 in “[BUG?] gitlink without .gitmodules no longer fails recursive clone”
  1. Jeff KingJun 6, 2017
  2. Stefan BellerJun 6, 2017
  3. Brandon WilliamsJun 6, 2017
  4. Jeff KingJun 6, 2017
  5. Jeff KingJun 9, 2017
  6. Junio C HamanoJun 10, 2017
  7. Jeff KingJun 10, 2017
  8. Junio C HamanoJun 10, 2017
  9. Stefan BellerJun 12, 2017
  10. Jeff KingJun 13, 2017
  11. 1/2 add: warn when adding an embedded repositoryJeff King, Jun 13, 2017
  12. Stefan BellerJun 13, 2017
  13. Brandon WilliamsJun 13, 2017
  14. Jeff KingJun 14, 2017
  15. 0/2 warn when adding an embedded repositoryJeff King, Jun 14, 2017
  16. 1/2 add: warn when adding an embedded repositoryJeff King, Jun 14, 2017
  17. 2/2 t: move "git add submodule" into test blocksJeff King, Jun 14, 2017
  18. Stefan BellerJun 14, 2017
  19. Jeff KingJun 15, 2017
  20. Junio C HamanoJun 13, 2017
  21. Jeff KingJun 14, 2017
  22. 2/2 t: move "git add submodule" into test blocksJeff King, Jun 13, 2017
  23. Stefan BellerJun 13, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.