Re: [PATCH v3 03/11] promisor-remote: implement promisor_remote_get_direct()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2019, 04:23 UTC
- Message-ID
- <xmqqpnqve71d.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190312132959.11764-4-chriscool@tuxfamily.org>
Christian Couder <christian.couder@gmail.com> writes:
Show 15 quoted lines
> @@ -98,3 +99,19 @@ int has_promisor_remote(void)
> {
> return !!promisor_remote_find(NULL);
> }
> +
> +int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
> +{
> + struct promisor_remote *o;
> +
> + promisor_remote_init();
> +
> + for (o = promisors; o; o = o->next) {
> + if (fetch_objects(o->remote_name, oids, oid_nr) < 0)
> + continue;
> + return 0;Suppose the caller asks to fetch 3 objects, A, B and C, from two promisors. The first promisor can give you A and B but cannot give you C. The second promisor only can give you C.
Does fetch_objects() return failure after attempting to fetch A, B and C from the first promisor? Then we go on to the second promisor but do we ask all three? That would mean the second promisor will also fail because it cannot give you A and B, and then the whole thing would fail. It would be nicer if the mechanism would allow us to fetch what is still missing from later promisor, perhaps.
As the original "fetch" protocol only allows you to fetch a pack with everything you asked for in it, instead of feeding you a pack with best effort, I think the answer to the above is "it is very hard to improve over what we have here", but people may have interesting ideas ;-)
Show 5 quoted lines
> + } > + > + return -1; > +} > +
Adding trailing blank line at the end?
Show 11 quoted lines
> diff --git a/promisor-remote.h b/promisor-remote.h
> index bfbf7c0f21..f9f5825417 100644
> --- a/promisor-remote.h
> +++ b/promisor-remote.h
> @@ -13,5 +13,6 @@ struct promisor_remote {
> extern struct promisor_remote *promisor_remote_new(const char *remote_name);
> extern struct promisor_remote *promisor_remote_find(const char *remote_name);
> extern int has_promisor_remote(void);
> +extern int promisor_remote_get_direct(const struct object_id *oids, int oid_nr);
>
> #endif /* PROMISOR_REMOTE_H */