Re: [PATCH 14/23] hash.h, repository.h: reverse the order of these dependencies
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 18, 2023, 23:29 UTC
- Message-ID
- <xmqqedogbwh3.fsf@gitster.g>
- In-Reply-To
- <ad90e716-ba23-040f-66be-4c4faff02ea8@github.com>
Derrick Stolee <derrickstolee@github.com> writes:
Show 23 quoted lines
> 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.