{"thread":{"id":"61901","subject":"2.46 submodule breakage","startedAt":"2024-08-05T11:32:13Z","lastAt":"2024-08-08T17:26:19Z","messageCount":35,"participants":["Jeppe Øland","Eric Sunshine","Patrick Steinhardt","Junio C Hamano","Kristoffer Haugsbakk"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"500060","messageId":"CA+osTZVApTAMogBDMaPDEVViJHrFT=BOer=Py4fjTvpsifzfKA@mail.gmail.com","threadId":"61901","inReplyTo":null,"subject":"2.46 submodule breakage","fromName":"Jeppe Øland","fromEmail":"joland@gmail.com","sentAt":"2024-08-05T11:31:59Z","receivedAt":"2024-08-05T11:32:13Z","isPatch":false,"sender":{"key":"joland@gmail.com","avatar":null},"body":"Hi there,\n\nI just upgraded from Git 2.45.2 to Git 2.46 (Git for Windows, but I\ndon't think this has anything to do with that), and it seems to have\nbroken some submodule stuff for me.\n\nSpecifically, when I do a recursive pull, I get the following error\nfrom one of the submodules:\n fatal: Unable to find current revision in submodule path 'path/to/sub'\n\nThere are other submodules in the repo that work fine.\nThe only difference with this one is that it was added using the\n--name option, so internally it's just known as \"sub\".\n\nLooks like the problem happens in the final pull stage when it executes:\n git submodule--helper update --recursive --rebase --\n\nA recursive clone with 2.46 works fine, but any subsequent recursive\npull gives the error.\nSo far I haven't been able to fix it, except by downgrading to 2.45.2.\n\nAny ideas?\n\nRegards,\n-Jeppe\n"},{"id":"500193","messageId":"CA+osTZX4FBT5za3Nw2Hc=DCNNbFg=xL4b3AhNmN8S2XKb5BXUA@mail.gmail.com","threadId":"61901","inReplyTo":"CA+osTZVApTAMogBDMaPDEVViJHrFT=BOer=Py4fjTvpsifzfKA@mail.gmail.com","subject":"Re: 2.46 submodule breakage","fromName":"Jeppe Øland","fromEmail":"joland@gmail.com","sentAt":"2024-08-06T13:18:25Z","receivedAt":"2024-08-06T13:18:37Z","isPatch":false,"sender":{"key":"joland@gmail.com","avatar":null},"body":"Hi all,\n\nI did a bunch of testing to narrow down the cause:\n\nIt is not related to the Windows port - all the testing was done in WSL.\nIt only happens when the clone containing submodules is done with\n--ref-format=reftable.\nThe commit breaking it is: 129cb1b99d hash: drop (mostly) unused\n`is_empty_{blob,tree}_sha1()` functions\n\nRegards,\n-Jeppe\n\nOn Mon, Aug 5, 2024 at 1:31 PM Jeppe Øland wrote:\n>\n> Hi there,\n>\n> I just upgraded from Git 2.45.2 to Git 2.46 (Git for Windows, but I\n> don't think this has anything to do with that), and it seems to have\n> broken some submodule stuff for me.\n>\n> Specifically, when I do a recursive pull, I get the following error\n> from one of the submodules:\n> fatal: Unable to find current revision in submodule path 'path/to/sub'\n>\n> There are other submodules in the repo that work fine.\n> The only difference with this one is that it was added using the\n> --name option, so internally it's just known as \"sub\".\n>\n> Looks like the problem happens in the final pull stage when it executes:\n> git submodule--helper update --recursive --rebase --\n>\n> A recursive clone with 2.46 works fine, but any subsequent recursive\n> pull gives the error.\n> So far I haven't been able to fix it, except by downgrading to 2.45.2.\n>\n> Any ideas?\n>\n> Regards,\n> -Jeppe\n"},{"id":"500245","messageId":"CAPig+cQ05ocHKELovvEhvptAn09DLuitQkdJyFQ=N5qvje-GhQ@mail.gmail.com","threadId":"61901","inReplyTo":"CA+osTZX4FBT5za3Nw2Hc=DCNNbFg=xL4b3AhNmN8S2XKb5BXUA@mail.gmail.com","subject":"Re: 2.46 submodule breakage","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-08-06T18:26:35Z","receivedAt":"2024-08-06T18:26:47Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 6, 2024 at 9:21 AM Jeppe Øland wrote:\n> I did a bunch of testing to narrow down the cause:\n>\n> It is not related to the Windows port - all the testing was done in WSL.\n> It only happens when the clone containing submodules is done with\n> --ref-format=reftable.\n> The commit breaking it is: 129cb1b99d hash: drop (mostly) unused\n> `is_empty_{blob,tree}_sha1()` functions\n\nCc:'ing Patrick, the author of that change.\n"},{"id":"500269","messageId":"ZrMWylPWZ8Tm5i45@tanuki","threadId":"61901","inReplyTo":"CAPig+cQ05ocHKELovvEhvptAn09DLuitQkdJyFQ=N5qvje-GhQ@mail.gmail.com","subject":"Re: 2.46 submodule breakage","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T06:40:10Z","receivedAt":"2024-08-07T06:40:16Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 06, 2024 at 02:26:35PM -0400, Eric Sunshine wrote:\n> On Tue, Aug 6, 2024 at 9:21 AM Jeppe Øland wrote:\n> > I did a bunch of testing to narrow down the cause:\n> >\n> > It is not related to the Windows port - all the testing was done in WSL.\n> > It only happens when the clone containing submodules is done with\n> > --ref-format=reftable.\n> > The commit breaking it is: 129cb1b99d hash: drop (mostly) unused\n> > `is_empty_{blob,tree}_sha1()` functions\n> \n> Cc:'ing Patrick, the author of that change.\n\nThanks for Cc'ing me, and thanks for the report.\n\nI just wanted to say that I couldn't reproduce, but then spotted that\nyou said that it only happens when using `--ref-format=reftable` during\nthe git-clone(1) step. And indeed, that causes the following test to\nfail:\n\n test_expect_success 'recursive pull with different non-default ref format' '\n # Set up the initial structure. This is an upstream repository that has\n # a submodule, as well as a downstream clone of the upstream\n # repository.\n git init submodule &&\n test_commit -C submodule submodule-base &&\n git init upstream &&\n test_commit -C upstream upstream-base &&\n git -C upstream submodule add \"file://$(pwd)/submodule\" &&\n git -C upstream commit -m \"upstream submodule\" &&\n git clone --ref-format=reftable --recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n\n # Update the submodule.\n test_commit -C submodule submodule-update &&\n git -C upstream/submodule pull &&\n git -C upstream commit -am \"update the submodule\" &&\n\n git -C downstream pull --recurse-submodules\n '\n\nThe issue here is that the recursive clone causes a mixture of ref\nformats: the parent repository uses the \"reftable\" backend, whereas the\nchild repository uses \"files\". In theory, I think Git should be able to\nhandle a mixture of ref formats like this, but I'm not surprised that it\nactually fails in practice. The question is whether this is sensible, or\nwhether submodules should use the same ref format as their parent.\n\nSo it feels like the commit you have bisected this to only unearths this\nissue, but isn't the actual root cause.\n\nI'll investigate further and will try to come up with a patch later this\nweek.\n\nPatrick\n"},{"id":"500294","messageId":"ZrMkje9VugtKz-gB@tanuki","threadId":"61901","inReplyTo":"ZrMWylPWZ8Tm5i45@tanuki","subject":"Re: 2.46 submodule breakage","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T07:38:53Z","receivedAt":"2024-08-07T07:38:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 07, 2024 at 08:40:10AM +0200, Patrick Steinhardt wrote:\n> On Tue, Aug 06, 2024 at 02:26:35PM -0400, Eric Sunshine wrote:\n> > On Tue, Aug 6, 2024 at 9:21 AM Jeppe Øland wrote:\n> > > I did a bunch of testing to narrow down the cause:\n> > >\n> > > It is not related to the Windows port - all the testing was done in WSL.\n> > > It only happens when the clone containing submodules is done with\n> > > --ref-format=reftable.\n> > > The commit breaking it is: 129cb1b99d hash: drop (mostly) unused\n> > > `is_empty_{blob,tree}_sha1()` functions\n> > \n> > Cc:'ing Patrick, the author of that change.\n> \n> Thanks for Cc'ing me, and thanks for the report.\n> \n> I just wanted to say that I couldn't reproduce, but then spotted that\n> you said that it only happens when using `--ref-format=reftable` during\n> the git-clone(1) step. And indeed, that causes the following test to\n> fail:\n> \n> test_expect_success 'recursive pull with different non-default ref format' '\n> # Set up the initial structure. This is an upstream repository that has\n> # a submodule, as well as a downstream clone of the upstream\n> # repository.\n> git init submodule &&\n> test_commit -C submodule submodule-base &&\n> git init upstream &&\n> test_commit -C upstream upstream-base &&\n> git -C upstream submodule add \"file://$(pwd)/submodule\" &&\n> git -C upstream commit -m \"upstream submodule\" &&\n> git clone --ref-format=reftable --recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n> \n> # Update the submodule.\n> test_commit -C submodule submodule-update &&\n> git -C upstream/submodule pull &&\n> git -C upstream commit -am \"update the submodule\" &&\n> \n> git -C downstream pull --recurse-submodules\n> '\n> \n> The issue here is that the recursive clone causes a mixture of ref\n> formats: the parent repository uses the \"reftable\" backend, whereas the\n> child repository uses \"files\". In theory, I think Git should be able to\n> handle a mixture of ref formats like this, but I'm not surprised that it\n> actually fails in practice. The question is whether this is sensible, or\n> whether submodules should use the same ref format as their parent.\n> \n> So it feels like the commit you have bisected this to only unearths this\n> issue, but isn't the actual root cause.\n> \n> I'll investigate further and will try to come up with a patch later this\n> week.\n\nFor the record, the fix is as simple as the below diff. We indeed end up\ninitializing submodule ref stores with the parent ref storage format,\nnot with the one of the subrepo. I'll spend some more time though to\ncheck whether other commands are impacted, as well, and write some more\ntests.\n\nPatrick\n\ndiff --git a/refs.c b/refs.c\nindex 915aeb4d1d..e4b1f4f8b1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2011,7 +2011,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,\n free(subrepo);\n goto done;\n }\n-\trefs = ref_store_init(subrepo, the_repository->ref_storage_format,\n+\trefs = ref_store_init(subrepo, subrepo->ref_storage_format,\n submodule_sb.buf,\n REF_STORE_READ | REF_STORE_ODB);\n register_ref_store_map(&repo->submodule_ref_stores, \"submodule\",\n"},{"id":"500303","messageId":"cover.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"CA+osTZVApTAMogBDMaPDEVViJHrFT=BOer=Py4fjTvpsifzfKA@mail.gmail.com","subject":"[PATCH 0/6] Improvements for ref storage formats with submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:43:44Z","receivedAt":"2024-08-07T12:43:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series contains some improvements for ref storage\nformats and their interaction with submodules. Notably:\n\n - Use the correct format for submodules in situations where the parent\n repository uses a different ref storage format than the submodule.\n\n - Wire up `--ref-format=` for git-submodule(1), such that users can\n explicitly use a different ref format for their submodules.\n\n - Propagate the `--ref-format=` flag of git-clone(1) into submodules\n when using `--recursive`.\n\nThe first three patches implement improvements for the above three\nissues and introduce tests. The test did hit some memory leaks, which\nget fixed by patches 3 to 6 such that the new test can be marked as leak\nfree.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (6):\n builtin/submodule: allow cloning with different ref storage format\n builtin/clone: propagate ref storage format to submodules\n refs: fix ref storage format for submodule ref stores\n submodule: fix leaking fetch tasks\n submodule: fix leaking seen submodule names\n object: fix leaking packfiles when closing object store\n\n Documentation/git-submodule.txt | 5 +-\n builtin/clone.c | 10 ++-\n builtin/submodule--helper.c | 30 +++++++\n git-submodule.sh | 9 ++\n object.c | 9 ++\n refs.c | 2 +-\n submodule.c | 18 ++--\n t/t5572-pull-submodule.sh | 1 +\n t/t7418-submodule-sparse-gitmodules.sh | 1 +\n t/t7424-submodule-mixed-ref-formats.sh | 120 +++++++++++++++++++++++++\n 10 files changed, 191 insertions(+), 14 deletions(-)\n create mode 100755 t/t7424-submodule-mixed-ref-formats.sh\n\n-- \n2.46.0.dirty\n\n"},{"id":"500304","messageId":"a450759bd1e0d84192fd8b278b660fc8527369ca.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:43:48Z","receivedAt":"2024-08-07T12:43:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"As submodules are proper self-contained repositories, it is perfectly\nvalid for them to have a different ref storage format than their parent\nrepository. There is no obvious way for users to ask for the ref storage\nformat when initializing submodules though. Whether the setup of such\nmixed-ref-storage-format constellations is all that useful remains to be\nseen. But there is no good reason to not expose such an option, and we\nwill require it in a subsequent patch.\n\nIntroduce a new `--ref-format=` option for git-submodule(1) that allows\nthe user to pick the ref storage format. This option will also be used\nin a subsequent commit, where we start to propagate the same flag from\ngit-clone(1) to cloning submodules with the `--recursive` switch.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-submodule.txt | 5 +++-\n builtin/submodule--helper.c | 30 +++++++++++++++++++\n git-submodule.sh | 9 ++++++\n t/t7424-submodule-mixed-ref-formats.sh | 41 ++++++++++++++++++++++++++\n 4 files changed, 84 insertions(+), 1 deletion(-)\n create mode 100755 t/t7424-submodule-mixed-ref-formats.sh\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex ca0347a37b..73ef8b9696 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -136,7 +136,7 @@ If you really want to remove a submodule from the repository and commit\n that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal\n options.\n \n-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--ref-format ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n@@ -185,6 +185,9 @@ submodule with the `--init` option.\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and update any nested submodules within.\n \n+If `--ref-format ` is specified, the ref storage format of newly\n+cloned submodules will be set accordingly.\n+\n If `--filter ` is specified, the given partial clone filter will be\n applied to the submodule. See linkgit:git-rev-list[1] for details on filter\n specifications.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f1218a1995..42a36bc2f7 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1532,6 +1532,7 @@ struct module_clone_data {\n \tconst char *url;\n \tconst char *depth;\n \tstruct list_objects_filter_options *filter_options;\n+\tenum ref_storage_format ref_storage_format;\n \tunsigned int quiet: 1;\n \tunsigned int progress: 1;\n \tunsigned int dissociate: 1;\n@@ -1540,6 +1541,7 @@ struct module_clone_data {\n };\n #define MODULE_CLONE_DATA_INIT { \\\n \t.single_branch = -1, \\\n+\t.ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN, \\\n }\n \n struct submodule_alternate_setup {\n@@ -1738,6 +1740,9 @@ static int clone_submodule(const struct module_clone_data *clone_data,\n \t\t\t\tstrvec_pushl(&cp.args, \"--reference\",\n \t\t\t\t\t item->string, NULL);\n \t\t}\n+\t\tif (clone_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tstrvec_pushf(&cp.args, \"--ref-format=%s\",\n+\t\t\t\t ref_storage_format_to_name(clone_data->ref_storage_format));\n \t\tif (clone_data->dissociate)\n \t\t\tstrvec_push(&cp.args, \"--dissociate\");\n \t\tif (sm_gitdir && *sm_gitdir)\n@@ -1832,6 +1837,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tstruct list_objects_filter_options filter_options =\n \t\tLIST_OBJECTS_FILTER_INIT;\n+\tconst char *ref_storage_format = NULL;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &clone_data.prefix,\n@@ -1849,6 +1855,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"reference\", &reference,\n \t\t\t N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"ref-format\", &ref_storage_format, N_(\"format\"),\n+\t\t\t N_(\"specify the reference format to use\")),\n \t\tOPT_BOOL(0, \"dissociate\", &dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n \t\tOPT_STRING(0, \"depth\", &clone_data.depth,\n@@ -1875,6 +1883,11 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, module_clone_options,\n \t\t\t git_submodule_helper_usage, 0);\n \n+\tif (ref_storage_format) {\n+\t\tclone_data.ref_storage_format = ref_storage_format_by_name(ref_storage_format);\n+\t\tif (clone_data.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_storage_format);\n+\t}\n \tclone_data.dissociate = !!dissociate;\n \tclone_data.quiet = !!quiet;\n \tclone_data.progress = !!progress;\n@@ -1974,6 +1987,7 @@ struct update_data {\n \tstruct submodule_update_strategy update_strategy;\n \tstruct list_objects_filter_options *filter_options;\n \tstruct module_list list;\n+\tenum ref_storage_format ref_storage_format;\n \tint depth;\n \tint max_jobs;\n \tint single_branch;\n@@ -1997,6 +2011,7 @@ struct update_data {\n #define UPDATE_DATA_INIT { \\\n \t.update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n \t.list = MODULE_LIST_INIT, \\\n+\t.ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN, \\\n \t.recommend_shallow = -1, \\\n \t.references = STRING_LIST_INIT_DUP, \\\n \t.single_branch = -1, \\\n@@ -2132,6 +2147,9 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\t\t expand_list_objects_filter_spec(suc->update_data->filter_options));\n \tif (suc->update_data->require_init)\n \t\tstrvec_push(&child->args, \"--require-init\");\n+\tif (suc->update_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\tstrvec_pushf(&child->args, \"--ref-format=%s\",\n+\t\t\t ref_storage_format_to_name(suc->update_data->ref_storage_format));\n \tstrvec_pushl(&child->args, \"--path\", sub->path, NULL);\n \tstrvec_pushl(&child->args, \"--name\", sub->name, NULL);\n \tstrvec_pushl(&child->args, \"--url\", url, NULL);\n@@ -2562,6 +2580,9 @@ static void update_data_to_args(const struct update_data *update_data,\n \t\tfor_each_string_list_item(item, &update_data->references)\n \t\t\tstrvec_pushl(args, \"--reference\", item->string, NULL);\n \t}\n+\tif (update_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\tstrvec_pushf(args, \"--ref-format=%s\",\n+\t\t\t ref_storage_format_to_name(update_data->ref_storage_format));\n \tif (update_data->filter_options && update_data->filter_options->choice)\n \t\tstrvec_pushf(args, \"--filter=%s\",\n \t\t\t\texpand_list_objects_filter_spec(\n@@ -2737,6 +2758,7 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \tstruct update_data opt = UPDATE_DATA_INIT;\n \tstruct list_objects_filter_options filter_options =\n \t\tLIST_OBJECTS_FILTER_INIT;\n+\tconst char *ref_storage_format = NULL;\n \tint ret;\n \tstruct option module_update_options[] = {\n \t\tOPT__SUPER_PREFIX(&opt.super_prefix),\n@@ -2760,6 +2782,8 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \t\t\tSM_UPDATE_REBASE),\n \t\tOPT_STRING_LIST(0, \"reference\", &opt.references, N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"ref-format\", &ref_storage_format, N_(\"format\"),\n+\t\t\t N_(\"specify the reference format to use\")),\n \t\tOPT_BOOL(0, \"dissociate\", &opt.dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n \t\tOPT_INTEGER(0, \"depth\", &opt.depth,\n@@ -2803,6 +2827,12 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \t\t\t\t module_update_options);\n \t}\n \n+\tif (ref_storage_format) {\n+\t\topt.ref_storage_format = ref_storage_format_by_name(ref_storage_format);\n+\t\tif (opt.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_storage_format);\n+\t}\n+\n \topt.filter_options = &filter_options;\n \topt.prefix = prefix;\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 7f9582d923..dcbe943071 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -268,6 +268,14 @@ cmd_update()\n \t\t-r|--rebase)\n \t\t\trebase=1\n \t\t\t;;\n+\t\t--ref-format)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tref_format=\"--ref-format=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--ref-format=*)\n+\t\t\tref_format=\"$1\"\n+\t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\treference=\"--reference=$2\"\n@@ -349,6 +357,7 @@ cmd_update()\n \t\t${rebase:+--rebase} \\\n \t\t${merge:+--merge} \\\n \t\t${checkout:+--checkout} \\\n+\t\t${ref_format:+\"$ref_format\"} \\\n \t\t${reference:+\"$reference\"} \\\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+\"$depth\"} \\\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nnew file mode 100755\nindex 0000000000..de84007554\n--- /dev/null\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -0,0 +1,41 @@\n+#!/bin/sh\n+\n+test_description='submodules handle mixed ref storage formats'\n+\n+. ./test-lib.sh\n+\n+test_ref_format () {\n+\techo \"$2\" >expect &&\n+\tgit -C \"$1\" rev-parse --show-ref-format >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+for OTHER_FORMAT in files reftable\n+do\n+\tif test \"$OTHER_FORMAT\" = \"$GIT_DEFAULT_REF_FORMAT\"\n+\tthen\n+\t\tcontinue\n+\tfi\n+\n+test_expect_success 'setup' '\n+\tgit config set --global protocol.file.allow always\n+'\n+\n+test_expect_success 'clone submodules with different ref storage format' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -m \"upstream submodule\" &&\n+\n+\tgit clone --no-recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n+\ttest_ref_format downstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\tgit -C downstream submodule update --init --ref-format=$OTHER_FORMAT &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n+'\n+\n+done\n+\n+test_done\n-- \n2.46.0.dirty\n\n"},{"id":"500305","messageId":"e5923c0b3364f28ce504c772f7fa75404f80e651.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 2/6] builtin/clone: propagate ref storage format to submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:43:53Z","receivedAt":"2024-08-07T12:43:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When recursively cloning a repository with a non-default ref storage\nformat, e.g. by passing the `--ref-format=` option, then only the\ntop-level repository will end up will end up using that ref storage\nformat. All recursively cloned submodules will instead use the default\nformat. While mixed-format constellations are expected to work alright,\nthe outcome still is somewhat surprising as we have essentially ignored\nthe user's request.\n\nFix this by propagating the requested ref format to cloned submodules.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/clone.c | 10 ++++++++--\n t/t7424-submodule-mixed-ref-formats.sh | 23 +++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex af6017d41a..75b15b5773 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -729,7 +729,8 @@ static int git_sparse_checkout_init(const char *repo)\n \treturn result;\n }\n \n-static int checkout(int submodule_progress, int filter_submodules)\n+static int checkout(int submodule_progress, int filter_submodules,\n+\t\t enum ref_storage_format ref_storage_format)\n {\n \tstruct object_id oid;\n \tchar *head;\n@@ -813,6 +814,10 @@ static int checkout(int submodule_progress, int filter_submodules)\n \t\t\tstrvec_push(&cmd.args, \"--no-fetch\");\n \t\t}\n \n+\t\tif (ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tstrvec_pushf(&cmd.args, \"--ref-format=%s\",\n+\t\t\t\t ref_storage_format_to_name(ref_storage_format));\n+\n \t\tif (filter_submodules && filter_options.choice)\n \t\t\tstrvec_pushf(&cmd.args, \"--filter=%s\",\n \t\t\t\t expand_list_objects_filter_spec(&filter_options));\n@@ -1536,7 +1541,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \n \tjunk_mode = JUNK_LEAVE_REPO;\n-\terr = checkout(submodule_progress, filter_submodules);\n+\terr = checkout(submodule_progress, filter_submodules,\n+\t\t ref_storage_format);\n \n \tfree(remote_name);\n \tstrbuf_release(&reflog_msg);\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex de84007554..4e4991d04c 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -21,6 +21,29 @@ test_expect_success 'setup' '\n \tgit config set --global protocol.file.allow always\n '\n \n+test_expect_success 'recursive clone propagates ref storage format' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -am \"add submodule\" &&\n+\n+\t# The upstream repository and its submodule should be using the default\n+\t# ref format.\n+\ttest_ref_format upstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format upstream/submodule \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\n+\t# The cloned repositories should use the other ref format that we have\n+\t# specified via `--ref-format`. The option should propagate to cloned\n+\t# submodules.\n+\tgit clone --ref-format=$OTHER_FORMAT --recurse-submodules \\\n+\t\tupstream downstream &&\n+\ttest_ref_format downstream \"$OTHER_FORMAT\" &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n+'\n+\n test_expect_success 'clone submodules with different ref storage format' '\n \ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n \n-- \n2.46.0.dirty\n\n"},{"id":"500306","messageId":"aaff9134ed849bcad08382168424d4aa92b153c8.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 3/6] refs: fix ref storage format for submodule ref stores","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:43:58Z","receivedAt":"2024-08-07T12:44:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When opening a submodule ref storage we accidentally use the ref storage\nformat of the owning repository, not of the submodule repository. As\nsubmodules may have a different storage format than their parent repo\nthis can lead to bugs when trying to access the submodule ref storage\nfrom the parent repository.\n\nOne such bug was reported when performing a recursive pull with mixed\nref stores, which fails with:\n\n $ git pull --recursive\n fatal: Unable to find current revision in submodule path 'path/to/sub'\n\nFix the bug by using the submodule repository's ref storage format\ninstead.\n\nNote that only the second added test fails without this fix. The other\none is included regardless as it exercises other parts where we might\nend up accessing submodule ref stores.\n\nReported-by: Jeppe Øland \nSigned-off-by: Patrick Steinhardt \n---\n refs.c | 2 +-\n t/t7424-submodule-mixed-ref-formats.sh | 57 +++++++++++++++++++++++++-\n 2 files changed, 57 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 915aeb4d1d..e4b1f4f8b1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2011,7 +2011,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,\n \t\tfree(subrepo);\n \t\tgoto done;\n \t}\n-\trefs = ref_store_init(subrepo, the_repository->ref_storage_format,\n+\trefs = ref_store_init(subrepo, subrepo->ref_storage_format,\n \t\t\t submodule_sb.buf,\n \t\t\t REF_STORE_READ | REF_STORE_ODB);\n \tregister_ref_store_map(&repo->submodule_ref_stores, \"submodule\",\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex 4e4991d04c..998a5d82e9 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -18,7 +18,10 @@ do\n \tfi\n \n test_expect_success 'setup' '\n-\tgit config set --global protocol.file.allow always\n+\tgit config set --global protocol.file.allow always &&\n+\t# Some tests migrate the ref storage format, which does not work with\n+\t# reflogs at the time of writing these tests.\n+\tgit config set --global core.logAllRefUpdates false\n '\n \n test_expect_success 'recursive clone propagates ref storage format' '\n@@ -59,6 +62,58 @@ test_expect_success 'clone submodules with different ref storage format' '\n \ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n '\n \n+test_expect_success 'status with mixed submodule ref storages' '\n+\ttest_when_finished \"rm -rf submodule main\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init main &&\n+\tgit -C main submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C main commit -m \"add submodule\" &&\n+\tgit -C main/submodule refs migrate --ref-format=$OTHER_FORMAT &&\n+\n+\t# The main repository should use the default ref format now, whereas\n+\t# the submodule should use the other format.\n+\ttest_ref_format main \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format main/submodule \"$OTHER_FORMAT\" &&\n+\n+\tcat >expect <<-EOF &&\n+\t $(git -C main/submodule rev-parse HEAD) submodule (submodule-initial)\n+\tEOF\n+\tgit -C main submodule status >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'recursive pull with mixed formats' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\t# Set up the initial structure with an upstream repository that has a\n+\t# submodule, as well as a downstream clone of the upstream repository.\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -m \"upstream submodule\" &&\n+\n+\t# Clone the upstream repository such that the main repo and its\n+\t# submodules have different formats.\n+\tgit clone --no-recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n+\tgit -C downstream submodule update --init --ref-format=$OTHER_FORMAT &&\n+\ttest_ref_format downstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\" &&\n+\n+\t# Update the upstream submodule as well as the owning repository such\n+\t# that we can do a recursive pull.\n+\ttest_commit -C submodule submodule-update &&\n+\tgit -C upstream/submodule pull &&\n+\tgit -C upstream commit -am \"update the submodule\" &&\n+\n+\tgit -C downstream pull --recurse-submodules &&\n+\tgit -C upstream/submodule rev-parse HEAD >expect &&\n+\tgit -C downstream/submodule rev-parse HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n done\n \n test_done\n-- \n2.46.0.dirty\n\n"},{"id":"500307","messageId":"8f8371c18a5f0a0f9b1412605199a21db334e90d.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 4/6] submodule: fix leaking fetch tasks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:44:03Z","receivedAt":"2024-08-07T12:44:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When done with a fetch task used for parallel fetches of submodules, we\nneed to both call `fetch_task_release()` to release the task's contents\nand `free()` to release the task itself. Most sites do this already, but\nsome only call `fetch_task_release()` and thus leak memory.\n\nWhile we could trivially fix this by adding the two missing calls to\nfree(3P), the result would be that we always call both functions. Let's\nthus refactor the code such that `fetch_task_release()` also frees the\nstructure itself. Rename it to `fetch_task_free()` accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n submodule.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex ab99a30253..f027a6455e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1496,7 +1496,7 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path)\n \treturn (const struct submodule *) ret;\n }\n \n-static void fetch_task_release(struct fetch_task *p)\n+static void fetch_task_free(struct fetch_task *p)\n {\n \tif (p->free_sub)\n \t\tfree((void*)p->sub);\n@@ -1508,6 +1508,7 @@ static void fetch_task_release(struct fetch_task *p)\n \tFREE_AND_NULL(p->repo);\n \n \tstrvec_clear(&p->git_args);\n+\tfree(p);\n }\n \n static struct repository *get_submodule_repo_for(struct repository *r,\n@@ -1576,8 +1577,7 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf\n \treturn task;\n \n cleanup:\n-\tfetch_task_release(task);\n-\tfree(task);\n+\tfetch_task_free(task);\n \treturn NULL;\n }\n \n@@ -1607,8 +1607,7 @@ get_fetch_task_from_index(struct submodule_parallel_fetch *spf,\n \t\t} else {\n \t\t\tstruct strbuf empty_submodule_path = STRBUF_INIT;\n \n-\t\t\tfetch_task_release(task);\n-\t\t\tfree(task);\n+\t\t\tfetch_task_free(task);\n \n \t\t\t/*\n \t\t\t * An empty directory is normal,\n@@ -1654,8 +1653,7 @@ get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,\n \t\t\t\t cs_data->path,\n \t\t\t\t repo_find_unique_abbrev(the_repository, cs_data->super_oid, DEFAULT_ABBREV));\n \n-\t\t\tfetch_task_release(task);\n-\t\t\tfree(task);\n+\t\t\tfetch_task_free(task);\n \t\t\tcontinue;\n \t\t}\n \n@@ -1763,7 +1761,7 @@ static int fetch_start_failure(struct strbuf *err UNUSED,\n \n \tspf->result = 1;\n \n-\tfetch_task_release(task);\n+\tfetch_task_free(task);\n \treturn 0;\n }\n \n@@ -1828,8 +1826,7 @@ static int fetch_finish(int retvalue, struct strbuf *err UNUSED,\n \t}\n \n out:\n-\tfetch_task_release(task);\n-\n+\tfetch_task_free(task);\n \treturn 0;\n }\n \n-- \n2.46.0.dirty\n\n"},{"id":"500308","messageId":"732142aaa6da70f7840eb0c4ae82ebba16f05c55.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 5/6] submodule: fix leaking seen submodule names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:44:07Z","receivedAt":"2024-08-07T12:44:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We keep track of submodules we have already seen via a string map such\nthat we don't process the same submodule twice. We never free that map\nthough, causing a memory leak.\n\nFix this leak by clearing the map.\n\nSigned-off-by: Patrick Steinhardt \n---\n submodule.c | 1 +\n t/t5572-pull-submodule.sh | 1 +\n t/t7418-submodule-sparse-gitmodules.sh | 1 +\n 3 files changed, 3 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex f027a6455e..13b8f8c19c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1880,6 +1880,7 @@ int fetch_submodules(struct repository *r,\n \tstrvec_clear(&spf.args);\n out:\n \tfree_submodules_data(&spf.changed_submodule_names);\n+\tstring_list_clear(&spf.seen_submodule_names, 0);\n \treturn spf.result;\n }\n \ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex 51744521f7..916e58c166 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -5,6 +5,7 @@ test_description='pull can handle submodules'\n GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1\n export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \ndiff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh\nindex dde11ecce8..e1d9bf2ee3 100755\n--- a/t/t7418-submodule-sparse-gitmodules.sh\n+++ b/t/t7418-submodule-sparse-gitmodules.sh\n@@ -15,6 +15,7 @@ also by committing .gitmodules and then just removing it from the filesystem.\n GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1\n export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.dirty\n\n"},{"id":"500309","messageId":"8dc7cc76d588ffe143d1d35e78e6eb86ed6d0b20.1723032100.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"[PATCH 6/6] object: fix leaking packfiles when closing object store","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T12:44:12Z","receivedAt":"2024-08-07T12:44:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `raw_object_store_clear()`, we close and free several\nresources associated with the object store. Part of that is to close and\nfree all the packfiles, which is handled by `close_object_store()`.\n\nThat function really only ends up closing the packfiles though, but it\ndoesn't free them. And in fact it can't, as that function is being\ncalled via `run_command()` when `close_object_store = 1`, which is done\ne.g. when we execute git-maintenance(1). At that point, other structures\nmay still have references on those packfiles, and thus we cannot free\nthem here. So while it is in fact intentional that we really only close\nthem, the result is a memory leak because `raw_object_store_clear()`\ndoes not free them, either.\n\nFix the leak by freeing the packfiles in `raw_object_store_clear()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n object.c | 9 +++++++++\n t/t7424-submodule-mixed-ref-formats.sh | 1 +\n 2 files changed, 10 insertions(+)\n\ndiff --git a/object.c b/object.c\nindex 0c0fcb76c0..d756c7f2ea 100644\n--- a/object.c\n+++ b/object.c\n@@ -614,6 +614,15 @@ void raw_object_store_clear(struct raw_object_store *o)\n \n \tINIT_LIST_HEAD(&o->packed_git_mru);\n \tclose_object_store(o);\n+\n+\t/*\n+\t * `close_object_store()` only closes the packfiles, but doesn't free\n+\t * them. We thus have to do this manually.\n+\t */\n+\tfor (struct packed_git *p = o->packed_git, *next; p; p = next) {\n+\t\tnext = p->next;\n+\t\tfree(p);\n+\t}\n \to->packed_git = NULL;\n \n \thashmap_clear(&o->pack_map);\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex 998a5d82e9..812496dc3b 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -2,6 +2,7 @@\n \n test_description='submodules handle mixed ref storage formats'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_ref_format () {\n-- \n2.46.0.dirty\n\n"},{"id":"500322","messageId":"ZrN0HNGLtFMhVp-c@tanuki","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"Re: [PATCH 0/6] Improvements for ref storage formats with submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-07T13:18:20Z","receivedAt":"2024-08-07T13:18:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 07, 2024 at 02:43:44PM +0200, Patrick Steinhardt wrote:\n> Hi,\n> \n> this small patch series contains some improvements for ref storage\n> formats and their interaction with submodules. Notably:\n> \n> - Use the correct format for submodules in situations where the parent\n> repository uses a different ref storage format than the submodule.\n> \n> - Wire up `--ref-format=` for git-submodule(1), such that users can\n> explicitly use a different ref format for their submodules.\n> \n> - Propagate the `--ref-format=` flag of git-clone(1) into submodules\n> when using `--recursive`.\n> \n> The first three patches implement improvements for the above three\n> issues and introduce tests. The test did hit some memory leaks, which\n> get fixed by patches 3 to 6 such that the new test can be marked as leak\n> free.\n\nJust got a shower thought that this isn't quite there yet. Most\nimportantly, I think we should default to the ref storage format of the\nparent submodule both when adding submodules and when cloning a\nsubmodule into a preexisting repository.\n\nI'll wait until tomorrow before addressing this though to ideally get\nsome feedback until then.\n\nPatrick\n"},{"id":"500332","messageId":"CA+osTZWES2eU7xwfd6fSNdprg_iJyaqRaA18ft+vynXgf40FXQ@mail.gmail.com","threadId":"61901","inReplyTo":"a450759bd1e0d84192fd8b278b660fc8527369ca.1723032100.git.ps@pks.im","subject":"Re: [PATCH 0/6] Improvements for ref storage formats with submodules","fromName":"Jeppe Øland","fromEmail":"joland@gmail.com","sentAt":"2024-08-07T14:45:16Z","receivedAt":"2024-08-07T14:45:29Z","isPatch":true,"sender":{"key":"joland@gmail.com","avatar":null},"body":"> Just got a shower thought that this isn't quite there yet. Most\n> importantly, I think we should default to the ref storage format of the\n> parent submodule both when adding submodules and when cloning a\n> submodule into a preexisting repository.\n\nYep, I was going to suggest exactly the same!\nThanks for the quick response.\n\nRegards\n-Jeppe\n"},{"id":"500334","messageId":"xmqq7ccspb54.fsf@gitster.g","threadId":"61901","inReplyTo":"ZrMkje9VugtKz-gB@tanuki","subject":"Re: 2.46 submodule breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-07T16:09:59Z","receivedAt":"2024-08-07T16:10:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> For the record, the fix is as simple as the below diff. We indeed end up\n> initializing submodule ref stores with the parent ref storage format,\n> not with the one of the subrepo. I'll spend some more time though to\n> check whether other commands are impacted, as well, and write some more\n> tests.\n\nI think allowing separate ref format in submodules is a very\nsensible thing to do. The containing superproject should not even\nhave to know what ref backend is in use in the submodule, unlike the\nobject format used there, which it has to know in order to store the\nname of commit objects in its tree entries as gitlinks.\n\nAnd the preimage of this particular change does look wrong to\nblindly reuse the format used in the superproject.\n\nThanks.\n\n> diff --git a/refs.c b/refs.c\n> index 915aeb4d1d..e4b1f4f8b1 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2011,7 +2011,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,\n> free(subrepo);\n> goto done;\n> }\n> -\trefs = ref_store_init(subrepo, the_repository->ref_storage_format,\n> +\trefs = ref_store_init(subrepo, subrepo->ref_storage_format,\n> submodule_sb.buf,\n> REF_STORE_READ | REF_STORE_ODB);\n> register_ref_store_map(&repo->submodule_ref_stores, \"submodule\",\n"},{"id":"500362","messageId":"xmqqwmkslz7m.fsf@gitster.g","threadId":"61901","inReplyTo":"a450759bd1e0d84192fd8b278b660fc8527369ca.1723032100.git.ps@pks.im","subject":"Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-07T22:55:57Z","receivedAt":"2024-08-07T22:56:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n> +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--ref-format ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n> +\n> --\n> Update the registered submodules to match what the superproject\n> @@ -185,6 +185,9 @@ submodule with the `--init` option.\n> If `--recursive` is specified, this command will recurse into the\n> registered submodules, and update any nested submodules within.\n> \n> +If `--ref-format ` is specified, the ref storage format of newly\n> +cloned submodules will be set accordingly.\n> +\n> If `--filter ` is specified, the given partial clone filter will be\n> applied to the submodule. See linkgit:git-rev-list[1] for details on filter\n> specifications.\n\nPresumably, if the named submodule has already been initialized, we\nare not converting its ref backend with --ref-format= option\nwhen \"git submodule update --ref-format=\" is run. Would it\nmake sense to say it is an error to give it without \"--init\", I\nwonder. If so, we probably would need to see if other existing\noptions like \"--filter\" also need a similar sanity check, if not\nalready done.\n\nThanks.\n"},{"id":"500365","messageId":"xmqqsevglyol.fsf@gitster.g","threadId":"61901","inReplyTo":"e5923c0b3364f28ce504c772f7fa75404f80e651.1723032100.git.ps@pks.im","subject":"Re: [PATCH 2/6] builtin/clone: propagate ref storage format to submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-07T23:07:22Z","receivedAt":"2024-08-07T23:07:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When recursively cloning a repository with a non-default ref storage\n> format, e.g. by passing the `--ref-format=` option, then only the\n> top-level repository will end up will end up using that ref storage\n> format. All recursively cloned submodules will instead use the default\n> format. While mixed-format constellations are expected to work alright,\n> the outcome still is somewhat surprising as we have essentially ignored\n> the user's request.\n>\n> Fix this by propagating the requested ref format to cloned submodules.\n\nMakes sense.\n"},{"id":"500377","messageId":"xmqqa5hnn7m7.fsf@gitster.g","threadId":"61901","inReplyTo":"cover.1723032100.git.ps@pks.im","subject":"Re: [PATCH 0/6] Improvements for ref storage formats with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-08T01:09:04Z","receivedAt":"2024-08-08T01:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> this small patch series contains some improvements for ref storage\n> formats and their interaction with submodules. Notably:\n>\n> - Use the correct format for submodules in situations where the parent\n> repository uses a different ref storage format than the submodule.\n>\n> - Wire up `--ref-format=` for git-submodule(1), such that users can\n> explicitly use a different ref format for their submodules.\n>\n> - Propagate the `--ref-format=` flag of git-clone(1) into submodules\n> when using `--recursive`.\n>\n> The first three patches implement improvements for the above three\n> issues and introduce tests. The test did hit some memory leaks, which\n> get fixed by patches 3 to 6 such that the new test can be marked as leak\n> free.\n\nNicely done. I've read through the series and did not find any\ndesign decisions in the series questionable.\n\nAs to propagating the choice of ref backend down from the\nsuperproject to the submodule, I am not sure if it matters all that\nmuch, so I view it as a relative low priority. If somebody wants to\nuse a specific ref backend for their repositories, then they want\nall their \"git init\" (or init_db()) to use that ref backend, and\nwould arrange configuration to make it so. When \"git submodule\ninit\" internally calls \"git init\" (or init_db()), as long as we make\nsure such a choice would propagate to the new repository that\nhappens to the one used for that submodule, we do not necessarily\nneed to have a custom logic that says \"ah, the user did not say\nanything about the ref backend, so let me peek the one used in the\nsuperproject and propagate it down\".\n\nThanks.\n\n"},{"id":"500404","messageId":"ZrRtB46vu9rC000y@tanuki","threadId":"61901","inReplyTo":"xmqqwmkslz7m.fsf@gitster.g","subject":"Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:00:23Z","receivedAt":"2024-08-08T07:00:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 07, 2024 at 03:55:57PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n> > +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--ref-format ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n> > +\n> > --\n> > Update the registered submodules to match what the superproject\n> > @@ -185,6 +185,9 @@ submodule with the `--init` option.\n> > If `--recursive` is specified, this command will recurse into the\n> > registered submodules, and update any nested submodules within.\n> > \n> > +If `--ref-format ` is specified, the ref storage format of newly\n> > +cloned submodules will be set accordingly.\n> > +\n> > If `--filter ` is specified, the given partial clone filter will be\n> > applied to the submodule. See linkgit:git-rev-list[1] for details on filter\n> > specifications.\n> \n> Presumably, if the named submodule has already been initialized, we\n> are not converting its ref backend with --ref-format= option\n> when \"git submodule update --ref-format=\" is run. Would it\n> make sense to say it is an error to give it without \"--init\", I\n> wonder. If so, we probably would need to see if other existing\n> options like \"--filter\" also need a similar sanity check, if not\n> already done.\n\nWell, even when \"--init\" was given it is not sure whether the ref\nstorage format will actually end up being used, because that option only\ntells us to initialize uninitialized submodules. So if the submodule was\ninitialized already, then the ref storage format won't be used.\n\nWe probably could add such a sanity check. But as you say, other options\nlike \"--filter\", \"--depth\", \"--reference\" and \"--recommend-shallow\"\ndon't have that check, either, so it would feel a bit like opening a can\nof worms. So personally, I'd rather defer this to another day where we\nthen implement the check for all of these options.\n\nPatrick\n"},{"id":"500405","messageId":"ZrRtEGIqEGgwyvhk@tanuki","threadId":"61901","inReplyTo":"xmqqa5hnn7m7.fsf@gitster.g","subject":"Re: [PATCH 0/6] Improvements for ref storage formats with submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:00:32Z","receivedAt":"2024-08-08T07:00:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 07, 2024 at 06:09:04PM -0700, Junio C Hamano wrote:\n> As to propagating the choice of ref backend down from the\n> superproject to the submodule, I am not sure if it matters all that\n> much, so I view it as a relative low priority. If somebody wants to\n> use a specific ref backend for their repositories, then they want\n> all their \"git init\" (or init_db()) to use that ref backend, and\n> would arrange configuration to make it so. When \"git submodule\n> init\" internally calls \"git init\" (or init_db()), as long as we make\n> sure such a choice would propagate to the new repository that\n> happens to the one used for that submodule, we do not necessarily\n> need to have a custom logic that says \"ah, the user did not say\n> anything about the ref backend, so let me peek the one used in the\n> superproject and propagate it down\".\n\nYou know, I'll just skip this for now. It feels somewhat orthogonal to\nthe changes in this series, and we're not closing the door on anything.\n\nPatrick\n"},{"id":"500406","messageId":"cover.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"CA+osTZVApTAMogBDMaPDEVViJHrFT=BOer=Py4fjTvpsifzfKA@mail.gmail.com","subject":"[PATCH v2 0/8] Improvements for ref storage formats with submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:21Z","receivedAt":"2024-08-08T07:35:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of my patch series that aims to improve\ninteraction of ref storage formats with submodules.\n\nChanges compared to v1:\n\n - I noticed that `git submodule add` also used to fail when used with\n a preexisting repository that uses a different ref format.\n\n - Add a preparatory refactoring that wraps overly long command lines\n in \"git-submodule.sh\".\n\n - Wire up support for `--ref-format=` in `git submodule add`.\n\nNotably not included is a change to make `git submodule add/clone` honor\nthe ref storage format of the parent repository. I think it is somewhat\northogonal to the changes I do in this series, and it is not entirely\nobvious whether it is a good idea or not. We can still do this in a\nfuture patch series, if we ever hear good arguments for it.\n\nThanks!\n\nPatrick Steinhardt (8):\n git-submodule.sh: break overly long command lines\n builtin/submodule: allow cloning with different ref storage format\n builtin/clone: propagate ref storage format to submodules\n refs: fix ref storage format for submodule ref stores\n builtin/submodule: allow \"add\" to use different ref storage format\n submodule: fix leaking fetch tasks\n submodule: fix leaking seen submodule names\n object: fix leaking packfiles when closing object store\n\n Documentation/git-submodule.txt | 10 +-\n builtin/clone.c | 10 +-\n builtin/submodule--helper.c | 46 +++++++-\n git-submodule.sh | 82 ++++++++++++--\n object.c | 9 ++\n refs.c | 2 +-\n submodule.c | 18 ++--\n t/t5572-pull-submodule.sh | 1 +\n t/t7418-submodule-sparse-gitmodules.sh | 1 +\n t/t7424-submodule-mixed-ref-formats.sh | 144 +++++++++++++++++++++++++\n 10 files changed, 298 insertions(+), 25 deletions(-)\n create mode 100755 t/t7424-submodule-mixed-ref-formats.sh\n\nRange-diff against v1:\n-: ---------- > 1: 6513c6b17d git-submodule.sh: break overly long command lines\n1: a450759bd1 = 2: e6cda43878 builtin/submodule: allow cloning with different ref storage format\n2: e5923c0b33 = 3: ed314f5333 builtin/clone: propagate ref storage format to submodules\n3: aaff9134ed ! 4: f13356581e refs: fix ref storage format for submodule ref stores\n @@ Commit message\n $ git pull --recursive\n fatal: Unable to find current revision in submodule path 'path/to/sub'\n \n - Fix the bug by using the submodule repository's ref storage format\n - instead.\n + The same issue occurs when adding a repository contained in the working\n + tree with a different ref storage format via `git submodule add`.\n \n - Note that only the second added test fails without this fix. The other\n - one is included regardless as it exercises other parts where we might\n - end up accessing submodule ref stores.\n + Fix the bug by using the submodule repository's ref storage format\n + instead and add some tests. Note that the test for `git submodule\n + status` was included as a precaution, only. The command worked alright\n + even without the bugfix.\n \n Reported-by: Jeppe Øland \n Signed-off-by: Patrick Steinhardt \n @@ t/t7424-submodule-mixed-ref-formats.sh: do\n +\t# Some tests migrate the ref storage format, which does not work with\n +\t# reflogs at the time of writing these tests.\n +\tgit config set --global core.logAllRefUpdates false\n ++'\n ++\n ++test_expect_success 'add existing repository with different ref storage format' '\n ++\ttest_when_finished \"rm -rf parent\" &&\n ++\n ++\tgit init parent &&\n ++\t(\n ++\t\tcd parent &&\n ++\t\ttest_commit parent &&\n ++\t\tgit init --ref-format=$OTHER_FORMAT submodule &&\n ++\t\ttest_commit -C submodule submodule &&\n ++\t\tgit submodule add ./submodule\n ++\t)\n '\n \n test_expect_success 'recursive clone propagates ref storage format' '\n-: ---------- > 5: 4ce17e44a1 builtin/submodule: allow \"add\" to use different ref storage format\n4: 8f8371c18a = 6: d92770290f submodule: fix leaking fetch tasks\n5: 732142aaa6 = 7: e9421189ca submodule: fix leaking seen submodule names\n6: 8dc7cc76d5 = 8: d05737c75f object: fix leaking packfiles when closing object store\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500407","messageId":"6513c6b17d261ed5900aa904a438c8770ae026cd.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 1/8] git-submodule.sh: break overly long command lines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:27Z","receivedAt":"2024-08-08T07:35:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"For most of the subcommands of git-submodule(1), we end up passing a\nbunch of arguments to the submodule helper. This quickly leads to overly\nlong lines, where it becomes hard to spot what has changed when one\nneeds to modify them.\n\nBreak up these lines into one argument per line, similarly to how it is\ndone for the \"clone\" subcommand already.\n\nSigned-off-by: Patrick Steinhardt \n---\n git-submodule.sh | 64 +++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 55 insertions(+), 9 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 7f9582d923..fd588b1864 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -129,7 +129,17 @@ cmd_add()\n \t\tusage\n \tfi\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add ${quiet:+--quiet} ${force:+--force} ${progress:+\"--progress\"} ${branch:+--branch \"$branch\"} ${reference_path:+--reference \"$reference_path\"} ${dissociate:+--dissociate} ${custom_name:+--name \"$custom_name\"} ${depth:+\"$depth\"} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${force:+--force} \\\n+\t\t${progress:+\"--progress\"} \\\n+\t\t${branch:+--branch \"$branch\"} \\\n+\t\t${reference_path:+--reference \"$reference_path\"} \\\n+\t\t${dissociate:+--dissociate} \\\n+\t\t${custom_name:+--name \"$custom_name\"} \\\n+\t\t${depth:+\"$depth\"} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -160,7 +170,11 @@ cmd_foreach()\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper foreach ${quiet:+--quiet} ${recursive:+--recursive} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper foreach \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${recursive:+--recursive} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -191,7 +205,10 @@ cmd_init()\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init ${quiet:+--quiet} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init \\\n+\t\t${quiet:+--quiet} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -227,7 +244,12 @@ cmd_deinit()\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit ${quiet:+--quiet} ${force:+--force} ${deinit_all:+--all} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${force:+--force} \\\n+\t\t${deinit_all:+--all} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -399,7 +421,12 @@ cmd_set_branch() {\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-branch ${quiet:+--quiet} ${branch:+--branch \"$branch\"} ${default:+--default} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-branch \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${branch:+--branch \"$branch\"} \\\n+\t\t${default:+--default} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -428,7 +455,10 @@ cmd_set_url() {\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-url ${quiet:+--quiet} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-url \\\n+\t\t${quiet:+--quiet} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n #\n@@ -480,7 +510,13 @@ cmd_summary() {\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper summary ${files:+--files} ${cached:+--cached} ${for_status:+--for-status} ${summary_limit:+-n $summary_limit} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper summary \\\n+\t\t${files:+--files} \\\n+\t\t${cached:+--cached} \\\n+\t\t${for_status:+--for-status} \\\n+\t\t${summary_limit:+-n $summary_limit} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n #\n # List all submodules, prefixed with:\n@@ -521,8 +557,14 @@ cmd_status()\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper status ${quiet:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper status \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${cached:+--cached} \\\n+\t\t${recursive:+--recursive} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n+\n #\n # Sync remote urls for submodules\n # This makes the value for remote.$remote.url match the value\n@@ -554,7 +596,11 @@ cmd_sync()\n \t\tesac\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper sync ${quiet:+--quiet} ${recursive:+--recursive} -- \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper sync \\\n+\t\t${quiet:+--quiet} \\\n+\t\t${recursive:+--recursive} \\\n+\t\t-- \\\n+\t\t\"$@\"\n }\n \n cmd_absorbgitdirs()\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500408","messageId":"e6cda43878040f0b190bb9dc0bc4d087b9852ef8.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 2/8] builtin/submodule: allow cloning with different ref storage format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:32Z","receivedAt":"2024-08-08T07:35:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"As submodules are proper self-contained repositories, it is perfectly\nvalid for them to have a different ref storage format than their parent\nrepository. There is no obvious way for users to ask for the ref storage\nformat when initializing submodules though. Whether the setup of such\nmixed-ref-storage-format constellations is all that useful remains to be\nseen. But there is no good reason to not expose such an option, and we\nwill require it in a subsequent patch.\n\nIntroduce a new `--ref-format=` option for git-submodule(1) that allows\nthe user to pick the ref storage format. This option will also be used\nin a subsequent commit, where we start to propagate the same flag from\ngit-clone(1) to cloning submodules with the `--recursive` switch.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-submodule.txt | 5 +++-\n builtin/submodule--helper.c | 30 +++++++++++++++++++\n git-submodule.sh | 9 ++++++\n t/t7424-submodule-mixed-ref-formats.sh | 41 ++++++++++++++++++++++++++\n 4 files changed, 84 insertions(+), 1 deletion(-)\n create mode 100755 t/t7424-submodule-mixed-ref-formats.sh\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex ca0347a37b..73ef8b9696 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -136,7 +136,7 @@ If you really want to remove a submodule from the repository and commit\n that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal\n options.\n \n-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--ref-format ] [--depth ] [--recursive] [--jobs ] [--[no-]single-branch] [--filter ] [--] [...]::\n +\n --\n Update the registered submodules to match what the superproject\n@@ -185,6 +185,9 @@ submodule with the `--init` option.\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and update any nested submodules within.\n \n+If `--ref-format ` is specified, the ref storage format of newly\n+cloned submodules will be set accordingly.\n+\n If `--filter ` is specified, the given partial clone filter will be\n applied to the submodule. See linkgit:git-rev-list[1] for details on filter\n specifications.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f1218a1995..42a36bc2f7 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1532,6 +1532,7 @@ struct module_clone_data {\n \tconst char *url;\n \tconst char *depth;\n \tstruct list_objects_filter_options *filter_options;\n+\tenum ref_storage_format ref_storage_format;\n \tunsigned int quiet: 1;\n \tunsigned int progress: 1;\n \tunsigned int dissociate: 1;\n@@ -1540,6 +1541,7 @@ struct module_clone_data {\n };\n #define MODULE_CLONE_DATA_INIT { \\\n \t.single_branch = -1, \\\n+\t.ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN, \\\n }\n \n struct submodule_alternate_setup {\n@@ -1738,6 +1740,9 @@ static int clone_submodule(const struct module_clone_data *clone_data,\n \t\t\t\tstrvec_pushl(&cp.args, \"--reference\",\n \t\t\t\t\t item->string, NULL);\n \t\t}\n+\t\tif (clone_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tstrvec_pushf(&cp.args, \"--ref-format=%s\",\n+\t\t\t\t ref_storage_format_to_name(clone_data->ref_storage_format));\n \t\tif (clone_data->dissociate)\n \t\t\tstrvec_push(&cp.args, \"--dissociate\");\n \t\tif (sm_gitdir && *sm_gitdir)\n@@ -1832,6 +1837,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tstruct list_objects_filter_options filter_options =\n \t\tLIST_OBJECTS_FILTER_INIT;\n+\tconst char *ref_storage_format = NULL;\n \n \tstruct option module_clone_options[] = {\n \t\tOPT_STRING(0, \"prefix\", &clone_data.prefix,\n@@ -1849,6 +1855,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"reference\", &reference,\n \t\t\t N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"ref-format\", &ref_storage_format, N_(\"format\"),\n+\t\t\t N_(\"specify the reference format to use\")),\n \t\tOPT_BOOL(0, \"dissociate\", &dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n \t\tOPT_STRING(0, \"depth\", &clone_data.depth,\n@@ -1875,6 +1883,11 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, module_clone_options,\n \t\t\t git_submodule_helper_usage, 0);\n \n+\tif (ref_storage_format) {\n+\t\tclone_data.ref_storage_format = ref_storage_format_by_name(ref_storage_format);\n+\t\tif (clone_data.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_storage_format);\n+\t}\n \tclone_data.dissociate = !!dissociate;\n \tclone_data.quiet = !!quiet;\n \tclone_data.progress = !!progress;\n@@ -1974,6 +1987,7 @@ struct update_data {\n \tstruct submodule_update_strategy update_strategy;\n \tstruct list_objects_filter_options *filter_options;\n \tstruct module_list list;\n+\tenum ref_storage_format ref_storage_format;\n \tint depth;\n \tint max_jobs;\n \tint single_branch;\n@@ -1997,6 +2011,7 @@ struct update_data {\n #define UPDATE_DATA_INIT { \\\n \t.update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n \t.list = MODULE_LIST_INIT, \\\n+\t.ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN, \\\n \t.recommend_shallow = -1, \\\n \t.references = STRING_LIST_INIT_DUP, \\\n \t.single_branch = -1, \\\n@@ -2132,6 +2147,9 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\t\t expand_list_objects_filter_spec(suc->update_data->filter_options));\n \tif (suc->update_data->require_init)\n \t\tstrvec_push(&child->args, \"--require-init\");\n+\tif (suc->update_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\tstrvec_pushf(&child->args, \"--ref-format=%s\",\n+\t\t\t ref_storage_format_to_name(suc->update_data->ref_storage_format));\n \tstrvec_pushl(&child->args, \"--path\", sub->path, NULL);\n \tstrvec_pushl(&child->args, \"--name\", sub->name, NULL);\n \tstrvec_pushl(&child->args, \"--url\", url, NULL);\n@@ -2562,6 +2580,9 @@ static void update_data_to_args(const struct update_data *update_data,\n \t\tfor_each_string_list_item(item, &update_data->references)\n \t\t\tstrvec_pushl(args, \"--reference\", item->string, NULL);\n \t}\n+\tif (update_data->ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\tstrvec_pushf(args, \"--ref-format=%s\",\n+\t\t\t ref_storage_format_to_name(update_data->ref_storage_format));\n \tif (update_data->filter_options && update_data->filter_options->choice)\n \t\tstrvec_pushf(args, \"--filter=%s\",\n \t\t\t\texpand_list_objects_filter_spec(\n@@ -2737,6 +2758,7 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \tstruct update_data opt = UPDATE_DATA_INIT;\n \tstruct list_objects_filter_options filter_options =\n \t\tLIST_OBJECTS_FILTER_INIT;\n+\tconst char *ref_storage_format = NULL;\n \tint ret;\n \tstruct option module_update_options[] = {\n \t\tOPT__SUPER_PREFIX(&opt.super_prefix),\n@@ -2760,6 +2782,8 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \t\t\tSM_UPDATE_REBASE),\n \t\tOPT_STRING_LIST(0, \"reference\", &opt.references, N_(\"repo\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"ref-format\", &ref_storage_format, N_(\"format\"),\n+\t\t\t N_(\"specify the reference format to use\")),\n \t\tOPT_BOOL(0, \"dissociate\", &opt.dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n \t\tOPT_INTEGER(0, \"depth\", &opt.depth,\n@@ -2803,6 +2827,12 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \t\t\t\t module_update_options);\n \t}\n \n+\tif (ref_storage_format) {\n+\t\topt.ref_storage_format = ref_storage_format_by_name(ref_storage_format);\n+\t\tif (opt.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_storage_format);\n+\t}\n+\n \topt.filter_options = &filter_options;\n \topt.prefix = prefix;\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fd588b1864..448d58b18b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -290,6 +290,14 @@ cmd_update()\n \t\t-r|--rebase)\n \t\t\trebase=1\n \t\t\t;;\n+\t\t--ref-format)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tref_format=\"--ref-format=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--ref-format=*)\n+\t\t\tref_format=\"$1\"\n+\t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\treference=\"--reference=$2\"\n@@ -371,6 +379,7 @@ cmd_update()\n \t\t${rebase:+--rebase} \\\n \t\t${merge:+--merge} \\\n \t\t${checkout:+--checkout} \\\n+\t\t${ref_format:+\"$ref_format\"} \\\n \t\t${reference:+\"$reference\"} \\\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+\"$depth\"} \\\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nnew file mode 100755\nindex 0000000000..de84007554\n--- /dev/null\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -0,0 +1,41 @@\n+#!/bin/sh\n+\n+test_description='submodules handle mixed ref storage formats'\n+\n+. ./test-lib.sh\n+\n+test_ref_format () {\n+\techo \"$2\" >expect &&\n+\tgit -C \"$1\" rev-parse --show-ref-format >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+for OTHER_FORMAT in files reftable\n+do\n+\tif test \"$OTHER_FORMAT\" = \"$GIT_DEFAULT_REF_FORMAT\"\n+\tthen\n+\t\tcontinue\n+\tfi\n+\n+test_expect_success 'setup' '\n+\tgit config set --global protocol.file.allow always\n+'\n+\n+test_expect_success 'clone submodules with different ref storage format' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -m \"upstream submodule\" &&\n+\n+\tgit clone --no-recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n+\ttest_ref_format downstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\tgit -C downstream submodule update --init --ref-format=$OTHER_FORMAT &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n+'\n+\n+done\n+\n+test_done\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500409","messageId":"ed314f533355dc38bf8acbf7a69651e47eb8f0f7.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 3/8] builtin/clone: propagate ref storage format to submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:37Z","receivedAt":"2024-08-08T07:35:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When recursively cloning a repository with a non-default ref storage\nformat, e.g. by passing the `--ref-format=` option, then only the\ntop-level repository will end up will end up using that ref storage\nformat. All recursively cloned submodules will instead use the default\nformat. While mixed-format constellations are expected to work alright,\nthe outcome still is somewhat surprising as we have essentially ignored\nthe user's request.\n\nFix this by propagating the requested ref format to cloned submodules.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/clone.c | 10 ++++++++--\n t/t7424-submodule-mixed-ref-formats.sh | 23 +++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex af6017d41a..75b15b5773 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -729,7 +729,8 @@ static int git_sparse_checkout_init(const char *repo)\n \treturn result;\n }\n \n-static int checkout(int submodule_progress, int filter_submodules)\n+static int checkout(int submodule_progress, int filter_submodules,\n+\t\t enum ref_storage_format ref_storage_format)\n {\n \tstruct object_id oid;\n \tchar *head;\n@@ -813,6 +814,10 @@ static int checkout(int submodule_progress, int filter_submodules)\n \t\t\tstrvec_push(&cmd.args, \"--no-fetch\");\n \t\t}\n \n+\t\tif (ref_storage_format != REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tstrvec_pushf(&cmd.args, \"--ref-format=%s\",\n+\t\t\t\t ref_storage_format_to_name(ref_storage_format));\n+\n \t\tif (filter_submodules && filter_options.choice)\n \t\t\tstrvec_pushf(&cmd.args, \"--filter=%s\",\n \t\t\t\t expand_list_objects_filter_spec(&filter_options));\n@@ -1536,7 +1541,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \n \tjunk_mode = JUNK_LEAVE_REPO;\n-\terr = checkout(submodule_progress, filter_submodules);\n+\terr = checkout(submodule_progress, filter_submodules,\n+\t\t ref_storage_format);\n \n \tfree(remote_name);\n \tstrbuf_release(&reflog_msg);\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex de84007554..4e4991d04c 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -21,6 +21,29 @@ test_expect_success 'setup' '\n \tgit config set --global protocol.file.allow always\n '\n \n+test_expect_success 'recursive clone propagates ref storage format' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -am \"add submodule\" &&\n+\n+\t# The upstream repository and its submodule should be using the default\n+\t# ref format.\n+\ttest_ref_format upstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format upstream/submodule \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\n+\t# The cloned repositories should use the other ref format that we have\n+\t# specified via `--ref-format`. The option should propagate to cloned\n+\t# submodules.\n+\tgit clone --ref-format=$OTHER_FORMAT --recurse-submodules \\\n+\t\tupstream downstream &&\n+\ttest_ref_format downstream \"$OTHER_FORMAT\" &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n+'\n+\n test_expect_success 'clone submodules with different ref storage format' '\n \ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n \n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500410","messageId":"f13356581ef984fa986f704343c895c4a57a0e4b.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 4/8] refs: fix ref storage format for submodule ref stores","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:42Z","receivedAt":"2024-08-08T07:35:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When opening a submodule ref storage we accidentally use the ref storage\nformat of the owning repository, not of the submodule repository. As\nsubmodules may have a different storage format than their parent repo\nthis can lead to bugs when trying to access the submodule ref storage\nfrom the parent repository.\n\nOne such bug was reported when performing a recursive pull with mixed\nref stores, which fails with:\n\n $ git pull --recursive\n fatal: Unable to find current revision in submodule path 'path/to/sub'\n\nThe same issue occurs when adding a repository contained in the working\ntree with a different ref storage format via `git submodule add`.\n\nFix the bug by using the submodule repository's ref storage format\ninstead and add some tests. Note that the test for `git submodule\nstatus` was included as a precaution, only. The command worked alright\neven without the bugfix.\n\nReported-by: Jeppe Øland \nSigned-off-by: Patrick Steinhardt \n---\n refs.c | 2 +-\n t/t7424-submodule-mixed-ref-formats.sh | 70 +++++++++++++++++++++++++-\n 2 files changed, 70 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 915aeb4d1d..e4b1f4f8b1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2011,7 +2011,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,\n \t\tfree(subrepo);\n \t\tgoto done;\n \t}\n-\trefs = ref_store_init(subrepo, the_repository->ref_storage_format,\n+\trefs = ref_store_init(subrepo, subrepo->ref_storage_format,\n \t\t\t submodule_sb.buf,\n \t\t\t REF_STORE_READ | REF_STORE_ODB);\n \tregister_ref_store_map(&repo->submodule_ref_stores, \"submodule\",\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex 4e4991d04c..d4e184970a 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -18,7 +18,23 @@ do\n \tfi\n \n test_expect_success 'setup' '\n-\tgit config set --global protocol.file.allow always\n+\tgit config set --global protocol.file.allow always &&\n+\t# Some tests migrate the ref storage format, which does not work with\n+\t# reflogs at the time of writing these tests.\n+\tgit config set --global core.logAllRefUpdates false\n+'\n+\n+test_expect_success 'add existing repository with different ref storage format' '\n+\ttest_when_finished \"rm -rf parent\" &&\n+\n+\tgit init parent &&\n+\t(\n+\t\tcd parent &&\n+\t\ttest_commit parent &&\n+\t\tgit init --ref-format=$OTHER_FORMAT submodule &&\n+\t\ttest_commit -C submodule submodule &&\n+\t\tgit submodule add ./submodule\n+\t)\n '\n \n test_expect_success 'recursive clone propagates ref storage format' '\n@@ -59,6 +75,58 @@ test_expect_success 'clone submodules with different ref storage format' '\n \ttest_ref_format downstream/submodule \"$OTHER_FORMAT\"\n '\n \n+test_expect_success 'status with mixed submodule ref storages' '\n+\ttest_when_finished \"rm -rf submodule main\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init main &&\n+\tgit -C main submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C main commit -m \"add submodule\" &&\n+\tgit -C main/submodule refs migrate --ref-format=$OTHER_FORMAT &&\n+\n+\t# The main repository should use the default ref format now, whereas\n+\t# the submodule should use the other format.\n+\ttest_ref_format main \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format main/submodule \"$OTHER_FORMAT\" &&\n+\n+\tcat >expect <<-EOF &&\n+\t $(git -C main/submodule rev-parse HEAD) submodule (submodule-initial)\n+\tEOF\n+\tgit -C main submodule status >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'recursive pull with mixed formats' '\n+\ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n+\n+\t# Set up the initial structure with an upstream repository that has a\n+\t# submodule, as well as a downstream clone of the upstream repository.\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\tgit -C upstream submodule add \"file://$(pwd)/submodule\" &&\n+\tgit -C upstream commit -m \"upstream submodule\" &&\n+\n+\t# Clone the upstream repository such that the main repo and its\n+\t# submodules have different formats.\n+\tgit clone --no-recurse-submodules \"file://$(pwd)/upstream\" downstream &&\n+\tgit -C downstream submodule update --init --ref-format=$OTHER_FORMAT &&\n+\ttest_ref_format downstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\ttest_ref_format downstream/submodule \"$OTHER_FORMAT\" &&\n+\n+\t# Update the upstream submodule as well as the owning repository such\n+\t# that we can do a recursive pull.\n+\ttest_commit -C submodule submodule-update &&\n+\tgit -C upstream/submodule pull &&\n+\tgit -C upstream commit -am \"update the submodule\" &&\n+\n+\tgit -C downstream pull --recurse-submodules &&\n+\tgit -C upstream/submodule rev-parse HEAD >expect &&\n+\tgit -C downstream/submodule rev-parse HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n done\n \n test_done\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500411","messageId":"4ce17e44a16335adf9423a227047d3810608aae4.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 5/8] builtin/submodule: allow \"add\" to use different ref storage format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:47Z","receivedAt":"2024-08-08T07:35:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Same as with \"clone\", users may want to add a submodule to a repository\nwith a non-default ref storage format. Wire up a new `--ref-format=`\noption that works the same as for `git submodule clone`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-submodule.txt | 5 ++++-\n builtin/submodule--helper.c | 16 +++++++++++++++-\n git-submodule.sh | 9 +++++++++\n t/t7424-submodule-mixed-ref-formats.sh | 11 +++++++++++\n 4 files changed, 39 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 73ef8b9696..87d8e0f0c5 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -34,7 +34,7 @@ COMMANDS\n With no arguments, shows the status of existing submodules. Several\n subcommands are available to perform operations on the submodules.\n \n-add [-b ] [-f|--force] [--name ] [--reference ] [--depth ] [--] []::\n+add [-b ] [-f|--force] [--name ] [--reference ] [--ref-format ] [--depth ] [--] []::\n \tAdd the given repository as a submodule at the given path\n \tto the changeset to be committed next to the current\n \tproject: the current project is termed the \"superproject\".\n@@ -71,6 +71,9 @@ submodule repositories will be kept together in the same relative\n location, and only the superproject's URL needs to be provided.\n git-submodule will correctly locate the submodule using the relative\n URL in `.gitmodules`.\n++\n+If `--ref-format ` is specified, the ref storage format of newly\n+cloned submodules will be set accordingly.\n \n status [--cached] [--recursive] [--] [...]::\n \tShow the status of the submodules. This will print the SHA-1 of the\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 42a36bc2f7..48f4577b53 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3128,13 +3128,17 @@ struct add_data {\n \tconst char *sm_name;\n \tconst char *repo;\n \tconst char *realrepo;\n+\tenum ref_storage_format ref_storage_format;\n \tint depth;\n \tunsigned int force: 1;\n \tunsigned int quiet: 1;\n \tunsigned int progress: 1;\n \tunsigned int dissociate: 1;\n };\n-#define ADD_DATA_INIT { .depth = -1 }\n+#define ADD_DATA_INIT { \\\n+\t.depth = -1, \\\n+\t.ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN, \\\n+}\n \n static void append_fetch_remotes(struct strbuf *msg, const char *git_dir_path)\n {\n@@ -3228,6 +3232,7 @@ static int add_submodule(const struct add_data *add_data)\n \n \t\t\tstring_list_append(&reference, p)->util = p;\n \t\t}\n+\t\tclone_data.ref_storage_format = add_data->ref_storage_format;\n \t\tclone_data.dissociate = add_data->dissociate;\n \t\tif (add_data->depth >= 0)\n \t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n@@ -3392,6 +3397,7 @@ static int module_add(int argc, const char **argv, const char *prefix)\n {\n \tint force = 0, quiet = 0, progress = 0, dissociate = 0;\n \tstruct add_data add_data = ADD_DATA_INIT;\n+\tconst char *ref_storage_format = NULL;\n \tchar *to_free = NULL;\n \tstruct option options[] = {\n \t\tOPT_STRING('b', \"branch\", &add_data.branch, N_(\"branch\"),\n@@ -3402,6 +3408,8 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"progress\", &progress, N_(\"force cloning progress\")),\n \t\tOPT_STRING(0, \"reference\", &add_data.reference_path, N_(\"repository\"),\n \t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"ref-format\", &ref_storage_format, N_(\"format\"),\n+\t\t\t N_(\"specify the reference format to use\")),\n \t\tOPT_BOOL(0, \"dissociate\", &dissociate, N_(\"borrow the objects from reference repositories\")),\n \t\tOPT_STRING(0, \"name\", &add_data.sm_name, N_(\"name\"),\n \t\t\t N_(\"sets the submodule's name to the given string \"\n@@ -3428,6 +3436,12 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \tif (argc == 0 || argc > 2)\n \t\tusage_with_options(usage, options);\n \n+\tif (ref_storage_format) {\n+\t\tadd_data.ref_storage_format = ref_storage_format_by_name(ref_storage_format);\n+\t\tif (add_data.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)\n+\t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_storage_format);\n+\t}\n+\n \tadd_data.repo = argv[0];\n \tif (argc == 1)\n \t\tadd_data.sm_path = git_url_basename(add_data.repo, 0, 0);\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 448d58b18b..03c5a220a2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -94,6 +94,14 @@ cmd_add()\n \t\t--reference=*)\n \t\t\treference_path=\"${1#--reference=}\"\n \t\t\t;;\n+\t\t--ref-format)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tref_format=\"--ref-format=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--ref-format=*)\n+\t\t\tref_format=\"$1\"\n+\t\t\t;;\n \t\t--dissociate)\n \t\t\tdissociate=1\n \t\t\t;;\n@@ -135,6 +143,7 @@ cmd_add()\n \t\t${progress:+\"--progress\"} \\\n \t\t${branch:+--branch \"$branch\"} \\\n \t\t${reference_path:+--reference \"$reference_path\"} \\\n+\t\t${ref_format:+\"$ref_format\"} \\\n \t\t${dissociate:+--dissociate} \\\n \t\t${custom_name:+--name \"$custom_name\"} \\\n \t\t${depth:+\"$depth\"} \\\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex d4e184970a..559713b607 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -37,6 +37,17 @@ test_expect_success 'add existing repository with different ref storage format'\n \t)\n '\n \n+test_expect_success 'add submodules with different ref storage format' '\n+\ttest_when_finished \"rm -rf submodule upstream\" &&\n+\n+\tgit init submodule &&\n+\ttest_commit -C submodule submodule-initial &&\n+\tgit init upstream &&\n+\ttest_ref_format upstream \"$GIT_DEFAULT_REF_FORMAT\" &&\n+\tgit -C upstream submodule add --ref-format=\"$OTHER_FORMAT\" \"file://$(pwd)/submodule\" &&\n+\ttest_ref_format upstream/submodule \"$OTHER_FORMAT\"\n+'\n+\n test_expect_success 'recursive clone propagates ref storage format' '\n \ttest_when_finished \"rm -rf submodule upstream downstream\" &&\n \n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500412","messageId":"d92770290f6fa3f1296f1ff9bdf8046543b6c34f.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 6/8] submodule: fix leaking fetch tasks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:51Z","receivedAt":"2024-08-08T07:35:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When done with a fetch task used for parallel fetches of submodules, we\nneed to both call `fetch_task_release()` to release the task's contents\nand `free()` to release the task itself. Most sites do this already, but\nsome only call `fetch_task_release()` and thus leak memory.\n\nWhile we could trivially fix this by adding the two missing calls to\nfree(3P), the result would be that we always call both functions. Let's\nthus refactor the code such that `fetch_task_release()` also frees the\nstructure itself. Rename it to `fetch_task_free()` accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n submodule.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex ab99a30253..f027a6455e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1496,7 +1496,7 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path)\n \treturn (const struct submodule *) ret;\n }\n \n-static void fetch_task_release(struct fetch_task *p)\n+static void fetch_task_free(struct fetch_task *p)\n {\n \tif (p->free_sub)\n \t\tfree((void*)p->sub);\n@@ -1508,6 +1508,7 @@ static void fetch_task_release(struct fetch_task *p)\n \tFREE_AND_NULL(p->repo);\n \n \tstrvec_clear(&p->git_args);\n+\tfree(p);\n }\n \n static struct repository *get_submodule_repo_for(struct repository *r,\n@@ -1576,8 +1577,7 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf\n \treturn task;\n \n cleanup:\n-\tfetch_task_release(task);\n-\tfree(task);\n+\tfetch_task_free(task);\n \treturn NULL;\n }\n \n@@ -1607,8 +1607,7 @@ get_fetch_task_from_index(struct submodule_parallel_fetch *spf,\n \t\t} else {\n \t\t\tstruct strbuf empty_submodule_path = STRBUF_INIT;\n \n-\t\t\tfetch_task_release(task);\n-\t\t\tfree(task);\n+\t\t\tfetch_task_free(task);\n \n \t\t\t/*\n \t\t\t * An empty directory is normal,\n@@ -1654,8 +1653,7 @@ get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,\n \t\t\t\t cs_data->path,\n \t\t\t\t repo_find_unique_abbrev(the_repository, cs_data->super_oid, DEFAULT_ABBREV));\n \n-\t\t\tfetch_task_release(task);\n-\t\t\tfree(task);\n+\t\t\tfetch_task_free(task);\n \t\t\tcontinue;\n \t\t}\n \n@@ -1763,7 +1761,7 @@ static int fetch_start_failure(struct strbuf *err UNUSED,\n \n \tspf->result = 1;\n \n-\tfetch_task_release(task);\n+\tfetch_task_free(task);\n \treturn 0;\n }\n \n@@ -1828,8 +1826,7 @@ static int fetch_finish(int retvalue, struct strbuf *err UNUSED,\n \t}\n \n out:\n-\tfetch_task_release(task);\n-\n+\tfetch_task_free(task);\n \treturn 0;\n }\n \n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500413","messageId":"e9421189ca0886e81511e191ace1af6e65c0e292.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 7/8] submodule: fix leaking seen submodule names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:35:56Z","receivedAt":"2024-08-08T07:35:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We keep track of submodules we have already seen via a string map such\nthat we don't process the same submodule twice. We never free that map\nthough, causing a memory leak.\n\nFix this leak by clearing the map.\n\nSigned-off-by: Patrick Steinhardt \n---\n submodule.c | 1 +\n t/t5572-pull-submodule.sh | 1 +\n t/t7418-submodule-sparse-gitmodules.sh | 1 +\n 3 files changed, 3 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex f027a6455e..13b8f8c19c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1880,6 +1880,7 @@ int fetch_submodules(struct repository *r,\n \tstrvec_clear(&spf.args);\n out:\n \tfree_submodules_data(&spf.changed_submodule_names);\n+\tstring_list_clear(&spf.seen_submodule_names, 0);\n \treturn spf.result;\n }\n \ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex 51744521f7..916e58c166 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -5,6 +5,7 @@ test_description='pull can handle submodules'\n GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1\n export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \ndiff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh\nindex dde11ecce8..e1d9bf2ee3 100755\n--- a/t/t7418-submodule-sparse-gitmodules.sh\n+++ b/t/t7418-submodule-sparse-gitmodules.sh\n@@ -15,6 +15,7 @@ also by committing .gitmodules and then just removing it from the filesystem.\n GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1\n export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500414","messageId":"d05737c75f0e027e2134ec5ace1899aff2bc97ab.1723102259.git.ps@pks.im","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"[PATCH v2 8/8] object: fix leaking packfiles when closing object store","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T07:36:00Z","receivedAt":"2024-08-08T07:36:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `raw_object_store_clear()`, we close and free several\nresources associated with the object store. Part of that is to close and\nfree all the packfiles, which is handled by `close_object_store()`.\n\nThat function really only ends up closing the packfiles though, but it\ndoesn't free them. And in fact it can't, as that function is being\ncalled via `run_command()` when `close_object_store = 1`, which is done\ne.g. when we execute git-maintenance(1). At that point, other structures\nmay still have references on those packfiles, and thus we cannot free\nthem here. So while it is in fact intentional that we really only close\nthem, the result is a memory leak because `raw_object_store_clear()`\ndoes not free them, either.\n\nFix the leak by freeing the packfiles in `raw_object_store_clear()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n object.c | 9 +++++++++\n t/t7424-submodule-mixed-ref-formats.sh | 1 +\n 2 files changed, 10 insertions(+)\n\ndiff --git a/object.c b/object.c\nindex 0c0fcb76c0..d756c7f2ea 100644\n--- a/object.c\n+++ b/object.c\n@@ -614,6 +614,15 @@ void raw_object_store_clear(struct raw_object_store *o)\n \n \tINIT_LIST_HEAD(&o->packed_git_mru);\n \tclose_object_store(o);\n+\n+\t/*\n+\t * `close_object_store()` only closes the packfiles, but doesn't free\n+\t * them. We thus have to do this manually.\n+\t */\n+\tfor (struct packed_git *p = o->packed_git, *next; p; p = next) {\n+\t\tnext = p->next;\n+\t\tfree(p);\n+\t}\n \to->packed_git = NULL;\n \n \thashmap_clear(&o->pack_map);\ndiff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh\nindex 559713b607..b43ef2ba67 100755\n--- a/t/t7424-submodule-mixed-ref-formats.sh\n+++ b/t/t7424-submodule-mixed-ref-formats.sh\n@@ -2,6 +2,7 @@\n \n test_description='submodules handle mixed ref storage formats'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_ref_format () {\n-- \n2.46.0.46.g406f326d27.dirty\n\n"},{"id":"500416","messageId":"24dad55d-584f-4a38-b6c0-dbedf91fdfc6@app.fastmail.com","threadId":"61901","inReplyTo":"ed314f533355dc38bf8acbf7a69651e47eb8f0f7.1723102259.git.ps@pks.im","subject":"Re: [PATCH v2 3/8] builtin/clone: propagate ref storage format to submodules","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-08-08T08:03:57Z","receivedAt":"2024-08-08T08:07:11Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Morning\n\nOn Thu, Aug 8, 2024, at 09:35, Patrick Steinhardt wrote:\n> When recursively cloning a repository with a non-default ref storage\n> format, e.g. by passing the `--ref-format=` option, then only the\n> top-level repository will end up will end up\n\nDouble “will end up”.\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"500465","messageId":"ZrTISl6uyaPq43-K@tanuki","threadId":"61901","inReplyTo":"24dad55d-584f-4a38-b6c0-dbedf91fdfc6@app.fastmail.com","subject":"Re: [PATCH v2 3/8] builtin/clone: propagate ref storage format to submodules","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T13:29:46Z","receivedAt":"2024-08-08T13:29:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Aug 08, 2024 at 10:03:57AM +0200, Kristoffer Haugsbakk wrote:\n> Morning\n> \n> On Thu, Aug 8, 2024, at 09:35, Patrick Steinhardt wrote:\n> > When recursively cloning a repository with a non-default ref storage\n> > format, e.g. by passing the `--ref-format=` option, then only the\n> > top-level repository will end up will end up\n> \n> Double “will end up”.\n\nThanks. I've fixed this locally, but will wait for additional comments\nto come in first.\n\nPatrick\n"},{"id":"500485","messageId":"xmqqsevfj8v2.fsf@gitster.g","threadId":"61901","inReplyTo":"ZrRtB46vu9rC000y@tanuki","subject":"Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-08T16:08:01Z","receivedAt":"2024-08-08T16:08:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> Presumably, if the named submodule has already been initialized, we\n>> are not converting its ref backend with --ref-format= option\n>> when \"git submodule update --ref-format=\" is run. Would it\n>> make sense to say it is an error to give it without \"--init\", I\n>> wonder. If so, we probably would need to see if other existing\n>> options like \"--filter\" also need a similar sanity check, if not\n>> already done.\n>\n> Well, even when \"--init\" was given it is not sure whether the ref\n> storage format will actually end up being used, because that option only\n> tells us to initialize uninitialized submodules. So if the submodule was\n> initialized already, then the ref storage format won't be used.\n>\n> We probably could add such a sanity check. But as you say, other options\n> like \"--filter\", \"--depth\", \"--reference\" and \"--recommend-shallow\"\n> don't have that check, either, so it would feel a bit like opening a can\n> of worms. So personally, I'd rather defer this to another day where we\n> then implement the check for all of these options.\n\nI am perfectly fine if we stopped by clearly documenting that these\noptions can be no-op when the submodule repository already exists.\nFailing the operation in the name of \"sanity check\" at this point,\nespecially for existing options, does not sound like a sensible\nchange.\n\nThanks.\n"},{"id":"500488","messageId":"E664DE72-49F5-4AFC-ACDB-0BB60A6C34C2@pks.im","threadId":"61901","inReplyTo":"xmqqsevfj8v2.fsf@gitster.g","subject":"Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-08T16:19:01Z","receivedAt":"2024-08-08T16:19:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On August 8, 2024 6:08:01 PM GMT+02:00, Junio C Hamano wrote:\n>Patrick Steinhardt writes:\n>\n>>> Presumably, if the named submodule has already been initialized, we\n>>> are not converting its ref backend with --ref-format= option\n>>> when \"git submodule update --ref-format=\" is run. Would it\n>>> make sense to say it is an error to give it without \"--init\", I\n>>> wonder. If so, we probably would need to see if other existing\n>>> options like \"--filter\" also need a similar sanity check, if not\n>>> already done.\n>>\n>> Well, even when \"--init\" was given it is not sure whether the ref\n>> storage format will actually end up being used, because that option only\n>> tells us to initialize uninitialized submodules. So if the submodule was\n>> initialized already, then the ref storage format won't be used.\n>>\n>> We probably could add such a sanity check. But as you say, other options\n>> like \"--filter\", \"--depth\", \"--reference\" and \"--recommend-shallow\"\n>> don't have that check, either, so it would feel a bit like opening a can\n>> of worms. So personally, I'd rather defer this to another day where we\n>> then implement the check for all of these options.\n>\n>I am perfectly fine if we stopped by clearly documenting that these\n>options can be no-op when the submodule repository already exists.\n>Failing the operation in the name of \"sanity check\" at this point,\n>especially for existing options, does not sound like a sensible\n>change.\n>\n>Thanks.\n\nThe documentation already points this out, as we explicitly say that it\nonly has an impact on newly cloned submodules:\n\n> If `--ref-format ` is specified, the ref storage format of newly\ncloned submodules will be set accordingly.\n\nI'm happy to make this more explicit though, in case you don't think\nthis is sufficient.\n\nPatrick \n"},{"id":"500490","messageId":"xmqqsevfhtio.fsf@gitster.g","threadId":"61901","inReplyTo":"cover.1723102259.git.ps@pks.im","subject":"Re: [PATCH v2 0/8] Improvements for ref storage formats with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-08T16:24:47Z","receivedAt":"2024-08-08T16:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> - Add a preparatory refactoring that wraps overly long command lines\n> in \"git-submodule.sh\".\n\nThis is very much appreciated.\n>\n> - Wire up support for `--ref-format=` in `git submodule add`.\n\nGood find. Thanks.\n"},{"id":"500511","messageId":"xmqqmslngc3u.fsf@gitster.g","threadId":"61901","inReplyTo":"E664DE72-49F5-4AFC-ACDB-0BB60A6C34C2@pks.im","subject":"Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-08T17:26:13Z","receivedAt":"2024-08-08T17:26:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> The documentation already points this out, as we explicitly say that it\n> only has an impact on newly cloned submodules:\n\nPerfect. Thanks.\n"}]}