{"thread":{"id":"58408","subject":"Question related to submodule and different recurse config options","startedAt":"2022-09-08T15:01:40Z","lastAt":"2022-09-19T18:44:10Z","messageCount":3,"participants":["Pieter-Jan Busschaert","Philippe Blain","Glen Choo"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"462807","messageId":"CABxXVRS-9unw-8_N8w2bdj4E5Ch780tMetJyD+++ixdWsBv90A@mail.gmail.com","threadId":"58408","inReplyTo":null,"subject":"Question related to submodule and different recurse config options","fromName":"Pieter-Jan Busschaert","fromEmail":"pieterjan.busschaert@gmail.com","sentAt":"2022-09-08T15:01:19Z","receivedAt":"2022-09-08T15:01:40Z","isPatch":false,"sender":{"key":"pieterjan.busschaert@gmail.com","avatar":null},"body":"Hello,\n\nI'm trying to set my git config like this (fetch.recurseSubmodules =\non-demand) and (submodule.recurse = true). The goal is to do fetch &\nsubmodule update if-and-only-if the pointed-to commit changes.\n\nHowever, it seems with this config a git pull will act as if\nfetch.recurseSubmodules was set to true (instead of on-demand) and do\na fetch on all of the submodules. For me this does not correspond to\nthe documentation (which says the value of submodule.recurse will only\nbe used if fetch.recurseSubmodules is NOT set). I would have thought\nthat this recent commit:\nhttps://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221\ndescribes this scenario and fixed it, but I still have that behaviour\n(git 2.37.3). Maybe that patch only covered true/false settings for\nfetch.recurseSubmodules and doesn't properly handle the on-demand\nsetting?\n\nIs what I see the intended behaviour?\nIs it possible in any way to configure git to only fetch & update\nsubmodules if the pointed-to commit changes?\n\n\n\nKind regards,\n\nPieter-Jan\n"},{"id":"463152","messageId":"36621ba8-ad8e-8758-3e17-ea7a83432b4c@gmail.com","threadId":"58408","inReplyTo":"CABxXVRS-9unw-8_N8w2bdj4E5Ch780tMetJyD+++ixdWsBv90A@mail.gmail.com","subject":"Re: Question related to submodule and different recurse config options","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-09-18T14:36:43Z","receivedAt":"2022-09-18T14:36:58Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Pieter-Jan,\n\nLe 2022-09-08 à 11:01, Pieter-Jan Busschaert a écrit :\n> Hello,\n> \n> I'm trying to set my git config like this (fetch.recurseSubmodules =\n> on-demand) and (submodule.recurse = true). The goal is to do fetch &\n> submodule update if-and-only-if the pointed-to commit changes.\n> \n\nThat makes sense.\n\n> However, it seems with this config a git pull will act as if\n> fetch.recurseSubmodules was set to true (instead of on-demand) and do\n> a fetch on all of the submodules. For me this does not correspond to\n> the documentation (which says the value of submodule.recurse will only\n> be used if fetch.recurseSubmodules is NOT set). I would have thought\n> that this recent commit:\n> https://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221\n> describes this scenario and fixed it, but I still have that behaviour\n> (git 2.37.3). Maybe that patch only covered true/false settings for\n> fetch.recurseSubmodules and doesn't properly handle the on-demand\n> setting?\n> \n> Is what I see the intended behaviour?\n> Is it possible in any way to configure git to only fetch & update\n> submodules if the pointed-to commit changes?\n\nI'm not sure if that's the bug you are hitting, but I remember noticing that\nthe actual order of the two configs in the config file mattered (whereas it should\nnot). With this:\n\n    fetch.recurseSubmodules = on-demand\n    submodule.recurse = true\n\nthe \"submodule.recurse = true\" is read last and then the \"on-demand\" setting for fetch\nis effectively ignored.\n\nWith this order:\n\n    submodule.recurse = true\n    fetch.recurseSubmodules = on-demand\n\nI think it would work correctly. But I might be misremembering (I did not try it).\n\nCheers,\n\nPhilippe.\n"},{"id":"463220","messageId":"kl6l8rmfcihk.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58408","inReplyTo":"36621ba8-ad8e-8758-3e17-ea7a83432b4c@gmail.com","subject":"Re: Question related to submodule and different recurse config options","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-09-19T18:43:51Z","receivedAt":"2022-09-19T18:44:10Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Philippe Blain <levraiphilippeblain@gmail.com> writes:\n\n> Hi Pieter-Jan,\n>\n> Le 2022-09-08 à 11:01, Pieter-Jan Busschaert a écrit :\n>> However, it seems with this config a git pull will act as if\n>> fetch.recurseSubmodules was set to true (instead of on-demand) and do\n>> a fetch on all of the submodules. For me this does not correspond to\n>> the documentation (which says the value of submodule.recurse will only\n>> be used if fetch.recurseSubmodules is NOT set). I would have thought\n>> that this recent commit:\n>> https://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221\n>> describes this scenario and fixed it, but I still have that behaviour\n>> (git 2.37.3). Maybe that patch only covered true/false settings for\n>> fetch.recurseSubmodules and doesn't properly handle the on-demand\n>> setting?\n>> \n>> Is what I see the intended behaviour?\n>> Is it possible in any way to configure git to only fetch & update\n>> submodules if the pointed-to commit changes?\n>\n> I'm not sure if that's the bug you are hitting, but I remember noticing that\n> the actual order of the two configs in the config file mattered (whereas it should\n> not). With this:\n>\n>     fetch.recurseSubmodules = on-demand\n>     submodule.recurse = true\n>\n> the \"submodule.recurse = true\" is read last and then the \"on-demand\" setting for fetch\n> is effectively ignored.\n>\n> With this order:\n>\n>     submodule.recurse = true\n>     fetch.recurseSubmodules = on-demand\n>\n> I think it would work correctly. But I might be misremembering (I did not try it).\n\nThanks Philippe! Rereading that patch, I'm quite convinced that `git\npull` is doing the right thing, so the issue is probably in `git fetch`.\n\nConfig ordering _does_ matter, because both submodule.recurse and\nfetch.recurseSubmodules are read into \"recurse_submodules\" (see\nbuiltin/fetch.c:git_fetch_config).\nhttps://git-scm.com/docs/git-config#Documentation/git-config.txt-fetchrecurseSubmodules\nsays \"(fetch.recurseSubmodules) Defaults to on-demand, or to the value\nof submodule.recurse if set\", which sounds to me like\nfetch.recurseSubmodules should take precedence over submodule.recurse,\nand there is a real bug here. I haven't invested time into figuring out\nwhether this has always been broken, or whether this is a more recent\nregression.\n\nHere's one (really ugly) fix:\n\n  diff --git a/builtin/fetch.c b/builtin/fetch.c\n  index fc5cecb483..5fb81e0d7d 100644\n  --- a/builtin/fetch.c\n  +++ b/builtin/fetch.c\n  @@ -78,6 +78,8 @@ static const char *submodule_prefix = \"\";\n  static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n  static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;\n  static int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;\n  +static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n  +static int config_submodule_recurse = RECURSE_SUBMODULES_DEFAULT;\n  static int shown_url = 0;\n  static struct refspec refmap = REFSPEC_INIT_FETCH;\n  static struct list_objects_filter_options filter_options;\n  @@ -107,14 +109,14 @@ static int git_fetch_config(const char *k, const char *v, void *cb)\n    if (!strcmp(k, \"submodule.recurse\")) {\n      int r = git_config_bool(k, v) ?\n        RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n  -\t\trecurse_submodules = r;\n  +\t\t config_submodule_recurse = r;\n    }\n\n    if (!strcmp(k, \"submodule.fetchjobs\")) {\n      submodule_fetch_jobs_config = parse_submodule_fetchjobs(k, v);\n      return 0;\n    } else if (!strcmp(k, \"fetch.recursesubmodules\")) {\n  -\t\trecurse_submodules = parse_fetch_recurse_submodules_arg(k, v);\n  +\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);\n      return 0;\n    }\n\n  @@ -2112,6 +2114,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n\n    if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)\n      recurse_submodules = recurse_submodules_cli;\n  +\telse if (config_fetch_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n  +\t\trecurse_submodules = config_fetch_recurse_submodules;\n  +\telse if (config_submodule_recurse != RECURSE_SUBMODULES_DEFAULT)\n  +\t\trecurse_submodules = config_submodule_recurse;\n\n    if (negotiate_only) {\n      switch (recurse_submodules_cli) {\n\nThis _should_ work, but I wouldn't merge it as-is since\n\n- The proliferation of static variables is just awful. Maybe we can do\n  better by using repo_config_get_*() instead of the config iterator\n  function.\n- this doesn't have test coverage\n\nI don't think I have the time to turn this into a full fix (not for a\nfew weeks at least), but I'd be happy to review patches.\n\n>\n> Cheers,\n>\n> Philippe.\n"}]}