[PATCH] fetch: in partial clone, check presence of targets
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Sep 20, 2018, 18:48 UTC
- Message-ID
- <20180920184843.20898-1-jonathantanmy@google.com>
When fetching an object that is known as a promisor object to the local repository, the connectivity check in quickfetch() in builtin/fetch.c succeeds, causing object transfer to be bypassed. However, this should not happen if that object is merely promised and not actually present.
Because this happens, when a user invokes "git fetch origin <sha-1>" on the command-line, the <sha-1> object may not actually be fetched even though the command returns an exit code of 0. This is a similar issue (but with a different cause) to the one fixed by a0c9016abd ("upload-pack: send refs' objects despite "filter"", 2018-07-09).
Therefore, update quickfetch() to also directly check for the presence of all objects to be fetched.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- In this patch, I have striven to avoid piping from Git commands that may fail, following the guidelines in [1].
[1] https://public-inbox.org/git/c625bfe2205d51b3158ef71e4bf472708642c146.1537223021.git.matvore@google.com/ --- builtin/fetch.c | 19 +++++++++++++++++++ t/t5616-partial-clone.sh | 17 +++++++++++++++++ 2 files changed, 36 insertions(+)
diff --git a/builtin/fetch.c b/builtin/fetch.c index 61bec5d21..e9640fe5a 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -938,6 +938,25 @@ static int quickfetch(struct ref *ref_map) */ if (deepen) return -1; + + if (repository_format_partial_clone) { + /* + * For the purposes of the connectivity check, + * check_connected() considers all objects promised by + * promisor objects as existing, which means that the + * connectivity check would pass even if a target object + * in rm is merely promised and not present. When + * fetching objects, we need all of them to be present + * (in addition to having correct connectivity), so + * check them directly. + */ + struct ref *r; + for (r = rm; r; r = r->next) { + if (!has_object_file(&r->old_oid)) + return -1; + } + } + opt.quiet = 1; return check_connected(iterate_ref_map, &rm, &opt); } diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index bbbe7537d..359d27d02 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -170,6 +170,23 @@ test_expect_success 'partial clone fetches blobs pointed to by refs even if norm git -C dst fsck ' +test_expect_success 'fetch what is specified on CLI even if already promised' ' + rm -rf src dst.git && + git init src && + test_commit -C src foo && + test_config -C src uploadpack.allowfilter 1 && + test_config -C src uploadpack.allowanysha1inwant 1 && + + git hash-object --stdin <src/foo.t >blob && + + git clone --bare --filter=blob:none "file://$(pwd)/src" dst.git && + git -C dst.git rev-list --objects --quiet --missing=print HEAD >missing_before && + grep "?$(cat blob)" missing_before && + git -C dst.git fetch origin $(cat blob) && + git -C dst.git rev-list --objects --quiet --missing=print HEAD >missing_after && + ! grep "?$(cat blob)" missing_after +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd
-- 2.19.0.444.g18242da7ef-goog