git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Question related to submodule and different recurse config options
@ 2022-09-08 15:01 Pieter-Jan Busschaert
  2022-09-18 14:36 ` Philippe Blain
  0 siblings, 1 reply; 3+ messages in thread
From: Pieter-Jan Busschaert @ 2022-09-08 15:01 UTC (permalink / raw)
  To: git

Hello,

I'm trying to set my git config like this (fetch.recurseSubmodules =
on-demand) and (submodule.recurse = true). The goal is to do fetch &
submodule update if-and-only-if the pointed-to commit changes.

However, it seems with this config a git pull will act as if
fetch.recurseSubmodules was set to true (instead of on-demand) and do
a fetch on all of the submodules. For me this does not correspond to
the documentation (which says the value of submodule.recurse will only
be used if fetch.recurseSubmodules is NOT set). I would have thought
that this recent commit:
https://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221
describes this scenario and fixed it, but I still have that behaviour
(git 2.37.3). Maybe that patch only covered true/false settings for
fetch.recurseSubmodules and doesn't properly handle the on-demand
setting?

Is what I see the intended behaviour?
Is it possible in any way to configure git to only fetch & update
submodules if the pointed-to commit changes?



Kind regards,

Pieter-Jan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question related to submodule and different recurse config options
  2022-09-08 15:01 Question related to submodule and different recurse config options Pieter-Jan Busschaert
@ 2022-09-18 14:36 ` Philippe Blain
  2022-09-19 18:43   ` Glen Choo
  0 siblings, 1 reply; 3+ messages in thread
From: Philippe Blain @ 2022-09-18 14:36 UTC (permalink / raw)
  To: Pieter-Jan Busschaert, git; +Cc: Glen Choo

Hi Pieter-Jan,

Le 2022-09-08 à 11:01, Pieter-Jan Busschaert a écrit :
> Hello,
> 
> I'm trying to set my git config like this (fetch.recurseSubmodules =
> on-demand) and (submodule.recurse = true). The goal is to do fetch &
> submodule update if-and-only-if the pointed-to commit changes.
> 

That makes sense.

> However, it seems with this config a git pull will act as if
> fetch.recurseSubmodules was set to true (instead of on-demand) and do
> a fetch on all of the submodules. For me this does not correspond to
> the documentation (which says the value of submodule.recurse will only
> be used if fetch.recurseSubmodules is NOT set). I would have thought
> that this recent commit:
> https://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221
> describes this scenario and fixed it, but I still have that behaviour
> (git 2.37.3). Maybe that patch only covered true/false settings for
> fetch.recurseSubmodules and doesn't properly handle the on-demand
> setting?
> 
> Is what I see the intended behaviour?
> Is it possible in any way to configure git to only fetch & update
> submodules if the pointed-to commit changes?

I'm not sure if that's the bug you are hitting, but I remember noticing that
the actual order of the two configs in the config file mattered (whereas it should
not). With this:

    fetch.recurseSubmodules = on-demand
    submodule.recurse = true

the "submodule.recurse = true" is read last and then the "on-demand" setting for fetch
is effectively ignored.

With this order:

    submodule.recurse = true
    fetch.recurseSubmodules = on-demand

I think it would work correctly. But I might be misremembering (I did not try it).

Cheers,

Philippe.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question related to submodule and different recurse config options
  2022-09-18 14:36 ` Philippe Blain
