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 <jonathanta...@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.matv...@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

Reply via email to