Re: [PATCH 4/9] odb: lift object existence check out of the "loose" backend

[Toon Claes <toon@iotcl.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Before writing a new loose object we first check whether the object
> already exists in any of the sources attached to the object database.
> This results in a couple of issues:
>
>   - We have a layering violation, where the source needs to be aware of
>     objects stored in any of the other sources.
>
>   - Every backend would have to reimplement this check, which feels
>     somewhat pointless.
>
>   - It is not possible to easily write an object into a source in case
>     the same object already exists in another source.
>
> Refactor the code and lift up the object existence check from the
> "loose" backend into the generic ODB layer. No callers need adjustment
> as none of them write via a specific source, but via the ODB layer.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c              | 7 +++++++
>  odb/source-loose.c | 8 ++------
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 4adbdf8a64..bfeca76f4e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -997,6 +997,13 @@ int odb_write_object_ext(struct object_database *odb,
>  
>  	hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
>  
> +	/*
> +	 * We can skip the write in case we already have the object available.
> +	 * In that case, we only freshen its mtime.
> +	 */
> +	if (odb_freshen_object(odb, oid))
> +		return 0;
> +

For the loose backend, this calls check_and_freshen_file(), which code
comment says:

    /*
     * All of the check_and_freshen functions return 1 if the file exists and was
     * freshened (if freshening was requested), 0 otherwise. If they return
     * 0, you should not assume that it is safe to skip a write of the object (it
     * either does not exist on disk, or has a stale mtime and may be subject to
     * pruning).
     */

And I see similar logic in the packed backend. So a non-zero value skips
the write and the file was freshened. Makes sense.

-- 
Cheers,
Toon