Re: [PATCH v5 10/40] Add initial external odb support

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2017, 19:34 UTC
Message-ID: <xmqqfud8xx6m.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170803091926.1755-11-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 19 quoted lines

> +int external_odb_has_object(const unsigned char *sha1)
> +{
> +	struct odb_helper *o;
> +
> +	external_odb_init();
> +
> +	for (o = helpers; o; o = o->next)
> +		if (odb_helper_has_object(o, sha1))
> +			return 1;
> +	return 0;
> +}
> +
> +int external_odb_get_object(const unsigned char *sha1)
> +{
> +	struct odb_helper *o;
> +	const char *path;
> +
> +	if (!external_odb_has_object(sha1))
> +		return -1;

This probably would not matter, as I do not expect one repository to connect to and backed by very many external odb instances, but I would have expected that the interaction would go more like "ah, we need this object that is lacking locally. let's see if there is anybody with the object. now we found who claims to have the object, let's ask that guy (and nobody else) to give the object to us".

IOW, I would have expected two functions:

 - struct odb_helper *external_odb_with(struct object_id *oid);
 - int external_odb_get(struct object_id *oid, struct odb_helper *odb);

where the latter may start like

    if (!odb) {
	odb = external_odb_with(oid);
	if (!odb)
	    return -1;
    }
    ... go ask that odb for the object ...

Show 12 quoted lines

> diff --git a/external-odb.h b/external-odb.h
> new file mode 100644
> index 0000000000..9989490c9e
> --- /dev/null
> +++ b/external-odb.h
> @@ -0,0 +1,8 @@
> +#ifndef EXTERNAL_ODB_H
> +#define EXTERNAL_ODB_H
> +
> +const char *external_odb_root(void);
> +int external_odb_has_object(const unsigned char *sha1);
> +int external_odb_get_object(const unsigned char *sha1);

Even though ancient codebase of ours deliberately omitted them, I think our recent trend is to explicitly spell "extern " in headers.

Show 5 quoted lines

> diff --git a/odb-helper.h b/odb-helper.h
> new file mode 100644
> index 0000000000..5800661704
> --- /dev/null
> +++ b/odb-helper.h

Likewise.