Do not stop at ancestors of our refs when deepening during fetching. This is because when performing such a fetch, shallow entries may have been updated; the client can reasonably assume that the existing refs are connected up to the old shallow points, but not the new.
This was noticed when a user tried to deepen an existing repository by fetching with --no-shallow from a server that did not send all necessary objects - the connectivity check as run by "git fetch" succeeded, but a subsequent "git fsck" failed.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- builtin/fetch.c | 5 ++++- connected.c | 6 ++++-- connected.h | 7 +++++++ t/t5537-fetch-shallow.sh | 23 +++++++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-)
Show changes to 4 files +38 −3
builtin/fetch.c, connected.c, connected.h, t/t5537-fetch-shallow.sh
diff --git a/builtin/fetch.c b/builtin/fetch.c index ea5b9669ad..2cfd13342f 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -780,6 +780,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, const char *filename = dry_run ? "/dev/null" : git_path_fetch_head(); int want_status; int summary_width = transport_summary_width(ref_map); + struct check_connected_options opt = CHECK_CONNECTED_INIT; fp = fopen(filename, "a"); if (!fp) @@ -791,7 +792,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, url = xstrdup("foreign"); rm = ref_map; - if (check_connected(iterate_ref_map, &rm, NULL)) { + if (deepen) + opt.is_deepening_fetch = 1; + if (check_connected(iterate_ref_map, &rm, &opt)) { rc = error(_("%s did not send all necessary objects\n"), url); goto abort; } diff --git a/connected.c b/connected.c index 91feb78815..1bba888eff 100644 --- a/connected.c +++ b/connected.c @@ -58,8 +58,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data, argv_array_push(&rev_list.args, "--stdin"); if (repository_format_partial_clone) argv_array_push(&rev_list.args, "--exclude-promisor-objects"); - argv_array_push(&rev_list.args, "--not"); - argv_array_push(&rev_list.args, "--all"); + if (!opt->is_deepening_fetch) { + argv_array_push(&rev_list.args, "--not"); + argv_array_push(&rev_list.args, "--all"); + } argv_array_push(&rev_list.args, "--quiet"); if (opt->progress) argv_array_pushf(&rev_list.args, "--progress=%s", diff --git a/connected.h b/connected.h index a53f03a61a..322dc76372 100644 --- a/connected.h +++ b/connected.h @@ -38,6 +38,13 @@ struct check_connected_options { * Insert these variables into the environment of the child process. */ const char **env; + + /* + * If non-zero, check the ancestry chain completely, not stopping at + * any existing ref. This is necessary when deepening existing refs + * during a fetch. + */ + unsigned is_deepening_fetch : 1; }; #define CHECK_CONNECTED_INIT { 0 } diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index df8d2f095a..ac4beac96b 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -186,4 +186,27 @@ EOF test_cmp expect actual ' +test_expect_success 'shallow fetches check connectivity without stopping at existing refs' ' + cp -R .git server.git && + + # Normally, the connectivity check stops at ancestors of existing refs. + git init client && + GIT_TRACE="$(pwd)/trace" git -C client fetch "$(pwd)/server.git" && + grep "run_command: git rev-list" trace >rev-list-command && + grep -e "--not --all" rev-list-command && + + # But it does not for a shallow fetch... + rm -rf client trace && + git init client && + GIT_TRACE="$(pwd)/trace" git -C client fetch --depth=1 "$(pwd)/server.git" && + grep "run_command: git rev-list" trace >rev-list-command && + ! grep -e "--not --all" rev-list-command && + + # ...and when deepening. + rm trace && + GIT_TRACE="$(pwd)/trace" git -C client fetch --unshallow "$(pwd)/server.git" && + grep "run_command: git rev-list" trace >rev-list-command && + ! grep -e "--not --all" rev-list-command +' + test_done
-- 2.18.0.rc2.346.g013aa6912e-goog
