Re: [PATCH v3 2/2] fetch: add fetch.submoduleErrors to make submodule fetch errors non-fatal
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 10, 2026, 22:21 UTC
- Message-ID
- <xmqqmrvybi5t.fsf@gitster.g>
- In-Reply-To
- <20260710122655.3066377-3-paulius.zaleckas@gmail.com>
Paulius Zaleckas <paulius.zaleckas@gmail.com> writes:
Show 105 quoted lines
> When fetching with --recurse-submodules, a submodule commit that is not
> yet reachable from any of the submodule's remote refs causes the entire
> fetch to fail. This is overly strict when the missing commit belongs to
> an upstream branch that is still being prepared (e.g. an in-progress
> merge topic): the local branch does not need that commit, so there is no
> reason to treat its absence as fatal.
>
> Add a new config key fetch.submoduleErrors (values: fail/warn) and a
> corresponding --submodule-errors=(fail|warn) command-line option that
> control this behaviour. The default remains fail (existing behaviour);
> setting the value to warn causes submodule fetch failures to be reported
> on stderr without affecting the overall exit status of git fetch / git
> pull.
>
> Forward the option to child fetches in add_options_to_argv() so that it
> also takes effect for `git fetch --all` / `--multiple` (where per-remote
> child processes handle the submodule recursion themselves) and for
> nested submodule recursion.
>
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
> ---
> Documentation/config/fetch.adoc | 14 ++++++
> Documentation/fetch-options.adoc | 8 ++++
> builtin/fetch.c | 41 ++++++++++++++++-
> submodule.c | 8 +++-
> submodule.h | 7 ++-
> t/t5526-fetch-submodules.sh | 76 ++++++++++++++++++++++++++++++++
> 6 files changed, 150 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
> index 04ac90912d..5c9c942a70 100644
> --- a/Documentation/config/fetch.adoc
> +++ b/Documentation/config/fetch.adoc
> @@ -10,6 +10,20 @@
> reference.
> Defaults to `on-demand`, or to the value of `submodule.recurse` if set.
>
> +`fetch.submoduleErrors`::
> + Controls how errors from submodule fetches are handled when
> + `--recurse-submodules` is in effect. When set to `fail` (the default),
> + any submodule fetch error causes the overall `git fetch` or `git pull`
> + to exit with a non-zero status. When set to `warn`, submodule fetch
> + errors are reported to standard error but do not affect the exit
> + status of the command. This is useful when working in repositories
> + where some branches reference submodule commits that are not yet
> + available on the submodule remote, but those commits are not needed
> + for the currently checked-out branch.
> ++
> +The value of this option can be overridden by the `--submodule-errors`
> +option of linkgit:git-fetch[1].
> +
> `fetch.fsckObjects`::
> If it is set to true, git-fetch-pack will check all fetched
> objects. See `transfer.fsckObjects` for what's
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index 035f780e58..78525f6848 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -294,6 +294,14 @@ ifndef::git-pull[]
> `--no-recurse-submodules`::
> Disable recursive fetching of submodules (this has the same effect as
> using the `--recurse-submodules=no` option).
> +
> +`--submodule-errors=(fail|warn)`::
> + Control how errors from submodule fetches are handled when
> + `--recurse-submodules` is in effect. When set to `fail` (the default),
> + any submodule fetch error causes the overall `git fetch` to exit with a
> + non-zero status. When set to `warn`, submodule fetch errors are reported
> + to standard error but do not affect the exit status of the command. Can
> + also be configured via `fetch.submoduleErrors`. See linkgit:git-config[1].
> endif::git-pull[]
>
> `--set-upstream`::
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index c1d7c672f4..40daaf5cc7 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -110,6 +110,7 @@ struct fetch_config {
> int recurse_submodules;
> int parallel;
> int submodule_fetch_jobs;
> + int submodule_errors;
> };
>
> static int git_fetch_config(const char *k, const char *v,
> @@ -152,6 +153,19 @@ static int git_fetch_config(const char *k, const char *v,
> return 0;
> }
>
> + if (!strcmp(k, "fetch.submoduleerrors")) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if (!strcasecmp(v, "fail"))
> + fetch_config->submodule_errors = SUBMODULE_ERRORS_FAIL;
> + else if (!strcasecmp(v, "warn"))
> + fetch_config->submodule_errors = SUBMODULE_ERRORS_WARN;
> + else
> + die(_("invalid value for '%s': '%s'"),
> + "fetch.submoduleErrors", v);
> + return 0;
> + }
> +
> if (!strcmp(k, "fetch.parallel")) {
> fetch_config->parallel = git_config_int(k, v, ctx->kvi);
> if (fetch_config->parallel < 0)Show 9 quoted lines
> @@ -2205,6 +2219,8 @@ static void add_options_to_argv(struct strvec *argv, > strvec_push(argv, "--no-recurse-submodules"); > else if (config->recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) > strvec_push(argv, "--recurse-submodules=on-demand"); > + if (config->submodule_errors == SUBMODULE_ERRORS_WARN) > + strvec_push(argv, "--submodule-errors=warn"); > if (tags == TAGS_SET) > strvec_push(argv, "--tags"); > else if (tags == TAGS_UNSET)
If (config->submodule_errors != SUBMODULE_ERRORS_WARN), then the argv[] would not see any --submodule-errors=<anything> to propagate down. Specifically, this function is called when recurse-submodules is not disabled, and prepares argv[] used to call fetch_submodules().
Show 9 quoted lines
> int cmd_fetch(int argc, > const char **argv, > const char *prefix, > @@ -2477,6 +2506,7 @@ int cmd_fetch(int argc, > .recurse_submodules = RECURSE_SUBMODULES_DEFAULT, > .parallel = 1, > .submodule_fetch_jobs = -1, > + .submodule_errors = SUBMODULE_ERRORS_FAIL, > };
Here, .submodule_errors member is initialized to SUBMODULE_ERRORS_FAIL (i.e. 0).
Show 16 quoted lines
> @@ -2491,6 +2521,7 @@ int cmd_fetch(int argc,
> int max_jobs = -1;
> int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
> int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
> + int submodule_errors_cli = -1; /* -1: not set on command line */
> int fetch_write_commit_graph = -1;
> int stdin_refspecs = 0;
> int negotiate_only = 0;
> @@ -2527,6 +2558,10 @@ int cmd_fetch(int argc,
> OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules_cli, N_("on-demand"),
> N_("control recursive fetching of submodules"),
> PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
> + OPT_CALLBACK_F(0, "submodule-errors", &submodule_errors_cli,
> + N_("(fail|warn)"),
> + N_("control how submodule fetch errors are handled"),
> + 0, option_parse_submodule_errors),And command line option "--submodule-errors={warn,fail}" may update the local variable submodule_errors_cli (initialied to -1) to one of SUBMODULE_ERRORS_{WARN,FAIL}. These are different from -1, so we can reliably tell if we saw a command line override, which is good.
Show 9 quoted lines
> OPT_BOOL(0, "dry-run", &dry_run,
> N_("dry run")),
> OPT_BOOL(0, "porcelain", &porcelain, N_("machine-readable output")),
> @@ -2616,6 +2651,9 @@ int cmd_fetch(int argc,
> if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
> config.recurse_submodules = recurse_submodules_cli;
>
> + if (submodule_errors_cli != -1)
> + config.submodule_errors = submodule_errors_cli;And we override what we read from the configuration if we got a command line override.
And the value in config.submodule_errors is used much later, in a call to add_options_to_argv() we saw earlier, but this patch does not touch the caller so we do not see the calling site.
I do not do submodules, so my expectation here may be a bit skewed, but what happens when we configure fetch.submoduleErrors to warn, but override it from the command line to fail? .submodule_errors is set to SUBMODULE_ERRORS_FAIL here? As we saw, add_options_to_argv() stuff --submodule-error=<setting> only when config.submodule_errors is set to SUBMODULE_ERRORS_WARN, so we do not pass command line override. Is this desirable? Don't we want to pass down not just --submodule-error=warn but --submodule-error=fail if that is what was given from the command line? Or does it not matter because fail is the default?
Thanks.