On Tue, Apr 18, 2023 at 4:29 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Derrick Stolee <derrickstolee@xxxxxxxxxx> 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. What would you think of either of the two following alternative solutions: * Move these into a new header, say "hash-repository.h" that folks could include. People wanting to use these macros would have to include an alternate header, but one that is perhaps more logically named, and would allow other headers to get the basics of hash.h without repository.h * Move everything else in hash.h to a new header, say "hash-basics.h", which hash.h will #include. That retains the ability for people to depend on hash.h and use these inline functions, while also giving other headers the option of including the basics of today's hash.h without repository.h.
