@matheusbsbs / matheus-bagliano / commits / 35f9e3e5e7

fetch: in partial clone, check presence of targets

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. Its documentation and name are also updated to better reflect what it does. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Sep 21, 2018 at 11:22 UTC 35f9e3e5e7a6436b50a8709e7e14a65a10cc1e7a
2 files changed +30 -2
builtin/fetch.c
+13 -2
@@ -931,10 +931,11 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
931 * everything we are going to fetch already exists and is connected
932 * locally.
933 */
934 -static int quickfetch(struct ref *ref_map)
934 +static int check_exist_and_connected(struct ref *ref_map)
935 {
936 struct ref *rm = ref_map;
937 struct check_connected_options opt = CHECK_CONNECTED_INIT;
938 + struct ref *r;
939
940 /*
941 * If we are deepening a shallow clone we already have these
@@ -945,13 +946,23 @@ static int quickfetch(struct ref *ref_map)
946 */
947 if (deepen)
948 return -1;
949 +
950 + /*
951 + * check_connected() allows objects to merely be promised, but
952 + * we need all direct targets to exist.
953 + */
954 + for (r = rm; r; r = r->next) {
955 + if (!has_object_file(&r->old_oid))
956 + return -1;
957 + }
958 +
959 opt.quiet = 1;
960 return check_connected(iterate_ref_map, &rm, &opt);
961 }
962
963 static int fetch_refs(struct transport *transport, struct ref *ref_map)
964 {
954 - int ret = quickfetch(ref_map);
965 + int ret = check_exist_and_connected(ref_map);
966 if (ret)
967 ret = transport_fetch_refs(transport, ref_map);
968 if (!ret)
t/t5616-partial-clone.sh
+17
@@ -170,6 +170,23 @@ test_expect_success 'partial clone fetches blobs pointed to by refs even if norm
170 git -C dst fsck
171 '
172
173 +test_expect_success 'fetch what is specified on CLI even if already promised' '
174 + rm -rf src dst.git &&
175 + git init src &&
176 + test_commit -C src foo &&
177 + test_config -C src uploadpack.allowfilter 1 &&
178 + test_config -C src uploadpack.allowanysha1inwant 1 &&
179 +
180 + git hash-object --stdin <src/foo.t >blob &&
181 +
182 + git clone --bare --filter=blob:none "file://$(pwd)/src" dst.git &&
183 + git -C dst.git rev-list --objects --quiet --missing=print HEAD >missing_before &&
184 + grep "?$(cat blob)" missing_before &&
185 + git -C dst.git fetch origin $(cat blob) &&
186 + git -C dst.git rev-list --objects --quiet --missing=print HEAD >missing_after &&
187 + ! grep "?$(cat blob)" missing_after
188 +'
189 +
190 . "$TEST_DIRECTORY"/lib-httpd.sh
191 start_httpd
192