From: SZEDER Gábor Date: Thu, 20 Aug 2015 22:35:06 GMT Subject: Re: [PATCH] wt-status: move #include "pathspec.h" to the header Message-ID: <20150821003506.Horde.cIQHHynKCAfMFot8jSGGhg7@webmail.informatik.kit.edu> In-Reply-To: Quoting Junio C Hamano : > SZEDER Gábor 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. 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.) > But for that goal, > direct inclusion of 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.