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>

Jonathan Tan committed Aug 20, 2020 at 10:51 UTC 1b03df5f1e98b64b3bdc8a59c56697eaf675daca

2 files changed +26 -7

fetch-pack.c

+10 -7

@@ -866,13 +866,16 @@ static int get_pack(struct fetch_pack_args *args,

866 * have this responsibility.

867 */

868 args->check_self_contained_and_connected = 0;

869 - /*

870 - * If we're obtaining the filename of a lockfile, we'll use

871 - * that filename to write a .promisor file with more

872 - * information below. If not, we need index-pack to do it for

873 - * us.

874 - */

875 - if (!(do_keep && pack_lockfiles) && args->from_promisor)

869 +

870 + if (args->from_promisor)

871 + /*

872 + * write_promisor_file() may be called afterwards but

873 + * we still need index-pack to know that this is a

874 + * promisor pack. For example, if transfer.fsckobjects

875 + * is true, index-pack needs to know that .gitmodules

876 + * is a promisor object (so that it won't complain if

877 + * it is missing).

878 + */

879 strvec_push(&cmd.args, "--promisor");

880 }

881 else {

t/t5616-partial-clone.sh

+16

@@ -163,6 +163,22 @@ test_expect_success 'manual prefetch of missing objects' '

163 test_line_count = 0 observed.oids

164 '

165

166 +test_expect_success 'partial clone with transfer.fsckobjects=1 works with submodules' '

167 + test_create_repo submodule &&

168 + test_commit -C submodule mycommit &&

169 +

170 + test_create_repo src_with_sub &&

171 + test_config -C src_with_sub uploadpack.allowfilter 1 &&

172 + test_config -C src_with_sub uploadpack.allowanysha1inwant 1 &&

173 +

174 + git -C src_with_sub submodule add "file://$(pwd)/submodule" mysub &&

175 + git -C src_with_sub commit -m "commit with submodule" &&

176 +

177 + git -c transfer.fsckobjects=1 \

178 + clone --filter="blob:none" "file://$(pwd)/src_with_sub" dst &&

179 + test_when_finished rm -rf dst

180 +'

181 +

182 test_expect_success 'partial clone with transfer.fsckobjects=1 uses index-pack --fsck-objects' '

183 git init src &&

184 test_commit -C src x &&