Skip to content

Commit

Permalink
fetch --negotiate-only: do not update submodules
Browse files Browse the repository at this point in the history
`git fetch --negotiate-only` is an implementation detail of push
negotiation and, unlike most `git fetch` invocations, does not actually
update the main repository. Thus it should not update submodules even
if submodule recursion is enabled.

This is not just slow, it is wrong e.g. push negotiation with
"submodule.recurse=true" will cause submodules to be updated because it
invokes `git fetch --negotiate-only`.

Fix this by disabling submodule recursion if --negotiate-only was given.
Since this makes --negotiate-only and --recurse-submodules incompatible,
check for this invalid combination and die.

This does not use the "goto cleanup" introduced in the previous commit
because we want to recurse through submodules whenever a ref is fetched,
and this can happen without introducing new objects.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
chooglen authored and gitster committed Jan 19, 2022
1 parent 135a12b commit 386c076
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
1 change: 1 addition & 0 deletions Documentation/fetch-options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ configuration variables documented in linkgit:git-config[1], and the
ancestors of the provided `--negotiation-tip=*` arguments,
which we have in common with the server.
+
This is incompatible with `--recurse-submodules=[yes|on-demand]`.
Internally this is used to implement the `push.negotiate` option, see
linkgit:git-config[1].

Expand Down
24 changes: 23 additions & 1 deletion builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
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 shown_url = 0;
static struct refspec refmap = REFSPEC_INIT_FETCH;
Expand Down Expand Up @@ -166,7 +167,7 @@ static struct option builtin_fetch_options[] = {
N_("prune remote-tracking branches no longer on remote")),
OPT_BOOL('P', "prune-tags", &prune_tags,
N_("prune local tags no longer on remote and clobber changed tags")),
OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules, N_("on-demand"),
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_BOOL(0, "dry-run", &dry_run,
Expand Down Expand Up @@ -1996,6 +1997,27 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)

argc = parse_options(argc, argv, prefix,
builtin_fetch_options, builtin_fetch_usage, 0);

if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
recurse_submodules = recurse_submodules_cli;

if (negotiate_only) {
switch (recurse_submodules_cli) {
case RECURSE_SUBMODULES_OFF:
case RECURSE_SUBMODULES_DEFAULT:
/*
* --negotiate-only should never recurse into
* submodules. Skip it by setting recurse_submodules to
* RECURSE_SUBMODULES_OFF.
*/
recurse_submodules = RECURSE_SUBMODULES_OFF;
break;

default:
die(_("--negotiate-only and --recurse-submodules cannot be used together"));
}
}

if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
int *sfjc = submodule_fetch_jobs_config == -1
? &submodule_fetch_jobs_config : NULL;
Expand Down
12 changes: 12 additions & 0 deletions t/t5516-fetch-push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,18 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f
test_i18ngrep "push negotiation failed" err
'

test_expect_success 'push with negotiation does not attempt to fetch submodules' '
mk_empty submodule_upstream &&
test_commit -C submodule_upstream submodule_commit &&
git submodule add ./submodule_upstream submodule &&
mk_empty testrepo &&
git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
test_commit -C testrepo unrelated_commit &&
git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
git -c submodule.recurse=true -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
! grep "Fetching submodule" err
'

test_expect_success 'push without wildcard' '
mk_empty testrepo &&
Expand Down
12 changes: 12 additions & 0 deletions t/t5702-protocol-v2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,18 @@ test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
test_cmp err.expect err.actual
'

test_expect_success 'usage: --negotiate-only with --recurse-submodules' '
cat >err.expect <<-\EOF &&
fatal: --negotiate-only and --recurse-submodules cannot be used together
EOF
test_must_fail git -c protocol.version=2 -C client fetch \
--negotiate-only \
--recurse-submodules \
origin 2>err.actual &&
test_cmp err.expect err.actual
'

test_expect_success 'file:// --negotiate-only' '
SERVER="server" &&
URI="file://$(pwd)/server" &&
Expand Down

0 comments on commit 386c076

Please sign in to comment.