fetch-pack: in partial clone, pass --promisor

When fetching a pack from a promisor remote, the corresponding .promisor
file needs to be created. "fetch-pack" originally did this by passing
"--promisor" to "index-pack", but in 5374a290aa ("fetch-pack: write
fetched refs to .promisor", 2019-10-16), "fetch-pack" was taught to do
this itself instead, because it needed to store ref information in the
.promisor file.

This causes a problem with superprojects when transfer.fsckobjects is
set, because in the current implementation, it is "index-pack" that
calls fsck_finish() to check the objects; before 5374a290aa,
fsck_finish() would see that .gitmodules is a promisor object and
tolerate it being missing, but after, there is no .promisor file (at the
time of the invocation of fsck_finish() by "index-pack") to tell it that
.gitmodules is a promisor object, so it returns an error.

Therefore, teach "fetch-pack" to pass "--promisor" to index pack once
again. "fetch-pack" will subsequently overwrite this file with the ref
information.

An alternative is to instead move object checking to "fetch-pack", and
let "index-pack" only index the files. However, since "index-pack" has
to inflate objects in order to index them, it seems reasonable to also
let it check the objects (which also require inflated files).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fetch-pack.c b/fetch-pack.c
index 7f20eca4f81ce401f951361ce59e1be6aa31cd67..d467edc24edeabd9f819003a8ddd3d1a6d497c47 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -866,13 +866,16 @@ static int get_pack(struct fetch_pack_args *args,
                         * have this responsibility.
                         */
                        args->check_self_contained_and_connected = 0;
-               /*
-                * If we're obtaining the filename of a lockfile, we'll use
-                * that filename to write a .promisor file with more
-                * information below. If not, we need index-pack to do it for
-                * us.
-                */
-               if (!(do_keep && pack_lockfiles) && args->from_promisor)
+
+               if (args->from_promisor)
+                       /*
+                        * write_promisor_file() may be called afterwards but
+                        * we still need index-pack to know that this is a
+                        * promisor pack. For example, if transfer.fsckobjects
+                        * is true, index-pack needs to know that .gitmodules
+                        * is a promisor object (so that it won't complain if
+                        * it is missing).
+                        */
                        strvec_push(&cmd.args, "--promisor");
        }
        else {

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 8827c2ed18394f3f7f5ae4bd35c3514a36eacc9d..5a01466db4837e1f80aac7236cd3a71e5aaee03d 100755 (executable)
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -163,6 +163,22 @@ test_expect_success 'manual prefetch of missing objects' '
        test_line_count = 0 observed.oids
 '
 
+test_expect_success 'partial clone with transfer.fsckobjects=1 works with submodules' '
+       test_create_repo submodule &&
+       test_commit -C submodule mycommit &&
+
+       test_create_repo src_with_sub &&
+       test_config -C src_with_sub uploadpack.allowfilter 1 &&
+       test_config -C src_with_sub uploadpack.allowanysha1inwant 1 &&
+
+       git -C src_with_sub submodule add "file://$(pwd)/submodule" mysub &&
+       git -C src_with_sub commit -m "commit with submodule" &&
+
+       git -c transfer.fsckobjects=1 \
+               clone --filter="blob:none" "file://$(pwd)/src_with_sub" dst &&
+       test_when_finished rm -rf dst
+'
+
 test_expect_success 'partial clone with transfer.fsckobjects=1 uses index-pack --fsck-objects' '
        git init src &&
        test_commit -C src x &&