Re: [GSoC PATCH v4 0/5] preserve promisor files content after repack
- From
Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com>
- Date
- Apr 10, 2026, 16:44 UTC
- Message-ID
- <adko0kvU5WX69GYQ@lorenzo-VM>
- In-Reply-To
- <xmqq1pgmrf4w.fsf@gitster.g>
On Fri, Apr 10, 2026 at 08:47:43AM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> LorenzoPegorari <lorenzo.pegorari2002@gmail.com> 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.
Show 17 quoted lines
> 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