Subject: Re: [PATCH 14/23] hash.h, repository.h: reverse the order of these dependencies

From: Junio C Hamano <[email-masked]>
Date: Tue, 18 Apr 2023 16:29:28 -0700

Derrick Stolee <[email-masked]> writes:

> This is the first patch in the series where I don't immediately agree
> with the patch. This is a big list of methods that don't seem like
> they fit in repository.h:
>
>> diff --git a/repository.h b/repository.h
>> +static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
>> +static inline int oidcmp(const struct object_id *oid1, const struct object_id *oid2)
>> +static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2)
>> +static inline int oideq(const struct object_id *oid1, const struct object_id *oid2)
>> +static inline int is_null_oid(const struct object_id *oid)
>> +static inline void hashcpy(unsigned char *sha_dst, const unsigned char *sha_src)
>> +static inline void oidcpy_with_padding(struct object_id *dst,
>> +				       const struct object_id *src)
>> +static inline void hashclr(unsigned char *hash)
>> +static inline void oidclr(struct object_id *oid)
>> +static inline void oidread(struct object_id *oid, const unsigned char *hash)
>> +static inline int is_empty_blob_sha1(const unsigned char *sha1)
>> +static inline int is_empty_blob_oid(const struct object_id *oid)
>> +static inline int is_empty_tree_sha1(const unsigned char *sha1)
>> +static inline int is_empty_tree_oid(const struct object_id *oid)
>
> The goal to remove repository.h from hash.h and object.h makes sense
> as a goal, but is there another way to do it?

Indeed.

All of the above sit very well in hash simply because they are all
about hashes.  It does not have much to do with "repository", not
more than "well, hashes we use to identify objects, and objects are
stored in repositories".

>From the point of view of somebody who needs to use these macros, it
is utterly unnatural that they have to include "repository.h" (as
opposed to, say, "hash.h") just to be able to compare two hash
values.  Most of our programs interact with only one repository, and
it is understandable to include a header "repository.h" if your
program needs to interact with an extra repository other than the
"current" one.  But this feels backwards and not quite satisfactory,
even though inlines are special and I can fully sympathize with the
author who felt that this patch was necessary.