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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 20, 2015, 19:05 UTC
Message-ID: <xmqq61497pc1.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1440079587-26491-1-git-send-email-szeder@ira.uka.de>

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.

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. 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.

Show 28 quoted lines

> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>  wt-status.c | 1 -
>  wt-status.h | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index 717fd48d13..c327fe8128 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1,5 +1,4 @@
>  #include "cache.h"
> -#include "pathspec.h"
>  #include "wt-status.h"
>  #include "object.h"
>  #include "dir.h"
> diff --git a/wt-status.h b/wt-status.h
> index e0a99f75c7..c9b3b744e9 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -4,6 +4,7 @@
>  #include <stdio.h>
>  #include "string-list.h"
>  #include "color.h"
> +#include "pathspec.h"
>  
>  enum color_wt_status {
>  	WT_STATUS_HEADER = 0,