Subject: Re: [RFC PATCH 2/2] headers: include dependent headers

From: Junio C Hamano <[email-masked]>
Date: Mon, 08 Sep 2014 11:05:50 -0700

René Scharfe <[email-masked]> writes:

> Am 06.09.2014 um 21:20 schrieb David Aguilar:
>> Add dependent headers so that including a header does not
>> require including additional headers.
>>
>> This makes it so that "gcc -c $header" succeeds for each header.
>>
>> Signed-off-by: David Aguilar <[email-masked]>
>> ---
>
>> diff --git a/branch.h b/branch.h
>> index 64173ab..a61fd1a 100644
>> --- a/branch.h
>> +++ b/branch.h
>> @@ -3,6 +3,9 @@
>>
>>   /* Functions for acting on the information about branches. */
>>
>> +#include "cache.h"
>> +#include "strbuf.h"
>
> cache.h includes strbuf.h, so the line above isn't necessary.

I think the primary objective of these two patches is to allow
anybody who wants to use a feature X to include the header H that is
about that feature without having to know what other header F and G
are required by the implementation of header H.

And I think that cuts both ways.  Somebody who wants to use a
feature defined in the header H shouldn't have to know that other
headers I and J he includes to use other features from happen to
already include H for their own use.

Here, branch.h does want to see "struct strbuf" for its own use, so
including "strbuf.h" is the right thing to do and in line with the
primary objective, no?  It shouldn't have to know that "cache.h"
happens to include "strbuf.h".

While assessing these two patches, we would need to decide what to
do with the "include git-compat-util.h first in any C source" rule
in the coding guidelines.  I think it is still a good practical
exception (in the sense that even if you do not use FLEX_ARRAY,
signed_add_overflows(), etc. yourself, you should follow the rule),
even if we choose to adopt "One that wants to use features from a
header only needs to include that header".