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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 8, 2014, 18:05 UTC
Message-ID: <xmqqbnqqng1t.fsf@gitster.dls.corp.google.com>
In-Reply-To: <540B7AA3.1090201@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 21 quoted lines

> 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 <davvid@gmail.com>
>> ---
>
>> 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".