Re: [PATCH] wt-status: move #include "pathspec.h" to the header

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Aug 20, 2015, 22:35 UTC
Message-ID: <20150821003506.Horde.cIQHHynKCAfMFot8jSGGhg7@webmail.informatik.kit.edu>
In-Reply-To: <xmqq61497pc1.fsf@gitster.dls.corp.google.com>

Quoting Junio C Hamano <gitster@pobox.com>:

Show 10 quoted lines

> SZEDER Gábor <szeder@ira.uka.de> writes:
>
>> The declaration of 'struct wt_status' requires the declararion of 'struct
>> pathspec'.
>
> I think this is fine.
>
> I am guessing that you are saying it is wrong to force wt-status.c
> to include pathspec.h before including wt-status.h; I am fine with
> that.

Show 6 quoted lines

> This is a tangent, but the above is different from saying that with
> a single liner test.c that has
>
>   #include "wt-status.h"
>
> your compilation "cc -c test.c" should succeed.

Sort of, but not quite, rather a combination of both. My point is a builtin command, starting with #include "builtin.h" as it should, that wanted to use only struct wt_status_state and wt_status_get_state() from the wt-status machinery and has nothing to do with pathspecs, yet it's not sufficient to #include "wt-status.h", because the required pathspec.h is included in the .c file instead of the header itsef. (Actually, that's exactly how I noticed.)

Show 15 quoted lines

> But for that goal,
> direct inclusion of <stdio.h> to wt-status.h is also iffy.  We
> include the system headers from git-compat-util.h because some
> platforms are picky about order of inclusion of system header files
> and definitions of feature test macros.
>
> Right now, the codebase is correct only because it is NOT our goal
> to guarantee that such a single-liner test.c file compiles.
> Instead, everybody is instructed to #include "git-compat-util.h" as
> the first thing, either directly or indirectly.
>
> So in that sense, we should also remove that inclusion from
> wt-status.h, I think.
>
> Thanks.