{"thread":{"id":"60024","subject":"git submodule set-url does not sync when name != path","startedAt":"2023-07-24T04:28:44Z","lastAt":"2023-11-22T09:50:51Z","messageCount":19,"participants":["Jan Alexander Steffens (heftig)","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"479788","messageId":"cafa28e0bfe1f603204a74f619ac713518989a40.camel@archlinux.org","threadId":"60024","inReplyTo":null,"subject":"git submodule set-url does not sync when name != path","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-07-24T04:21:14Z","receivedAt":"2023-07-24T04:28:44Z","isPatch":false,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\nHi,\n\nI've encountered this problem with the flatpak-builder repository:\n\nhttps://github.com/flatpak/flatpak-builder/blob/main/.gitmodules\n\n[submodule \"libglnx\"]\n\tpath = subprojects/libglnx\n\turl = https://gitlab.gnome.org/GNOME/libglnx.git\n[submodule \"debugedit\"]\n\tpath = subprojects/debugedit\n\turl = https://sourceware.org/git/debugedit.git\n\nAfter 'git submodule init', using 'git submodule set-url libglnx foo'\nsuccessfully modifies .gitmodules but does not touch .git/config.\nHowever, a subsequent 'git submodule sync' does sync the modified url\nto the local config.\n\nI've investigated a bit and it seems 'git submodule set-url' calls\nsync_submodule with \"libglnx\" as the path, which does not work, while\n'git submodule sync' calls it with \"subprojects/libglnx\" as the path,\nwhich does work.\n\nGreetings,\nJan\n-----BEGIN PGP SIGNATURE-----\n\niIsEARYIADMWIQQGaHodnU+rCLUP2Ss7lKgOUKR3xwUCZL38OhUcaGVmdGlnQGFy\nY2hsaW51eC5vcmcACgkQO5SoDlCkd8cfsAD+M6POceTxAkFpnnmAH7j3RYLBEjFP\nw4kzY3U0UPVMU3wBAIPRBGjiwGFp/ws/iMeGQKif1E0ElqcxEbFAQ36HytIG\n=4OVq\n-----END PGP SIGNATURE-----\n"},{"id":"481635","messageId":"0a0a157f88321d25fdb0be771a454b3410a449f3.camel@archlinux.org","threadId":"60024","inReplyTo":"cafa28e0bfe1f603204a74f619ac713518989a40.camel@archlinux.org","subject":"Bug: git submodule set-url does not handle name != path correctly","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-09-09T22:26:30Z","receivedAt":"2023-09-09T22:26:50Z","isPatch":false,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"On Mon, 2023-07-24 at 06:21 +0200, Jan Alexander Steffens (heftig)\nwrote:\n> Hi,\n> \n> I've encountered this problem with the flatpak-builder repository:\n> \n> https://github.com/flatpak/flatpak-builder/blob/main/.gitmodules\n> \n> [submodule \"libglnx\"]\n>         path = subprojects/libglnx\n>         url = https://gitlab.gnome.org/GNOME/libglnx.git\n> [submodule \"debugedit\"]\n>         path = subprojects/debugedit\n>         url = https://sourceware.org/git/debugedit.git\n> \n> After 'git submodule init', using 'git submodule set-url libglnx foo'\n> successfully modifies .gitmodules but does not touch .git/config.\n> However, a subsequent 'git submodule sync' does sync the modified url\n> to the local config.\n> \n> I've investigated a bit and it seems 'git submodule set-url' calls\n> sync_submodule with \"libglnx\" as the path, which does not work, while\n> 'git submodule sync' calls it with \"subprojects/libglnx\" as the path,\n> which does work.\n> \n> Greetings,\n> Jan\n\nFriendly bump.\n\nThe docs say set-url needs the path to the submodule, not its name, so\nit should be 'git submodule set-url subprojects/libglnx foo'.\n\nHowever, using that actually creates a new\n'submodule.subprojects/libglnx.url' option instead of modifying the\nexisting 'submodule.libglnx.url'.\n\nIt looks like set-url needs to translate the path to the name for\nmodifying .gitmodules but does not do so.\n\n'git submodule set-branch' is also affected by this.\n\n"},{"id":"482587","messageId":"20231003185047.2697995-2-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH 2/6] submodule--helper: return error from set-url when modifying failed","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:43Z","receivedAt":"2023-10-03T18:51:04Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"set-branch will return an error when setting the config fails so I don't\nsee why set-url shouldn't. Also skip the sync in this case.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n builtin/submodule--helper.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f376466a5e..e2175083a6 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2890,39 +2890,41 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n \n static int module_set_url(int argc, const char **argv, const char *prefix)\n {\n-\tint quiet = 0;\n+\tint quiet = 0, ret;\n \tconst char *newurl;\n \tconst char *path;\n \tchar *config_name;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress output for setting url of a submodule\")),\n \t\tOPT_END()\n \t};\n \tconst char *const usage[] = {\n \t\tN_(\"git submodule set-url [--quiet] <path> <newurl>\"),\n \t\tNULL\n \t};\n \tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))\n \t\tusage_with_options(usage, options);\n \n \tsub = submodule_from_path(the_repository, null_oid(), path);\n \n \tif (!sub)\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n \t\t    path);\n \n \tconfig_name = xstrfmt(\"submodule.%s.url\", sub->name);\n-\tconfig_set_in_gitmodules_file_gently(config_name, newurl);\n+\tret = config_set_in_gitmodules_file_gently(config_name, newurl);\n \n-\trepo_read_gitmodules (the_repository, 0);\n-\tsync_submodule(sub->path, prefix, NULL, quiet ? OPT_QUIET : 0);\n+\tif (!ret) {\n+\t\trepo_read_gitmodules(the_repository, 0);\n+\t\tsync_submodule(sub->path, prefix, NULL, quiet ? OPT_QUIET : 0);\n+\t}\n \n \tfree(config_name);\n-\treturn 0;\n+\treturn !!ret;\n }\n \n static int module_set_branch(int argc, const char **argv, const char *prefix)\n-- \n2.42.0\n\n"},{"id":"482588","messageId":"20231003185047.2697995-5-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH 5/6] t7419: Test that we correctly handle renamed submodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:46Z","receivedAt":"2023-10-03T18:51:05Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"Add the submodule again with an explicitly different name and path. Test\nthat calling set-branch modifies the correct .gitmodules entries. Make\nsure we don't create a section named after the path instead of the name.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7419-submodule-set-branch.sh | 30 +++++++++++++++++++++++++++++-\n 1 file changed, 29 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 3cd30865a7..a5d1bc5c54 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -38,7 +38,8 @@ test_expect_success 'submodule config cache setup' '\n \t(cd super &&\n \t\tgit init &&\n \t\tgit submodule add ../submodule &&\n-\t\tgit commit -m \"add submodule\"\n+\t\tgit submodule add --name thename ../submodule thepath &&\n+\t\tgit commit -m \"add submodules\"\n \t)\n '\n \n@@ -100,4 +101,31 @@ test_expect_success 'test submodule set-branch -d' '\n \t)\n '\n \n+test_expect_success 'test submodule set-branch --branch with named submodule' '\n+\t(cd super &&\n+\t\tgit submodule set-branch --branch topic thepath &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.thename.branch &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thepath.branch &&\n+\t\tgit submodule update --remote &&\n+\t\tcat <<-\\EOF >expect &&\n+\t\tb\n+\t\tEOF\n+\t\tgit -C thepath show -s --pretty=%s >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'test submodule set-branch --default with named submodule' '\n+\t(cd super &&\n+\t\tgit submodule set-branch --default thepath &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thename.branch &&\n+\t\tgit submodule update --remote &&\n+\t\tcat <<-\\EOF >expect &&\n+\t\ta\n+\t\tEOF\n+\t\tgit -C thepath show -s --pretty=%s >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.42.0\n\n"},{"id":"482589","messageId":"20231003185047.2697995-1-heftig@archlinux.org","threadId":"60024","inReplyTo":"0a0a157f88321d25fdb0be771a454b3410a449f3.camel@archlinux.org","subject":"[PATCH 1/6] submodule--helper: use submodule_from_path in set-{url,branch}","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:42Z","receivedAt":"2023-10-03T18:51:07Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"The commands need a path to a submodule but treated it as the name when\nmodifying the .gitmodules file, leading to confusion when a submodule's\nname does not match its path.\n\nBecause calling submodule_from_path initializes the submodule cache, we\nneed to manually trigger a reread before syncing, as the cache is\nmissing the config change we just made.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n builtin/submodule--helper.c | 22 ++++++++++++++++++----\n 1 file changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f6871efd95..f376466a5e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2902,19 +2902,26 @@ static int module_set_url(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule set-url [--quiet] <path> <newurl>\"),\n \t\tNULL\n \t};\n+\tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))\n \t\tusage_with_options(usage, options);\n \n-\tconfig_name = xstrfmt(\"submodule.%s.url\", path);\n+\tsub = submodule_from_path(the_repository, null_oid(), path);\n \n+\tif (!sub)\n+\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n+\t\t    path);\n+\n+\tconfig_name = xstrfmt(\"submodule.%s.url\", sub->name);\n \tconfig_set_in_gitmodules_file_gently(config_name, newurl);\n-\tsync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);\n+\n+\trepo_read_gitmodules (the_repository, 0);\n+\tsync_submodule(sub->path, prefix, NULL, quiet ? OPT_QUIET : 0);\n \n \tfree(config_name);\n-\n \treturn 0;\n }\n \n@@ -2942,19 +2949,26 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>\"),\n \t\tNULL\n \t};\n+\tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (!opt_branch && !opt_default)\n \t\tdie(_(\"--branch or --default required\"));\n \n \tif (opt_branch && opt_default)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--branch\", \"--default\");\n \n \tif (argc != 1 || !(path = argv[0]))\n \t\tusage_with_options(usage, options);\n \n-\tconfig_name = xstrfmt(\"submodule.%s.branch\", path);\n+\tsub = submodule_from_path(the_repository, null_oid(), path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n+\t\t    path);\n+\n+\tconfig_name = xstrfmt(\"submodule.%s.branch\", sub->name);\n \tret = config_set_in_gitmodules_file_gently(config_name, opt_branch);\n \n \tfree(config_name);\n-- \n2.42.0\n\n"},{"id":"482590","messageId":"20231003185047.2697995-4-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH 4/6] t7419, t7420: Use test_cmp_config instead of grepping .gitmodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:45Z","receivedAt":"2023-10-03T18:51:08Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"We have a test function to verify config files. Use it as it's more\nprecise.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7419-submodule-set-branch.sh | 10 +++++-----\n t/t7420-submodule-set-url.sh    |  2 +-\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 5ac16d0eb7..3cd30865a7 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -44,53 +44,53 @@ test_expect_success 'submodule config cache setup' '\n \n test_expect_success 'ensure submodule branch is unset' '\n \t(cd super &&\n-\t\t! grep branch .gitmodules\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch\n \t)\n '\n \n test_expect_success 'test submodule set-branch --branch' '\n \t(cd super &&\n \t\tgit submodule set-branch --branch topic submodule &&\n-\t\tgrep \"branch = topic\" .gitmodules &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch --default' '\n \t(cd super &&\n \t\tgit submodule set-branch --default submodule &&\n-\t\t! grep branch .gitmodules &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -b' '\n \t(cd super &&\n \t\tgit submodule set-branch -b topic submodule &&\n-\t\tgrep \"branch = topic\" .gitmodules &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -d' '\n \t(cd super &&\n \t\tgit submodule set-branch -d submodule &&\n-\t\t! grep branch .gitmodules &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\ta\ndiff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh\nindex d6bf62b3ac..aa63d806fe 100755\n--- a/t/t7420-submodule-set-url.sh\n+++ b/t/t7420-submodule-set-url.sh\n@@ -49,7 +49,7 @@ test_expect_success 'test submodule set-url' '\n \t\tcd super &&\n \t\ttest_must_fail git submodule update --remote &&\n \t\tgit submodule set-url submodule ../newsubmodule &&\n-\t\tgrep -F \"url = ../newsubmodule\" .gitmodules &&\n+\t\ttest_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&\n \t\tgit submodule update --remote\n \t) &&\n \tgit -C super/submodule show >actual &&\n-- \n2.42.0\n\n"},{"id":"482591","messageId":"20231003185047.2697995-3-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH 3/6] t7419: Actually test the branch switching","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:44Z","receivedAt":"2023-10-03T18:51:10Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"The submodule repo the test set up had the 'topic' branch checked out,\nmeaning the repo's default branch (HEAD) is the 'topic' branch.\n\nThe following tests then pretended to switch between the default branch\nand the 'topic' branch. This was papered over by continually adding\ncommits to the 'topic' branch and checking if the submodule gets updated\nto this new commit.\n\nReturn the submodule repo to the 'main' branch after setup so we can\nactually test the switching behavior.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7419-submodule-set-branch.sh | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 232065504c..5ac16d0eb7 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -11,23 +11,28 @@ as expected.\n \n TEST_PASSES_SANITIZE_LEAK=true\n TEST_NO_CREATE_REPO=1\n+\n+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n+\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n \tgit config --global protocol.file.allow always\n '\n \n test_expect_success 'submodule config cache setup' '\n \tmkdir submodule &&\n \t(cd submodule &&\n \t\tgit init &&\n \t\techo a >a &&\n \t\tgit add . &&\n \t\tgit commit -ma &&\n \t\tgit checkout -b topic &&\n \t\techo b >a &&\n \t\tgit add . &&\n-\t\tgit commit -mb\n+\t\tgit commit -mb &&\n+\t\tgit checkout main\n \t) &&\n \tmkdir super &&\n \t(cd super &&\n@@ -57,41 +62,38 @@ test_expect_success 'test submodule set-branch --branch' '\n '\n \n test_expect_success 'test submodule set-branch --default' '\n-\ttest_commit -C submodule c &&\n \t(cd super &&\n \t\tgit submodule set-branch --default submodule &&\n \t\t! grep branch .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n-\t\tc\n+\t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -b' '\n-\ttest_commit -C submodule b &&\n \t(cd super &&\n \t\tgit submodule set-branch -b topic submodule &&\n \t\tgrep \"branch = topic\" .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -d' '\n-\ttest_commit -C submodule d &&\n \t(cd super &&\n \t\tgit submodule set-branch -d submodule &&\n \t\t! grep branch .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n-\t\td\n+\t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n-- \n2.42.0\n\n"},{"id":"482592","messageId":"20231003185047.2697995-6-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH 6/6] t7420: Test that we correctly handle renamed submodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-10-03T18:50:47Z","receivedAt":"2023-10-03T18:51:11Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"Create a second submodule with a name that differs from its path. Test\nthat calling set-url modifies the correct .gitmodules entries. Make sure\nwe don't create a section named after the path instead of the name.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7420-submodule-set-url.sh | 26 ++++++++++++++++++++++++--\n 1 file changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh\nindex aa63d806fe..bf7f15ee79 100755\n--- a/t/t7420-submodule-set-url.sh\n+++ b/t/t7420-submodule-set-url.sh\n@@ -25,34 +25,56 @@ test_expect_success 'submodule config cache setup' '\n \t\tgit add file &&\n \t\tgit commit -ma\n \t) &&\n+\tmkdir namedsubmodule &&\n+\t(\n+\t\tcd namedsubmodule &&\n+\t\tgit init &&\n+\t\techo 1 >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m1\n+\t) &&\n \tmkdir super &&\n \t(\n \t\tcd super &&\n \t\tgit init &&\n \t\tgit submodule add ../submodule &&\n-\t\tgit commit -m \"add submodule\"\n+\t\tgit submodule add --name thename ../namedsubmodule thepath &&\n+\t\tgit commit -m \"add submodules\"\n \t)\n '\n \n test_expect_success 'test submodule set-url' '\n-\t# add a commit and move the submodule (change the url)\n+\t# add commits and move the submodules (change the urls)\n \t(\n \t\tcd submodule &&\n \t\techo b >>file &&\n \t\tgit add file &&\n \t\tgit commit -mb\n \t) &&\n \tmv submodule newsubmodule &&\n \n+\t(\n+\t\tcd namedsubmodule &&\n+\t\techo 2 >>file &&\n+\t\tgit add file &&\n+\t\tgit commit -m2\n+\t) &&\n+\tmv namedsubmodule newnamedsubmodule &&\n+\n \tgit -C newsubmodule show >expect &&\n+\tgit -C newnamedsubmodule show >>expect &&\n \t(\n \t\tcd super &&\n \t\ttest_must_fail git submodule update --remote &&\n \t\tgit submodule set-url submodule ../newsubmodule &&\n \t\ttest_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&\n+\t\tgit submodule set-url thepath ../newnamedsubmodule &&\n+\t\ttest_cmp_config ../newnamedsubmodule -f .gitmodules submodule.thename.url &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thepath.url &&\n \t\tgit submodule update --remote\n \t) &&\n \tgit -C super/submodule show >actual &&\n+\tgit -C super/thepath show >>actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.42.0\n\n"},{"id":"482627","messageId":"xmqqsf6ruwse.fsf@gitster.g","threadId":"60024","inReplyTo":"20231003185047.2697995-2-heftig@archlinux.org","subject":"Re: [PATCH 2/6] submodule--helper: return error from set-url when modifying failed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-03T23:10:57Z","receivedAt":"2023-10-03T23:11:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Alexander Steffens (heftig)\" <heftig@archlinux.org> writes:\n\n> set-branch will return an error when setting the config fails so I don't\n> see why set-url shouldn't. Also skip the sync in this case.\n\nTwo other failures in this helper function gives end-user readable\nmessage (parameter errors are greeted with usage, giving wrong path\nis greeted with a die()), but this new error condition will silently\ndie unless config_set_in_gitmodules_file_gently() complains.\n\nI wonder if we should give an error message in this case, too.\n\nIn any case, not calling sync after failed set_in_gitmodules is a\nsensible design decision.\n"},{"id":"482629","messageId":"xmqqedibutk4.fsf@gitster.g","threadId":"60024","inReplyTo":"20231003185047.2697995-3-heftig@archlinux.org","subject":"Re: [PATCH 3/6] t7419: Actually test the branch switching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-04T00:20:43Z","receivedAt":"2023-10-04T00:20:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Alexander Steffens (heftig)\" <heftig@archlinux.org> writes:\n\n> Subject: Re: [PATCH 3/6] t7419: Actually test the branch switching\n\n\"Actually\" -> \"actually\".\n\n> The submodule repo the test set up had the 'topic' branch checked out,\n> meaning the repo's default branch (HEAD) is the 'topic' branch.\n>\n> The following tests then pretended to switch between the default branch\n> and the 'topic' branch. This was papered over by continually adding\n> commits to the 'topic' branch and checking if the submodule gets updated\n> to this new commit.\n>\n> Return the submodule repo to the 'main' branch after setup so we can\n> actually test the switching behavior.\n\nNicely spotted.\n\n> diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\n> index 232065504c..5ac16d0eb7 100755\n> --- a/t/t7419-submodule-set-branch.sh\n> +++ b/t/t7419-submodule-set-branch.sh\n> @@ -11,23 +11,28 @@ as expected.\n>  \n>  TEST_PASSES_SANITIZE_LEAK=true\n>  TEST_NO_CREATE_REPO=1\n> +\n> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n\nAnd it is good that this variable is used to prepare the test for\nboth kinds of CI runs that use 'main' or 'master' as the default\nbranch.\n\n"},{"id":"482630","messageId":"xmqqmswztcoc.fsf@gitster.g","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"Re: [PATCH 1/6] submodule--helper: use submodule_from_path in set-{url,branch}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-04T01:10:43Z","receivedAt":"2023-10-04T01:10:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Alexander Steffens (heftig)\" <heftig@archlinux.org> writes:\n\n> The commands need a path to a submodule but treated it as the name when\n> modifying the .gitmodules file, leading to confusion when a submodule's\n> name does not match its path.\n\nThanks for noticing and fixing this common mix-up.\n\n> Because calling submodule_from_path initializes the submodule cache, we\n> need to manually trigger a reread before syncing, as the cache is\n> missing the config change we just made.\n\n> Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index f6871efd95..f376466a5e 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -2902,19 +2902,26 @@ static int module_set_url(int argc, const char **argv, const char *prefix)\n>  \t\tN_(\"git submodule set-url [--quiet] <path> <newurl>\"),\n>  \t\tNULL\n>  \t};\n> +\tconst struct submodule *sub;\n>  \n>  \targc = parse_options(argc, argv, prefix, options, usage, 0);\n>  \n>  \tif (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))\n>  \t\tusage_with_options(usage, options);\n>  \n> -\tconfig_name = xstrfmt(\"submodule.%s.url\", path);\n> +\tsub = submodule_from_path(the_repository, null_oid(), path);\n>  \n> +\tif (!sub)\n> +\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n> +\t\t    path);\n> +\n> +\tconfig_name = xstrfmt(\"submodule.%s.url\", sub->name);\n\nLooks correct.\n\n>  \tconfig_set_in_gitmodules_file_gently(config_name, newurl);\n> -\tsync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);\n> +\n> +\trepo_read_gitmodules (the_repository, 0);\n\nStyle.  No extra space between function name and \"(\".\n\nBut more importantly, is this sufficient?  repo_read_gitmodules()\ndoes not seem to clear the cache and build its contents from scratch\n(as submodule_cache_check_init() bypasses itself upon second call).\n\nThe code is correct because submodule-config.c::config_from() does\nset .overwrite to true, so submodule.name.url would be overwritten\nto the new value, I think, but somebody else who is more familiar\nwith the recent submodule code may want to sanity check my analysis.\n\n> +\tsync_submodule(sub->path, prefix, NULL, quiet ? OPT_QUIET : 0);\n\nIs the use of \"sub\" still safe here?  \n\nI think it is safe as repo_read_gitmodules() did not rebuild the\nin-core cache from scratch but did selective overwrite, so the\nin-core instance \"sub\" is still valid, but again somebody else who\nis more familiar with the recent submodule code may want to sanity\ncheck.\n\n> @@ -2942,19 +2949,26 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)\n>  \t\tN_(\"git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>\"),\n>  \t\tNULL\n>  \t};\n> +\tconst struct submodule *sub;\n>  \n>  \targc = parse_options(argc, argv, prefix, options, usage, 0);\n>  \n>  \tif (!opt_branch && !opt_default)\n>  \t\tdie(_(\"--branch or --default required\"));\n>  \n>  \tif (opt_branch && opt_default)\n>  \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--branch\", \"--default\");\n>  \n>  \tif (argc != 1 || !(path = argv[0]))\n>  \t\tusage_with_options(usage, options);\n>  \n> -\tconfig_name = xstrfmt(\"submodule.%s.branch\", path);\n> +\tsub = submodule_from_path(the_repository, null_oid(), path);\n> +\n> +\tif (!sub)\n> +\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n> +\t\t    path);\n> +\n> +\tconfig_name = xstrfmt(\"submodule.%s.branch\", sub->name);\n>  \tret = config_set_in_gitmodules_file_gently(config_name, opt_branch);\n\nThis side happens not to require re-reading of gitmodules file,\nbecause, unlike the URL helper, we do not care what we have in the\nin-core cache is stale.  It is correct but feels a bit brittle.\n\n"},{"id":"485036","messageId":"20231121203413.176414-1-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231003185047.2697995-1-heftig@archlinux.org","subject":"[PATCH v2 1/6] submodule--helper: use submodule_from_path in set-{url,branch}","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:42Z","receivedAt":"2023-11-21T20:34:23Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"The commands need a path to a submodule but treated it as the name when\nmodifying the .gitmodules file, leading to confusion when a submodule's\nname does not match its path.\n\nBecause calling submodule_from_path initializes the submodule cache, we\nneed to manually trigger a reread before syncing, as the cache is\nmissing the config change we just made.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n\nNotes:\n    v2 changes:\n        - fixed code style\n        - replaced potentially unsafe use of `sub->path` with `path`\n\n builtin/submodule--helper.c | 20 +++++++++++++++++---\n 1 file changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6f3bf33e61..af461ada8b 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2901,19 +2901,26 @@ static int module_set_url(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule set-url [--quiet] <path> <newurl>\"),\n \t\tNULL\n \t};\n+\tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))\n \t\tusage_with_options(usage, options);\n \n-\tconfig_name = xstrfmt(\"submodule.%s.url\", path);\n+\tsub = submodule_from_path(the_repository, null_oid(), path);\n \n+\tif (!sub)\n+\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n+\t\t    path);\n+\n+\tconfig_name = xstrfmt(\"submodule.%s.url\", sub->name);\n \tconfig_set_in_gitmodules_file_gently(config_name, newurl);\n+\n+\trepo_read_gitmodules(the_repository, 0);\n \tsync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);\n \n \tfree(config_name);\n-\n \treturn 0;\n }\n \n@@ -2941,19 +2948,26 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>\"),\n \t\tNULL\n \t};\n+\tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (!opt_branch && !opt_default)\n \t\tdie(_(\"--branch or --default required\"));\n \n \tif (opt_branch && opt_default)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--branch\", \"--default\");\n \n \tif (argc != 1 || !(path = argv[0]))\n \t\tusage_with_options(usage, options);\n \n-\tconfig_name = xstrfmt(\"submodule.%s.branch\", path);\n+\tsub = submodule_from_path(the_repository, null_oid(), path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n+\t\t    path);\n+\n+\tconfig_name = xstrfmt(\"submodule.%s.branch\", sub->name);\n \tret = config_set_in_gitmodules_file_gently(config_name, opt_branch);\n \n \tfree(config_name);\n-- \n2.43.0\n\n"},{"id":"485037","messageId":"20231121203413.176414-2-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"[PATCH v2 2/6] submodule--helper: return error from set-url when modifying failed","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:43Z","receivedAt":"2023-11-21T20:34:23Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"set-branch will return an error when setting the config fails so I don't\nsee why set-url shouldn't. Also skip the sync in this case.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n builtin/submodule--helper.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex af461ada8b..0013ea1ab0 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2889,39 +2889,41 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n \n static int module_set_url(int argc, const char **argv, const char *prefix)\n {\n-\tint quiet = 0;\n+\tint quiet = 0, ret;\n \tconst char *newurl;\n \tconst char *path;\n \tchar *config_name;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress output for setting url of a submodule\")),\n \t\tOPT_END()\n \t};\n \tconst char *const usage[] = {\n \t\tN_(\"git submodule set-url [--quiet] <path> <newurl>\"),\n \t\tNULL\n \t};\n \tconst struct submodule *sub;\n \n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))\n \t\tusage_with_options(usage, options);\n \n \tsub = submodule_from_path(the_repository, null_oid(), path);\n \n \tif (!sub)\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n \t\t    path);\n \n \tconfig_name = xstrfmt(\"submodule.%s.url\", sub->name);\n-\tconfig_set_in_gitmodules_file_gently(config_name, newurl);\n+\tret = config_set_in_gitmodules_file_gently(config_name, newurl);\n \n-\trepo_read_gitmodules(the_repository, 0);\n-\tsync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);\n+\tif (!ret) {\n+\t\trepo_read_gitmodules(the_repository, 0);\n+\t\tsync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);\n+\t}\n \n \tfree(config_name);\n-\treturn 0;\n+\treturn !!ret;\n }\n \n static int module_set_branch(int argc, const char **argv, const char *prefix)\n-- \n2.43.0\n\n"},{"id":"485038","messageId":"20231121203413.176414-4-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"[PATCH v2 4/6] t7419, t7420: use test_cmp_config instead of grepping .gitmodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:45Z","receivedAt":"2023-11-21T20:34:23Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"We have a test function to verify config files. Use it as it's more\nprecise.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7419-submodule-set-branch.sh | 10 +++++-----\n t/t7420-submodule-set-url.sh    |  2 +-\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 5ac16d0eb7..3cd30865a7 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -44,53 +44,53 @@ test_expect_success 'submodule config cache setup' '\n \n test_expect_success 'ensure submodule branch is unset' '\n \t(cd super &&\n-\t\t! grep branch .gitmodules\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch\n \t)\n '\n \n test_expect_success 'test submodule set-branch --branch' '\n \t(cd super &&\n \t\tgit submodule set-branch --branch topic submodule &&\n-\t\tgrep \"branch = topic\" .gitmodules &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch --default' '\n \t(cd super &&\n \t\tgit submodule set-branch --default submodule &&\n-\t\t! grep branch .gitmodules &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -b' '\n \t(cd super &&\n \t\tgit submodule set-branch -b topic submodule &&\n-\t\tgrep \"branch = topic\" .gitmodules &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -d' '\n \t(cd super &&\n \t\tgit submodule set-branch -d submodule &&\n-\t\t! grep branch .gitmodules &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.submodule.branch &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\ta\ndiff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh\nindex d6bf62b3ac..aa63d806fe 100755\n--- a/t/t7420-submodule-set-url.sh\n+++ b/t/t7420-submodule-set-url.sh\n@@ -49,7 +49,7 @@ test_expect_success 'test submodule set-url' '\n \t\tcd super &&\n \t\ttest_must_fail git submodule update --remote &&\n \t\tgit submodule set-url submodule ../newsubmodule &&\n-\t\tgrep -F \"url = ../newsubmodule\" .gitmodules &&\n+\t\ttest_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&\n \t\tgit submodule update --remote\n \t) &&\n \tgit -C super/submodule show >actual &&\n-- \n2.43.0\n\n"},{"id":"485039","messageId":"20231121203413.176414-3-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"[PATCH v2 3/6] t7419: actually test the branch switching","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:44Z","receivedAt":"2023-11-21T20:34:23Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"The submodule repo the test set up had the 'topic' branch checked out,\nmeaning the repo's default branch (HEAD) is the 'topic' branch.\n\nThe following tests then pretended to switch between the default branch\nand the 'topic' branch. This was papered over by continually adding\ncommits to the 'topic' branch and checking if the submodule gets updated\nto this new commit.\n\nReturn the submodule repo to the 'main' branch after setup so we can\nactually test the switching behavior.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n\nNotes:\n    v2 changes:\n        - fixed subject\n\n t/t7419-submodule-set-branch.sh | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 232065504c..5ac16d0eb7 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -11,23 +11,28 @@ as expected.\n \n TEST_PASSES_SANITIZE_LEAK=true\n TEST_NO_CREATE_REPO=1\n+\n+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n+\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n \tgit config --global protocol.file.allow always\n '\n \n test_expect_success 'submodule config cache setup' '\n \tmkdir submodule &&\n \t(cd submodule &&\n \t\tgit init &&\n \t\techo a >a &&\n \t\tgit add . &&\n \t\tgit commit -ma &&\n \t\tgit checkout -b topic &&\n \t\techo b >a &&\n \t\tgit add . &&\n-\t\tgit commit -mb\n+\t\tgit commit -mb &&\n+\t\tgit checkout main\n \t) &&\n \tmkdir super &&\n \t(cd super &&\n@@ -57,41 +62,38 @@ test_expect_success 'test submodule set-branch --branch' '\n '\n \n test_expect_success 'test submodule set-branch --default' '\n-\ttest_commit -C submodule c &&\n \t(cd super &&\n \t\tgit submodule set-branch --default submodule &&\n \t\t! grep branch .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n-\t\tc\n+\t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -b' '\n-\ttest_commit -C submodule b &&\n \t(cd super &&\n \t\tgit submodule set-branch -b topic submodule &&\n \t\tgrep \"branch = topic\" .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n \t\tb\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n \n test_expect_success 'test submodule set-branch -d' '\n-\ttest_commit -C submodule d &&\n \t(cd super &&\n \t\tgit submodule set-branch -d submodule &&\n \t\t! grep branch .gitmodules &&\n \t\tgit submodule update --remote &&\n \t\tcat <<-\\EOF >expect &&\n-\t\td\n+\t\ta\n \t\tEOF\n \t\tgit -C submodule show -s --pretty=%s >actual &&\n \t\ttest_cmp expect actual\n-- \n2.43.0\n\n"},{"id":"485040","messageId":"20231121203413.176414-5-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"[PATCH v2 5/6] t7419: test that we correctly handle renamed submodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:46Z","receivedAt":"2023-11-21T20:34:23Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"Add the submodule again with an explicitly different name and path. Test\nthat calling set-branch modifies the correct .gitmodules entries. Make\nsure we don't create a section named after the path instead of the name.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7419-submodule-set-branch.sh | 30 +++++++++++++++++++++++++++++-\n 1 file changed, 29 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh\nindex 3cd30865a7..a5d1bc5c54 100755\n--- a/t/t7419-submodule-set-branch.sh\n+++ b/t/t7419-submodule-set-branch.sh\n@@ -38,7 +38,8 @@ test_expect_success 'submodule config cache setup' '\n \t(cd super &&\n \t\tgit init &&\n \t\tgit submodule add ../submodule &&\n-\t\tgit commit -m \"add submodule\"\n+\t\tgit submodule add --name thename ../submodule thepath &&\n+\t\tgit commit -m \"add submodules\"\n \t)\n '\n \n@@ -100,4 +101,31 @@ test_expect_success 'test submodule set-branch -d' '\n \t)\n '\n \n+test_expect_success 'test submodule set-branch --branch with named submodule' '\n+\t(cd super &&\n+\t\tgit submodule set-branch --branch topic thepath &&\n+\t\ttest_cmp_config topic -f .gitmodules submodule.thename.branch &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thepath.branch &&\n+\t\tgit submodule update --remote &&\n+\t\tcat <<-\\EOF >expect &&\n+\t\tb\n+\t\tEOF\n+\t\tgit -C thepath show -s --pretty=%s >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'test submodule set-branch --default with named submodule' '\n+\t(cd super &&\n+\t\tgit submodule set-branch --default thepath &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thename.branch &&\n+\t\tgit submodule update --remote &&\n+\t\tcat <<-\\EOF >expect &&\n+\t\ta\n+\t\tEOF\n+\t\tgit -C thepath show -s --pretty=%s >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \n2.43.0\n\n"},{"id":"485041","messageId":"20231121203413.176414-6-heftig@archlinux.org","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"[PATCH v2 6/6] t7420: test that we correctly handle renamed submodules","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-21T20:32:47Z","receivedAt":"2023-11-21T20:34:26Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"Create a second submodule with a name that differs from its path. Test\nthat calling set-url modifies the correct .gitmodules entries. Make sure\nwe don't create a section named after the path instead of the name.\n\nSigned-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>\n---\n t/t7420-submodule-set-url.sh | 26 ++++++++++++++++++++++++--\n 1 file changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh\nindex aa63d806fe..bf7f15ee79 100755\n--- a/t/t7420-submodule-set-url.sh\n+++ b/t/t7420-submodule-set-url.sh\n@@ -25,34 +25,56 @@ test_expect_success 'submodule config cache setup' '\n \t\tgit add file &&\n \t\tgit commit -ma\n \t) &&\n+\tmkdir namedsubmodule &&\n+\t(\n+\t\tcd namedsubmodule &&\n+\t\tgit init &&\n+\t\techo 1 >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m1\n+\t) &&\n \tmkdir super &&\n \t(\n \t\tcd super &&\n \t\tgit init &&\n \t\tgit submodule add ../submodule &&\n-\t\tgit commit -m \"add submodule\"\n+\t\tgit submodule add --name thename ../namedsubmodule thepath &&\n+\t\tgit commit -m \"add submodules\"\n \t)\n '\n \n test_expect_success 'test submodule set-url' '\n-\t# add a commit and move the submodule (change the url)\n+\t# add commits and move the submodules (change the urls)\n \t(\n \t\tcd submodule &&\n \t\techo b >>file &&\n \t\tgit add file &&\n \t\tgit commit -mb\n \t) &&\n \tmv submodule newsubmodule &&\n \n+\t(\n+\t\tcd namedsubmodule &&\n+\t\techo 2 >>file &&\n+\t\tgit add file &&\n+\t\tgit commit -m2\n+\t) &&\n+\tmv namedsubmodule newnamedsubmodule &&\n+\n \tgit -C newsubmodule show >expect &&\n+\tgit -C newnamedsubmodule show >>expect &&\n \t(\n \t\tcd super &&\n \t\ttest_must_fail git submodule update --remote &&\n \t\tgit submodule set-url submodule ../newsubmodule &&\n \t\ttest_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&\n+\t\tgit submodule set-url thepath ../newnamedsubmodule &&\n+\t\ttest_cmp_config ../newnamedsubmodule -f .gitmodules submodule.thename.url &&\n+\t\ttest_cmp_config \"\" -f .gitmodules --default \"\" submodule.thepath.url &&\n \t\tgit submodule update --remote\n \t) &&\n \tgit -C super/submodule show >actual &&\n+\tgit -C super/thepath show >>actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.43.0\n\n"},{"id":"485046","messageId":"xmqq4jhedxza.fsf@gitster.g","threadId":"60024","inReplyTo":"20231121203413.176414-1-heftig@archlinux.org","subject":"Re: [PATCH v2 1/6] submodule--helper: use submodule_from_path in set-{url,branch}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-22T07:54:33Z","receivedAt":"2023-11-22T07:54:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Alexander Steffens (heftig)\" <heftig@archlinux.org> writes:\n\n> Notes:\n>     v2 changes:\n>         - fixed code style\n>         - replaced potentially unsafe use of `sub->path` with `path`\n\nHasn't the previous iteration of this topic long been merged to not\njust 'next' but to 'master' and appears in a released version of Git?\n\nWe are all human, so mistakes are inevitable, but if we discover a\nmistake that needs fixing after a topic hits 'next', we take it as a\nsign that the particular mistake was easy to make and hard to spot,\nand the fix for it deserves its own explanation.  Please make an\nincremental update on top of what has already been merged with a\ngood explanation (explain why sub->path is \"potentially unsafe\" and\nwhy using path is better, for example).\n\nThanks.\n"},{"id":"485048","messageId":"ccbab7a73f734b09fb6915ab30506eb2c0db2321.camel@archlinux.org","threadId":"60024","inReplyTo":"xmqq4jhedxza.fsf@gitster.g","subject":"Re: [PATCH v2 1/6] submodule--helper: use submodule_from_path in set-{url,branch}","fromName":"Jan Alexander Steffens (heftig)","fromEmail":"heftig@archlinux.org","sentAt":"2023-11-22T09:50:44Z","receivedAt":"2023-11-22T09:50:51Z","isPatch":true,"sender":{"key":"heftig@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/204006?v=4"},"body":"On Wed, 2023-11-22 at 16:54 +0900, Junio C Hamano wrote:\n> \"Jan Alexander Steffens (heftig)\" <heftig@archlinux.org> writes:\n> \n> > Notes:\n> >     v2 changes:\n> >         - fixed code style\n> >         - replaced potentially unsafe use of `sub->path` with\n> > `path`\n> \n> Hasn't the previous iteration of this topic long been merged to not\n> just 'next' but to 'master' and appears in a released version of Git?\n> \n> We are all human, so mistakes are inevitable, but if we discover a\n> mistake that needs fixing after a topic hits 'next', we take it as a\n> sign that the particular mistake was easy to make and hard to spot,\n> and the fix for it deserves its own explanation.  Please make an\n> incremental update on top of what has already been merged with a\n> good explanation (explain why sub->path is \"potentially unsafe\" and\n> why using path is better, for example).\n> \n> Thanks.\n\nSo it has. I'm sorry. I was only keeping track of the 'maint' branch.\n"}]}