{"thread":{"id":"40141","subject":"[PATCH] wt-status: move #include \"pathspec.h\" to the header","startedAt":"2015-08-20T14:06:27Z","lastAt":"2015-08-20T22:35:06Z","messageCount":4,"participants":["SZEDER Gábor","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"268364","messageId":"1440079587-26491-1-git-send-email-szeder@ira.uka.de","threadId":"40141","inReplyTo":null,"subject":"[PATCH] wt-status: move #include \"pathspec.h\" to the header","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-08-20T14:06:27Z","receivedAt":"2015-08-20T14:06:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The declaration of 'struct wt_status' requires the declararion of 'struct\npathspec'.\n\nSigned-off-by: SZEDER Gábor \n---\n wt-status.c | 1 -\n wt-status.h | 1 +\n 2 files changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 717fd48d13..c327fe8128 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1,5 +1,4 @@\n #include \"cache.h\"\n-#include \"pathspec.h\"\n #include \"wt-status.h\"\n #include \"object.h\"\n #include \"dir.h\"\ndiff --git a/wt-status.h b/wt-status.h\nindex e0a99f75c7..c9b3b744e9 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -4,6 +4,7 @@\n #include \n #include \"string-list.h\"\n #include \"color.h\"\n+#include \"pathspec.h\"\n \n enum color_wt_status {\n \tWT_STATUS_HEADER = 0,\n-- \n2.5.0.415.g33d64ef\n"},{"id":"268385","messageId":"xmqq61497pc1.fsf@gitster.dls.corp.google.com","threadId":"40141","inReplyTo":"1440079587-26491-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] wt-status: move #include \"pathspec.h\" to the header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-20T19:05:34Z","receivedAt":"2015-08-20T19:05:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> The declaration of 'struct wt_status' requires the declararion of 'struct\n> pathspec'.\n\nI think this is fine.\n\nI am guessing that you are saying it is wrong to force wt-status.c\nto include pathspec.h before including wt-status.h; I am fine with\nthat.\n\nThis is a tangent, but the above is different from saying that with\na single liner test.c that has\n\n #include \"wt-status.h\"\n\nyour compilation \"cc -c test.c\" should succeed. But for that goal,\ndirect inclusion of to wt-status.h is also iffy. We\ninclude the system headers from git-compat-util.h because some\nplatforms are picky about order of inclusion of system header files\nand definitions of feature test macros.\n\nRight now, the codebase is correct only because it is NOT our goal\nto guarantee that such a single-liner test.c file compiles.\nInstead, everybody is instructed to #include \"git-compat-util.h\" as\nthe first thing, either directly or indirectly.\n\nSo in that sense, we should also remove that inclusion from\nwt-status.h, I think.\n\nThanks.\n\n> Signed-off-by: SZEDER Gábor \n> ---\n> wt-status.c | 1 -\n> wt-status.h | 1 +\n> 2 files changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index 717fd48d13..c327fe8128 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1,5 +1,4 @@\n> #include \"cache.h\"\n> -#include \"pathspec.h\"\n> #include \"wt-status.h\"\n> #include \"object.h\"\n> #include \"dir.h\"\n> diff --git a/wt-status.h b/wt-status.h\n> index e0a99f75c7..c9b3b744e9 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -4,6 +4,7 @@\n> #include \n> #include \"string-list.h\"\n> #include \"color.h\"\n> +#include \"pathspec.h\"\n> \n> enum color_wt_status {\n> \tWT_STATUS_HEADER = 0,\n"},{"id":"268388","messageId":"20150820203841.GA12386@sigill.intra.peff.net","threadId":"40141","inReplyTo":"xmqq61497pc1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] wt-status: move #include \"pathspec.h\" to the header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-20T20:38:41Z","receivedAt":"2015-08-20T20:38:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 20, 2015 at 12:05:34PM -0700, Junio C Hamano wrote:\n\n> This is a tangent, but the above is different from saying that with\n> a single liner test.c that has\n> \n> #include \"wt-status.h\"\n> \n> your compilation \"cc -c test.c\" should succeed. But for that goal,\n> direct inclusion of to wt-status.h is also iffy. We\n> include the system headers from git-compat-util.h because some\n> platforms are picky about order of inclusion of system header files\n> and definitions of feature test macros.\n> \n> Right now, the codebase is correct only because it is NOT our goal\n> to guarantee that such a single-liner test.c file compiles.\n> Instead, everybody is instructed to #include \"git-compat-util.h\" as\n> the first thing, either directly or indirectly.\n> \n> So in that sense, we should also remove that inclusion from\n> wt-status.h, I think.\n\nYeah, I think that is actively wrong. Running[1]:\n\n git grep '#include <' -- '*.h' ':!git-compat-util.h'\n\nshows a few other weird ones. Things like zlib.h, pcre.h, etc, are\nprobably OK. They are not really \"system\" headers, and it is probably OK\nas long as they come after git-compat-util (which they must, if they are\nin a header file).\n\nThe inclusion of stdlib.h in compat/bswap.h is suspect (both are\nincluded by git-compat-util itself). But it is inside an #ifdef _MSC_VER\nblock, so I cannot test that there is not some subtle interaction going\non.\n\nA few headers include pthread.h. I would have thought that should go in\ngit-compat-util for the usual reasons, but nobody has complained (and\nmost files do not need it, though that has not generally stopped us from\nmaking git-compat-util a catch-all for other system files).\n\n-Peff\n\n[1] Too bad I needed the \"--\" separator there. Recent git nicely learned\n to DWIM \"*.h\" as a pathspec, but we do not seem to give the same\n treatment to \"magic\" pathspecs.\n"},{"id":"268391","messageId":"20150821003506.Horde.cIQHHynKCAfMFot8jSGGhg7@webmail.informatik.kit.edu","threadId":"40141","inReplyTo":"xmqq61497pc1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] wt-status: move #include \"pathspec.h\" to the header","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-08-20T22:35:06Z","receivedAt":"2015-08-20T22:35:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Junio C Hamano :\n\n> SZEDER Gábor writes:\n>\n>> The declaration of 'struct wt_status' requires the declararion of 'struct\n>> pathspec'.\n>\n> I think this is fine.\n>\n> I am guessing that you are saying it is wrong to force wt-status.c\n> to include pathspec.h before including wt-status.h; I am fine with\n> that.\n\n> This is a tangent, but the above is different from saying that with\n> a single liner test.c that has\n>\n> #include \"wt-status.h\"\n>\n> your compilation \"cc -c test.c\" should succeed.\n\nSort of, but not quite, rather a combination of both. My point is a\nbuiltin command, starting with #include \"builtin.h\" as it should, that\nwanted to use only struct wt_status_state and wt_status_get_state()\nfrom the wt-status machinery and has nothing to do with pathspecs, yet\nit's not sufficient to #include \"wt-status.h\", because the required\npathspec.h is included in the .c file instead of the header itsef.\n(Actually, that's exactly how I noticed.)\n\n\n> But for that goal,\n> direct inclusion of to wt-status.h is also iffy. We\n> include the system headers from git-compat-util.h because some\n> platforms are picky about order of inclusion of system header files\n> and definitions of feature test macros.\n>\n> Right now, the codebase is correct only because it is NOT our goal\n> to guarantee that such a single-liner test.c file compiles.\n> Instead, everybody is instructed to #include \"git-compat-util.h\" as\n> the first thing, either directly or indirectly.\n>\n> So in that sense, we should also remove that inclusion from\n> wt-status.h, I think.\n>\n> Thanks.\n"}]}