@ 2022-09-19 18:43   ` Glen Choo
  0 siblings, 0 replies; 3+ messages in thread
From: Glen Choo @ 2022-09-19 18:43 UTC (permalink / raw)
  To: Philippe Blain, Pieter-Jan Busschaert, git

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> Hi Pieter-Jan,
>
> Le 2022-09-08 à 11:01, Pieter-Jan Busschaert a écrit :
>> However, it seems with this config a git pull will act as if
>> fetch.recurseSubmodules was set to true (instead of on-demand) and do
>> a fetch on all of the submodules. For me this does not correspond to
>> the documentation (which says the value of submodule.recurse will only
>> be used if fetch.recurseSubmodules is NOT set). I would have thought
>> that this recent commit:
>> https://github.com/git/git/commit/ed54e1b31ad1a9a35ef6c23024d325a2c4d85221
>> describes this scenario and fixed it, but I still have that behaviour
>> (git 2.37.3). Maybe that patch only covered true/false settings for
>> fetch.recurseSubmodules and doesn't properly handle the on-demand
>> setting?
>> 
>> Is what I see the intended behaviour?
>> Is it possible in any way to configure git to only fetch & update
>> submodules if the pointed-to commit changes?
>
> I'm not sure if that's the bug you are hitting, but I remember noticing that
> the actual order of the two configs in the config file mattered (whereas it should
> not). With this:
>
>     fetch.recurseSubmodules = on-demand
>     submodule.recurse = true
>
> the "submodule.recurse = true" is read last and then the "on-demand" setting for fetch
> is effectively ignored.
>
> With this order:
>
>     submodule.recurse = true
>     fetch.recurseSubmodules = on-demand
>
> I think it would work correctly. But I might be misremembering (I did not try it).

Thanks Philippe! Rereading that patch, I'm quite convinced that `git
pull` is doing the right thing, so the issue is probably in `git fetch`.

Config ordering _does_ matter, because both submodule.recurse and
fetch.recurseSubmodules are read into "recurse_submodules" (see
builtin/fetch.c:git_fetch_config).
https://git-scm.com/docs/git-config#Documentation/git-config.txt-fetchrecurseSubmodules
says "(fetch.recurseSubmodules) Defaults to on-demand, or to the value
of submodule.recurse if set", which sounds to me like
fetch.recurseSubmodules should take precedence over submodule.recurse,
and there is a real bug here. I haven't invested time into figuring out
whether this has always been broken, or whether this is a more recent
regression.

Here's one (really ugly) fix:

  diff --git a/builtin/fetch.c b/builtin/fetch.c
  index fc5cecb483..5fb81e0d7d 100644
  --- a/builtin/fetch.c
  +++ b/builtin/fetch.c
  @@ -78,6 +78,8 @@ static const char *submodule_prefix = "";
  static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
  static int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
  +static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  +static int config_submodule_recurse = RECURSE_SUBMODULES_DEFAULT;
  static int shown_url = 0;
  static struct refspec refmap = REFSPEC_INIT_FETCH;
  static struct list_objects_filter_options filter_options;
  @@ -107,14 +109,14 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
    if (!strcmp(k, "submodule.recurse")) {
      int r = git_config_bool(k, v) ?
        RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
  -		recurse_submodules = r;
  +		 config_submodule_recurse = r;
    }

    if (!strcmp(k, "submodule.fetchjobs")) {
      submodule_fetch_jobs_config = parse_submodule_fetchjobs(k, v);
      return 0;
    } else if (!strcmp(k, "fetch.recursesubmodules")) {
  -		recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);
  +		config_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(k, v);
      return 0;
    }

  @@ -2112,6 +2114,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)

    if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
      recurse_submodules = recurse_submodules_cli;
  +	else if (config_fetch_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
  +		recurse_submodules = config_fetch_recurse_submodules;
  +	else if (config_submodule_recurse != RECURSE_SUBMODULES_DEFAULT)
  +		recurse_submodules = config_submodule_recurse;

    if (negotiate_only) {
      switch (recurse_submodules_cli) {

This _should_ work, but I wouldn't merge it as-is since

- The proliferation of static variables is just awful. Maybe we can do
  better by using repo_config_get_*() instead of the config iterator
  function.
- this doesn't have test coverage

I don't think I have the time to turn this into a full fix (not for a
few weeks at least), but I'd be happy to review patches.

>
> Cheers,
>
> Philippe.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-09-19 18:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08 15:01 Question related to submodule and different recurse config options Pieter-Jan Busschaert
2022-09-18 14:36 ` Philippe Blain
2022-09-19 18:43   ` Glen Choo

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).