Subject: Re: [GSoC PATCH v4 0/5] preserve promisor files content after repack

From: Lorenzo Pegorari <[email-masked]>
Date: Fri, 10 Apr 2026 18:44:02 +0200

On Fri, Apr 10, 2026 at 08:47:43AM -0700, Junio C Hamano wrote:
> LorenzoPegorari <[email-masked]> writes:
> 
> > QUESTION:
> > The "CodingGuidelines" explicitly state that:
> >  "A C file must directly include the header files that declare the
> >   functions and the types it uses, except for the functions and types
> >   that are made available to it by including one of the header files
> >   it must include by the previous rule"
> > where "the previous rule" is (if I understand correctly), the one related
> > to "<git-compat-util.h>". From what I understand then, I should have
> > added an include for "strmap.h" (which is needed for `strset`), correct?
> > And if I am correct, shouldn't "strbuf.h", "hash.h", "odb.h",
> > "string-list.h" and "strvec.h" also be included?
> 
> If you are using any of the facilities declared in these header
> files in your program, yes.

Got it.

> In practice many header files pull in other header files for
> definitions they themselves use.  For example, <X.h> that defines
> "struct X" may include <Y.h> for the definition of "struct Y" because
> the former embeds an instance of the latter, instead of having a pointer
> to an on-heap instance of the latter.
> 
> If you use both "struct X" and "struct Y", your program may compile
> with only <X.h> included without <Y.h> included in such a case, but
> the guideline suggests against doing so, because it should not be
> relied on.  The implementation of "struct X" may change in the
> future and stop depending on "struct Y", at which time <X.h> stops
> including <Y.h> itself, and your program would start failing to
> build, because you use "struct Y" but without including <Y.h>.
> 
> But in practice, use of strbuf is so widespread and the header is
> included in some other headers that do not need to, so your build
> may happen to work without including <strbuf.h>, for example.

Yeah, I 100% understand this. I simply found it weird that there were
many missing headers, so I was scared that I was not understanding the
guidelines.

I will add a 6th patch that adds these missing headers, in order to
comply with the guidelines.

Thanks,
Lorenzo