{"thread":{"id":"48657","subject":"BUG: submodule code prints '(null)'","startedAt":"2018-06-05T15:32:16Z","lastAt":"2018-06-17T14:03:02Z","messageCount":12,"participants":["Duy Nguyen","Kaartic Sivaraam","Stefan Beller","Heiko Voigt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"349350","messageId":"CACsJy8CNrQ-CKoJ+1NCR1rsO+v0ZNZ9CVAFsJpmcRWZY6HUtKw@mail.gmail.com","threadId":"48657","inReplyTo":null,"subject":"BUG: submodule code prints '(null)'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-06-05T15:31:41Z","receivedAt":"2018-06-05T15:32:16Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I do not know how to reproduce this (and didn't bother to look deeply\ninto it after I found it was not a trivial fix) but one of my \"git\nfetch\" showed\n\nwarning: Submodule in commit be2db96a6c506464525f588da59cade0cedddb5e\nat path: '(null)' collides with a submodule named the same. Skipping\nit.\n\nI think it's reported that some libc implementation will not be able\nto gracefully handle NULL strings like glibc and may crash instead of\nprinting '(null)' here. I'll leave it to submodule people to fix this\n:)\n-- \nDuy\n"},{"id":"349541","messageId":"91ba2ec1-ba0f-e7ff-6533-ee3e2aa7ddd4@gmail.com","threadId":"48657","inReplyTo":"CACsJy8CNrQ-CKoJ+1NCR1rsO+v0ZNZ9CVAFsJpmcRWZY6HUtKw@mail.gmail.com","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-06-06T18:32:33Z","receivedAt":"2018-06-06T18:32:42Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Tuesday 05 June 2018 09:01 PM, Duy Nguyen wrote:\n> I'll leave it to submodule people to fix this :)\n> \n\nI'm Ccing the only one I know to gain attention.\n\n\n-- \nSivaraam\n\nQUOTE:\n\n“The three principal virtues of a programmer are Laziness, Impatience,\nand Hubris.”\n\n\t- Camel book\n\nSivaraam?\n\nYou possibly might have noticed that my signature recently changed from\n'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the\nnew signature to be better for several reasons one of which is that the\nformer signature has a lot of ambiguities in the place I live as it is a\ncommon name (NOTE: it's not a common spelling, just a common name). So,\nI switched signatures before it's too late.\n\nThat said, I won't mind you calling me 'Kaartic' if you like it [of\ncourse ;-)]. You can always call me using either of the names.\n\n\nKIND NOTE TO THE NATIVE ENGLISH SPEAKER:\n\nAs I'm not a native English speaker myself, there might be mistaeks in\nmy usage of English. I apologise for any mistakes that I make.\n\nIt would be \"helpful\" if you take the time to point out the mistakes.\n\nIt would be \"super helpful\" if you could provide suggestions about how\nto correct those mistakes.\n\nThanks in advance!\n\n"},{"id":"349796","messageId":"20180609110414.GA5273@duynguyen.home","threadId":"48657","inReplyTo":"CACsJy8CNrQ-CKoJ+1NCR1rsO+v0ZNZ9CVAFsJpmcRWZY6HUtKw@mail.gmail.com","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-06-09T11:04:15Z","receivedAt":"2018-06-09T11:04:22Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 05, 2018 at 05:31:41PM +0200, Duy Nguyen wrote:\n> I do not know how to reproduce this (and didn't bother to look deeply\n> into it after I found it was not a trivial fix) but one of my \"git\n> fetch\" showed\n> \n> warning: Submodule in commit be2db96a6c506464525f588da59cade0cedddb5e\n> at path: '(null)' collides with a submodule named the same. Skipping\n> it.\n\nThe problem is default_name_or_path() can return NULL when a submodule\nis not populated. The fix could simply be printing path instead of\nname (because we are talking about path in the commit message), like\nbelow.\n\nBut I don't really understand c68f837576 (implement fetching of moved\nsubmodules - 2017-10-16), the commit that made this change, and not\nsure if we should be reporting name here or path. Heiko?\n\ndiff --git a/submodule.c b/submodule.c\nindex 939d6870ec..61c2177755 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -745,7 +745,7 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n \t\t\t\twarning(\"Submodule in commit %s at path: \"\n \t\t\t\t\t\"'%s' collides with a submodule named \"\n \t\t\t\t\t\"the same. Skipping it.\",\n-\t\t\t\t\toid_to_hex(commit_oid), name);\n+\t\t\t\t\toid_to_hex(commit_oid), p->two->path);\n \t\t\t\tname = NULL;\n \t\t\t}\n \t\t}\n\n\n\n> \n> I think it's reported that some libc implementation will not be able\n> to gracefully handle NULL strings like glibc and may crash instead of\n> printing '(null)' here. I'll leave it to submodule people to fix this\n> :)\n> -- \n> Duy\n"},{"id":"349956","messageId":"CAGZ79kZk=OGPJdsDEHtgmPUrO7P2rOLSV40aJawdLs5e0=Kduw@mail.gmail.com","threadId":"48657","inReplyTo":"20180609110414.GA5273@duynguyen.home","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-06-11T22:56:16Z","receivedAt":"2018-06-11T22:56:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Jun 9, 2018 at 4:04 AM Duy Nguyen wrote:\n>\n> On Tue, Jun 05, 2018 at 05:31:41PM +0200, Duy Nguyen wrote:\n> > I do not know how to reproduce this (and didn't bother to look deeply\n> > into it after I found it was not a trivial fix) but one of my \"git\n> > fetch\" showed\n> >\n> > warning: Submodule in commit be2db96a6c506464525f588da59cade0cedddb5e\n> > at path: '(null)' collides with a submodule named the same. Skipping\n> > it.\n>\n> The problem is default_name_or_path() can return NULL when a submodule\n> is not populated. The fix could simply be printing path instead of\n> name (because we are talking about path in the commit message), like\n> below.\n>\n> But I don't really understand c68f837576 (implement fetching of moved\n> submodules - 2017-10-16), the commit that made this change, and not\n> sure if we should be reporting name here or path. Heiko?\n>\n> diff --git a/submodule.c b/submodule.c\n> index 939d6870ec..61c2177755 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -745,7 +745,7 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n\n[Not in the context of this patch, but in the code right before the\ncontext starts:]\n\n name = default_name_or_path(p->two->path);\n /* make sure name does not collide with existing one */\n submodule = submodule_from_name(the_repository, commit_oid, name);\n if (submodule) {\n\nCurrently I see 4 callers of default_name_or_path and the other 3 except this\none have checks against NULL in place, which is good.\nHowever I think we have to guard this even more, and have to check\nfor !name before we call submodule_from_name.\n\nIt is technically ok to call submodule_from_name with a NULL name,\nbut it is semantically broken, see the comment in config_from that\nis called from submodule_from_name:\n\n /*\n * If any parameter except the cache is a NULL pointer just\n * return the first submodule. Can be used to check whether\n * there are any submodules parsed.\n */\n if (!treeish_name || !key) {\n ...\n\n\n> warning(\"Submodule in commit %s at path: \"\n> \"'%s' collides with a submodule named \"\n> \"the same. Skipping it.\",\n> - oid_to_hex(commit_oid), name);\n> + oid_to_hex(commit_oid), p->two->path);\n\nThis is correct for the error message, both in terms of not crashing as well\nas correctness, we really need to report a *path* here and no the name_or_path,\nwhich default_name_or_path gives.\n"},{"id":"349993","messageId":"CAGZ79kb3_0W7osWbU4tcvGvy0KVQJBpFD7q6njTjWJ7vOEmrtg@mail.gmail.com","threadId":"48657","inReplyTo":"20180609110414.GA5273@duynguyen.home","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-06-12T17:35:59Z","receivedAt":"2018-06-12T17:36:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Jun 9, 2018 at 4:04 AM Duy Nguyen wrote:\n>\n> On Tue, Jun 05, 2018 at 05:31:41PM +0200, Duy Nguyen wrote:\n> > I do not know how to reproduce this (and didn't bother to look deeply\n> > into it after I found it was not a trivial fix) but one of my \"git\n> > fetch\" showed\n> >\n> > warning: Submodule in commit be2db96a6c506464525f588da59cade0cedddb5e\n> > at path: '(null)' collides with a submodule named the same. Skipping\n> > it.\n>\n> The problem is default_name_or_path() can return NULL when a submodule\n> is not populated. The fix could simply be printing path instead of\n> name (because we are talking about path in the commit message), like\n> below.\n>\n> But I don't really understand c68f837576 (implement fetching of moved\n> submodules - 2017-10-16), the commit that made this change, and not\n> sure if we should be reporting name here or path. Heiko?\n\nThat change is quite interesting as I did not understand it at first\nsight as well.\nSee https://public-inbox.org/git/20171016135827.GC12756@book.hvoigt.net/\nand the follow ups, specifically\nhttps://public-inbox.org/git/20171019181109.27792-2-sbeller@google.com/\nthat tries to clean up the code, but was ultimately dropped.\n"},{"id":"350155","messageId":"20180614151521.GA2436@book.hvoigt.net","threadId":"48657","inReplyTo":"CAGZ79kZk=OGPJdsDEHtgmPUrO7P2rOLSV40aJawdLs5e0=Kduw@mail.gmail.com","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-06-14T15:15:21Z","receivedAt":"2018-06-14T15:15:28Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Jun 11, 2018 at 03:56:16PM -0700, Stefan Beller wrote:\n> On Sat, Jun 9, 2018 at 4:04 AM Duy Nguyen wrote:\n> >\n> > On Tue, Jun 05, 2018 at 05:31:41PM +0200, Duy Nguyen wrote:\n> > > I do not know how to reproduce this (and didn't bother to look deeply\n> > > into it after I found it was not a trivial fix) but one of my \"git\n> > > fetch\" showed\n> > >\n> > > warning: Submodule in commit be2db96a6c506464525f588da59cade0cedddb5e\n> > > at path: '(null)' collides with a submodule named the same. Skipping\n> > > it.\n> >\n> > The problem is default_name_or_path() can return NULL when a submodule\n> > is not populated. The fix could simply be printing path instead of\n> > name (because we are talking about path in the commit message), like\n> > below.\n> >\n> > But I don't really understand c68f837576 (implement fetching of moved\n> > submodules - 2017-10-16), the commit that made this change, and not\n> > sure if we should be reporting name here or path. Heiko?\n> >\n> > diff --git a/submodule.c b/submodule.c\n> > index 939d6870ec..61c2177755 100644\n> > --- a/submodule.c\n> > +++ b/submodule.c\n> > @@ -745,7 +745,7 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n> \n> [Not in the context of this patch, but in the code right before the\n> context starts:]\n> \n> name = default_name_or_path(p->two->path);\n> /* make sure name does not collide with existing one */\n> submodule = submodule_from_name(the_repository, commit_oid, name);\n> if (submodule) {\n> \n> Currently I see 4 callers of default_name_or_path and the other 3 except this\n> one have checks against NULL in place, which is good.\n> However I think we have to guard this even more, and have to check\n> for !name before we call submodule_from_name.\n> \n> It is technically ok to call submodule_from_name with a NULL name,\n> but it is semantically broken, see the comment in config_from that\n> is called from submodule_from_name:\n> \n> /*\n> * If any parameter except the cache is a NULL pointer just\n> * return the first submodule. Can be used to check whether\n> * there are any submodules parsed.\n> */\n> if (!treeish_name || !key) {\n> ...\n> \n> \n> > warning(\"Submodule in commit %s at path: \"\n> > \"'%s' collides with a submodule named \"\n> > \"the same. Skipping it.\",\n> > - oid_to_hex(commit_oid), name);\n> > + oid_to_hex(commit_oid), p->two->path);\n> \n> This is correct for the error message, both in terms of not crashing as well\n> as correctness, we really need to report a *path* here and no the name_or_path,\n> which default_name_or_path gives.\n\nSorry for the late reply. I agree with Stefan, this change is correct,\nsince we are talking about the path in the warning message anyway. It\nseems to me that this resulted from the name being the same as the path\nin this location here.\n\nI think we should report both here, if we can, the path and the name.\n\nWe are skipping the submodule if we can not get a name later:\n\n if (!name) \n continue;\n\nI also agree, that it does not make sense to call submodule_from_name\nwith a NULL name. I think we should simply skip calling it in case the\nname is NULL and then let the code later handle it.\n\nE.g.: \n\n...\n /* make sure name does not collide with existing one */\n\t\t if (name)\t\n submodule = submodule_from_name(the_repository, commit_oid, name);\n if (submodule) {\n warning(\"Submodule in commit %s at path: \"\n...\n\nWould you want to update your patch? Or should I put one on top?\n\nCheers Heiko\n"},{"id":"350156","messageId":"CACsJy8Ab3HoVWSWOtCBRYcsnnHnpO-2oEfV60f=H15RuzwpWwQ@mail.gmail.com","threadId":"48657","inReplyTo":"20180614151521.GA2436@book.hvoigt.net","subject":"Re: BUG: submodule code prints '(null)'","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-06-14T15:44:29Z","receivedAt":"2018-06-14T15:44:59Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 14, 2018 at 5:15 PM Heiko Voigt wrote:\n> ...\n> Would you want to update your patch? Or should I put one on top?\n\nI think it's better that you make a proper patch. You can provide\nexplanation and all. I am more like a bug reporter :)\n-- \nDuy\n"},{"id":"350166","messageId":"20180614173107.201885-1-sbeller@google.com","threadId":"48657","inReplyTo":"CACsJy8Ab3HoVWSWOtCBRYcsnnHnpO-2oEfV60f=H15RuzwpWwQ@mail.gmail.com","subject":"[PATCH] submodule: fix NULL correctness in renamed broken submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-06-14T17:31:07Z","receivedAt":"2018-06-14T17:31:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When fetching with recursing into submodules, the fetch logic inspects\nthe superproject which submodules actually need to be fetched. This is\ntricky for submodules that were renamed in the fetched range of commits.\nThis was implemented in c68f8375760 (implement fetching of moved\nsubmodules, 2017-10-16), and this patch fixes a mistake in the logic\nthere.\n\nWhen the warning is printed, the `name` might be NULL as\ndefault_name_or_path can return NULL, so fix the warning to use the path\nas obtained from the diff machinery, as that is not NULL.\n\nWhile at it, make sure we only attempt to load the submodule if a git\ndirectory of the submodule is found as default_name_or_path will return\nNULL in case the git directory cannot be found. Note that passing NULL\nto submodule_from_name is just a semantic error, as submodule_from_name\naccepts NULL as a value, but then the return value is not the submodule\nthat was asked for, but some arbitrary other submodule. (Cf. 'config_from'\nin submodule-config.c: \"If any parameter except the cache is a NULL\npointer just return the first submodule. Can be used to check whether\nthere are any submodules parsed.\")\n\nReported-by: Duy Nguyen \nHelped-by: Duy Nguyen \nHelped-by: Heiko Voigt \nSigned-off-by: Stefan Beller \n---\n submodule.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 939d6870ecd..0998ea23458 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -740,12 +740,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n \t\telse {\n \t\t\tname = default_name_or_path(p->two->path);\n \t\t\t/* make sure name does not collide with existing one */\n-\t\t\tsubmodule = submodule_from_name(the_repository, commit_oid, name);\n+\t\t\tif (name)\n+\t\t\t\tsubmodule = submodule_from_name(the_repository,\n+\t\t\t\t\t\t\t\tcommit_oid, name);\n \t\t\tif (submodule) {\n \t\t\t\twarning(\"Submodule in commit %s at path: \"\n \t\t\t\t\t\"'%s' collides with a submodule named \"\n \t\t\t\t\t\"the same. Skipping it.\",\n-\t\t\t\t\toid_to_hex(commit_oid), name);\n+\t\t\t\t\toid_to_hex(commit_oid), p->two->path);\n \t\t\t\tname = NULL;\n \t\t\t}\n \t\t}\n-- \n2.18.0.rc1.244.gcf134e6275-goog\n\n"},{"id":"350171","messageId":"20180614173730.205646-1-sbeller@google.com","threadId":"48657","inReplyTo":"20180614173107.201885-1-sbeller@google.com","subject":"[PATCH] t5526: test recursive submodules when fetching moved submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-06-14T17:37:30Z","receivedAt":"2018-06-14T17:37:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The topic merged in 0c7ecb7c311 (Merge branch 'sb/submodule-move-nested',\n2018-05-08) provided support for moving nested submodules.\n\nRemove the NEEDSWORK comment and implement the nested submodules test as\nthe comment hinted at.\n\nSigned-off-by: Stefan Beller \n---\n\nI found this when digging around for the previous patch.\n\nThanks,\nStefan\n\n t/t5526-fetch-submodules.sh | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 9cc4b569c05..359e03ff836 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -574,11 +574,7 @@ test_expect_success \"fetch new commits when submodule got renamed\" '\n \tgit clone . downstream_rename &&\n \t(\n \t\tcd downstream_rename &&\n-\t\tgit submodule update --init &&\n-# NEEDSWORK: we omitted --recursive for the submodule update here since\n-# that does not work. See test 7001 for mv \"moving nested submodules\"\n-# for details. Once that is fixed we should add the --recursive option\n-# here.\n+\t\tgit submodule update --init --recursive &&\n \t\tgit checkout -b rename &&\n \t\tgit mv submodule submodule_renamed &&\n \t\t(\n-- \n2.18.0.rc1.244.gcf134e6275-goog\n\n"},{"id":"350199","messageId":"20180614202409.GC2686@book.hvoigt.net","threadId":"48657","inReplyTo":"20180614173107.201885-1-sbeller@google.com","subject":"Re: [PATCH] submodule: fix NULL correctness in renamed broken submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-06-14T20:24:09Z","receivedAt":"2018-06-14T20:29:00Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Thu, Jun 14, 2018 at 10:31:07AM -0700, Stefan Beller wrote:\n> When fetching with recursing into submodules, the fetch logic inspects\n> the superproject which submodules actually need to be fetched. This is\n> tricky for submodules that were renamed in the fetched range of commits.\n> This was implemented in c68f8375760 (implement fetching of moved\n> submodules, 2017-10-16), and this patch fixes a mistake in the logic\n> there.\n> \n> When the warning is printed, the `name` might be NULL as\n> default_name_or_path can return NULL, so fix the warning to use the path\n> as obtained from the diff machinery, as that is not NULL.\n> \n> While at it, make sure we only attempt to load the submodule if a git\n> directory of the submodule is found as default_name_or_path will return\n> NULL in case the git directory cannot be found. Note that passing NULL\n> to submodule_from_name is just a semantic error, as submodule_from_name\n> accepts NULL as a value, but then the return value is not the submodule\n> that was asked for, but some arbitrary other submodule. (Cf. 'config_from'\n> in submodule-config.c: \"If any parameter except the cache is a NULL\n> pointer just return the first submodule. Can be used to check whether\n> there are any submodules parsed.\")\n> \n> Reported-by: Duy Nguyen \n> Helped-by: Duy Nguyen \n> Helped-by: Heiko Voigt \n> Signed-off-by: Stefan Beller \n> ---\n\nLooks good to me.\n\nCheers Heiko\n"},{"id":"350200","messageId":"20180614202305.GB2686@book.hvoigt.net","threadId":"48657","inReplyTo":"20180614173730.205646-1-sbeller@google.com","subject":"Re: [PATCH] t5526: test recursive submodules when fetching moved submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-06-14T20:23:05Z","receivedAt":"2018-06-14T20:34:40Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Jun 14, 2018 at 10:37:30AM -0700, Stefan Beller wrote:\n> The topic merged in 0c7ecb7c311 (Merge branch 'sb/submodule-move-nested',\n> 2018-05-08) provided support for moving nested submodules.\n> \n> Remove the NEEDSWORK comment and implement the nested submodules test as\n> the comment hinted at.\n> \n> Signed-off-by: Stefan Beller \n> ---\n\nLooks good to me.\n\nCheers Heiko\n"},{"id":"350356","messageId":"9588295f-f809-c95c-664c-c05b04d7fc5c@gmail.com","threadId":"48657","inReplyTo":"20180614173107.201885-1-sbeller@google.com","subject":"Re: [PATCH] submodule: fix NULL correctness in renamed broken submodules","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-06-17T14:02:51Z","receivedAt":"2018-06-17T14:03:02Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thursday 14 June 2018 11:01 PM, Stefan Beller wrote:\n> While at it, make sure we only attempt to load the submodule if a git\n> directory of the submodule is found as default_name_or_path will return\n> NULL in case the git directory cannot be found.\n\nI found this a little hard to read. Maybe it could be sentence could be\nshrinked a little. Possibly,\n\n While at it, make sure we only attempt to load the submodule if\n a git directory of the submodule is found.\n\nI guess the other part of the sentence doesn't make much sense in the\nlog message. Maybe it could be an in-code comment. Speaking of in-code\ncomment, the following would also fit there, wouldn't it?\n\n> Note that passing NULL\n> to submodule_from_name is just a semantic error, as submodule_from_name\n> accepts NULL as a value, but then the return value is not the submodule\n> that was asked for, but some arbitrary other submodule. (Cf. 'config_from'\n> in submodule-config.c: \"If any parameter except the cache is a NULL\n> pointer just return the first submodule. Can be used to check whether\n> there are any submodules parsed.\")\n> \n\n\n-- \nSivaraam\n\nQUOTE:\n\n“The three principal virtues of a programmer are Laziness, Impatience,\nand Hubris.”\n\n\t- Camel book\n\nSivaraam?\n\nYou possibly might have noticed that my signature recently changed from\n'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the\nnew signature to be better for several reasons one of which is that the\nformer signature has a lot of ambiguities in the place I live as it is a\ncommon name (NOTE: it's not a common spelling, just a common name). So,\nI switched signatures before it's too late.\n\nThat said, I won't mind you calling me 'Kaartic' if you like it [of\ncourse ;-)]. You can always call me using either of the names.\n\n\nKIND NOTE TO THE NATIVE ENGLISH SPEAKER:\n\nAs I'm not a native English speaker myself, there might be mistaeks in\nmy usage of English. I apologise for any mistakes that I make.\n\nIt would be \"helpful\" if you take the time to point out the mistakes.\n\nIt would be \"super helpful\" if you could provide suggestions about how\nto correct those mistakes.\n\nThanks in advance!\n\n"}]}