Re: [PATCH] fetch: in partial clone, check presence of targets
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 20, 2018, 20:50 UTC
- Message-ID
- <xmqqh8ijzyep.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180920184843.20898-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 25 quoted lines
> 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;
> + }Because check_connected() lies in the presense of "promisor", we can defeat it this way, which makes sense.
I wonder if it makes sense to do this check unconditionally, even when we are in a fully functioning clone. The check is quite cheap and can reject a ref_map that has an object we do not know about, without check_connected() having to ask the rev-list.
Show 32 quoted lines
> + } > + > 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