Re: [PATCH v2 13/22] hash-ll.h: split out of hash.h to remove dependency on repository.h

[Elijah Newren <newren@gmail.com>]

On Mon, Apr 24, 2023 at 11:51 AM Glen Choo <chooglen@google.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >                                 Move the parts of hash.h that do not
> > depend upon repository.h into a new file hash-ll.h (the "low level"
> > parts of hash.h), and adjust other files to use this new header where
> > the convenience inline functions aren't needed.
>
> Suggestion: To maintain this property, it might be helpful to capture
> the rules of hash-ll.h vs hash.h in a top-level comment.
>
> > diff --git a/hash-ll.h b/hash-ll.h
> > new file mode 100644
> > index 00000000000..80509251370
> > --- /dev/null
> > +++ b/hash-ll.h
> > +const struct object_id *null_oid(void);
> > [...]
> > +const char *empty_tree_oid_hex(void);
> > +const char *empty_blob_oid_hex(void);
>
> hash-ll.h doesn't depend on repository.h, but these functions' bodies
> use the_hash_algo. Does it matter?

That's a good point.  I think eventually moving these functions from
hash-ll.h to hash.h on this basis makes sense.  But I kind of view
this series (and its predecessors) as focusing on cleaning up header
dependencies, with the idea that hopefully we'd go through and do
further cleanup of dependencies of C files that would likely result in
further changes to some of the headers.  I suspect this isn't the only
change we'd need to make for that kind of consistency, which might
lengthen this already sizable series, and I've probably missed a few
similar cases in my previous series as well.

> Moving the functions to hash.h requires changing 8 files to #include
> "hash.h", all of which seem to be because they were getting hash-ll.h
> indirectly via object-name.h.

Yeah, seems like a reasonable further change.  Are you okay with that
being in a future series, or do you think it should be included in
this one?