Skip to content

Commit 472a219

Browse files
gitstergitster
committed
Merge branch 'gc/fetch-negotiate-only-early-return'
"git fetch --negotiate-only" is an internal command used by "git push" to figure out which part of our history is missing from the other side. It should never recurse into submodules even when fetch.recursesubmodules configuration variable is set, nor it should trigger "gc". The code has been tightened up to ensure it only does common ancestry discovery and nothing else. * gc/fetch-negotiate-only-early-return: fetch: help translators by reusing the same message template fetch --negotiate-only: do not update submodules fetch: skip tasks related to fetching objects fetch: use goto cleanup in cmd_fetch()
2 parents ec4f70e + de4eaae commit 472a219

File tree

4 files changed

+63
-3
lines changed

4 files changed

+63
-3
lines changed

‎Documentation/fetch-options.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ configuration variables documented in linkgit:git-config[1], and the
7171
ancestors of the provided `--negotiation-tip=*` arguments,
7272
which we have in common with the server.
7373
+
74+
This is incompatible with `--recurse-submodules=[yes|on-demand]`.
7475
Internally this is used to implement the `push.negotiate` option, see
7576
linkgit:git-config[1].
7677

‎builtin/fetch.c‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ static struct transport *gtransport;
7676
static struct transport *gsecondary;
7777
static const char *submodule_prefix = "";
7878
static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
79+
static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
7980
static int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
8081
static int shown_url = 0;
8182
static struct refspec refmap = REFSPEC_INIT_FETCH;
@@ -167,7 +168,7 @@ static struct option builtin_fetch_options[] = {
167168
N_("prune remote-tracking branches no longer on remote")),
168169
OPT_BOOL('P', "prune-tags", &prune_tags,
169170
N_("prune local tags no longer on remote and clobber changed tags")),
170-
OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules, N_("on-demand"),
171+
OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules_cli, N_("on-demand"),
171172
N_("control recursive fetching of submodules"),
172173
PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
173174
OPT_BOOL(0, "dry-run", &dry_run,
@@ -2019,6 +2020,28 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
20192020

20202021
argc = parse_options(argc, argv, prefix,
20212022
builtin_fetch_options, builtin_fetch_usage, 0);
2023+
2024+
if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
2025+
recurse_submodules = recurse_submodules_cli;
2026+
2027+
if (negotiate_only) {
2028+
switch (recurse_submodules_cli) {
2029+
case RECURSE_SUBMODULES_OFF:
2030+
case RECURSE_SUBMODULES_DEFAULT:
2031+
/*
2032+
* --negotiate-only should never recurse into
2033+
* submodules. Skip it by setting recurse_submodules to
2034+
* RECURSE_SUBMODULES_OFF.
2035+
*/
2036+
recurse_submodules = RECURSE_SUBMODULES_OFF;
2037+
break;
2038+
2039+
default:
2040+
die(_("options '%s' and '%s' cannot be used together"),
2041+
"--negotiate-only", "--recurse-submodules");
2042+
}
2043+
}
2044+
20222045
if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
20232046
int *sfjc = submodule_fetch_jobs_config == -1
20242047
? &submodule_fetch_jobs_config : NULL;
@@ -2100,7 +2123,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
21002123
gtransport->smart_options->acked_commits = &acked_commits;
21012124
} else {
21022125
warning(_("protocol does not support --negotiate-only, exiting"));
2103-
return 1;
2126+
result = 1;
2127+
goto cleanup;
21042128
}
21052129
if (server_options.nr)
21062130
gtransport->server_options = &server_options;
@@ -2156,7 +2180,16 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
21562180
strvec_clear(&options);
21572181
}
21582182

2159-
string_list_clear(&list, 0);
2183+
/*
2184+
* Skip irrelevant tasks because we know objects were not
2185+
* fetched.
2186+
*
2187+
* NEEDSWORK: as a future optimization, we can return early
2188+
* whenever objects were not fetched e.g. if we already have all
2189+
* of them.
2190+
*/
2191+
if (negotiate_only)
2192+
goto cleanup;
21602193

21612194
prepare_repo_settings(the_repository);
21622195
if (fetch_write_commit_graph > 0 ||
@@ -2175,5 +2208,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
21752208
if (enable_auto_gc)
21762209
run_auto_maintenance(verbosity < 0);
21772210

2211+
cleanup:
2212+
string_list_clear(&list, 0);
21782213
return result;
21792214
}

‎t/t5516-fetch-push.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,18 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f
229229
test_i18ngrep "push negotiation failed" err
230230
'
231231

232+
test_expect_success 'push with negotiation does not attempt to fetch submodules' '
233+
mk_empty submodule_upstream &&
234+
test_commit -C submodule_upstream submodule_commit &&
235+
git submodule add ./submodule_upstream submodule &&
236+
mk_empty testrepo &&
237+
git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
238+
test_commit -C testrepo unrelated_commit &&
239+
git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
240+
git -c submodule.recurse=true -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
241+
! grep "Fetching submodule" err
242+
'
243+
232244
test_expect_success 'push without wildcard' '
233245
mk_empty testrepo &&
234246

‎t/t5702-protocol-v2.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,18 @@ test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
628628
test_cmp err.expect err.actual
629629
'
630630

631+
test_expect_success 'usage: --negotiate-only with --recurse-submodules' '
632+
cat >err.expect <<-\EOF &&
633+
fatal: options '\''--negotiate-only'\'' and '\''--recurse-submodules'\'' cannot be used together
634+
EOF
635+
636+
test_must_fail git -c protocol.version=2 -C client fetch \
637+
--negotiate-only \
638+
--recurse-submodules \
639+
origin 2>err.actual &&
640+
test_cmp err.expect err.actual
641+
'
642+
631643
test_expect_success 'file:// --negotiate-only' '
632644
SERVER="server" &&
633645
URI="file://$(pwd)/server" &&

0 commit comments

Comments
 (0)