{"thread":{"id":"64281","subject":"[PATCH 0/2] Organize mingw includes","startedAt":"2025-10-09T07:46:05Z","lastAt":"2025-10-13T16:29:30Z","messageCount":9,"participants":["Johannes Schindelin via GitGitGadget","Patrick Steinhardt","Johannes Schindelin","Junio C Hamano","Matthias Aßhauer","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"528367","messageId":"pull.1985.git.1759995961.gitgitgadget@gmail.com","threadId":"64281","inReplyTo":null,"subject":"[PATCH 0/2] Organize mingw includes","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-09T07:45:59Z","receivedAt":"2025-10-09T07:46:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Following in the footsteps of the many, many recent #include refactorings,\nthis patch series orders the #include statements in compat/mingw.c.\n\nJohannes Schindelin (2):\n mingw: avoid relative `#include`s\n mingw: order `#include`s alphabetically\n\n compat/mingw.c | 30 +++++++++++++++---------------\n 1 file changed, 15 insertions(+), 15 deletions(-)\n\n\nbase-commit: 79cf913ea9321f774da29b2330b5781d5ff420ef\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1985%2Fdscho%2Forganize-mingw-includes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1985/dscho/organize-mingw-includes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1985\n-- \ngitgitgadget\n"},{"id":"528368","messageId":"484ef8b825e5d1d68a61f0f8cc2520457e8f4f3a.1759995961.git.gitgitgadget@gmail.com","threadId":"64281","inReplyTo":"pull.1985.git.1759995961.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mingw: avoid relative `#include`s","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-09T07:46:00Z","receivedAt":"2025-10-09T07:46:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe want to make them relative to the top-level directory.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 8538e3d172..da99473f56 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1,22 +1,22 @@\n #define USE_THE_REPOSITORY_VARIABLE\n #define DISABLE_SIGN_COMPARE_WARNINGS\n \n-#include \"../git-compat-util.h\"\n+#include \"git-compat-util.h\"\n #include \"win32.h\"\n #include \n #include \n #include \n #include \n-#include \"../strbuf.h\"\n-#include \"../run-command.h\"\n-#include \"../abspath.h\"\n-#include \"../alloc.h\"\n+#include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"abspath.h\"\n+#include \"alloc.h\"\n #include \"win32/lazyload.h\"\n-#include \"../config.h\"\n-#include \"../environment.h\"\n-#include \"../trace2.h\"\n-#include \"../symlinks.h\"\n-#include \"../wrapper.h\"\n+#include \"config.h\"\n+#include \"environment.h\"\n+#include \"trace2.h\"\n+#include \"symlinks.h\"\n+#include \"wrapper.h\"\n #include \"dir.h\"\n #include \"gettext.h\"\n #define SECURITY_WIN32\n-- \ngitgitgadget\n\n"},{"id":"528369","messageId":"5303aa57c4e5aa8e88b4a3d553ff3a69dbe54871.1759995961.git.gitgitgadget@gmail.com","threadId":"64281","inReplyTo":"pull.1985.git.1759995961.gitgitgadget@gmail.com","subject":"[PATCH 2/2] mingw: order `#include`s alphabetically","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-10-09T07:46:01Z","receivedAt":"2025-10-09T07:46:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt allows for more consistent patches that way.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex da99473f56..736a07a028 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2,25 +2,25 @@\n #define DISABLE_SIGN_COMPARE_WARNINGS\n \n #include \"git-compat-util.h\"\n-#include \"win32.h\"\n-#include \n-#include \n-#include \n-#include \n-#include \"strbuf.h\"\n-#include \"run-command.h\"\n #include \"abspath.h\"\n #include \"alloc.h\"\n-#include \"win32/lazyload.h\"\n #include \"config.h\"\n+#include \"dir.h\"\n #include \"environment.h\"\n-#include \"trace2.h\"\n+#include \"gettext.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n #include \"symlinks.h\"\n+#include \"trace2.h\"\n+#include \"win32.h\"\n+#include \"win32/lazyload.h\"\n #include \"wrapper.h\"\n-#include \"dir.h\"\n-#include \"gettext.h\"\n+#include \n+#include \n+#include \n #define SECURITY_WIN32\n #include \n+#include \n #include \n \n #define STATUS_DELETE_PENDING ((NTSTATUS) 0xC0000056)\n-- \ngitgitgadget\n"},{"id":"528480","messageId":"aOjXjEJePkpCDoI3@pks.im","threadId":"64281","inReplyTo":"pull.1985.git.1759995961.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Organize mingw includes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-10T09:53:16Z","receivedAt":"2025-10-10T09:53:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 09, 2025 at 07:45:59AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> Following in the footsteps of the many, many recent #include refactorings,\n> this patch series orders the #include statements in compat/mingw.c.\n\nBoth of these patches look good to me and I like the improved\nconsistency that they bring. I may also do the same for our code in\n\"refs/\", where some of the files use relative includes, as well. Might\nbe worth to document this somewhere if it isn't already, but that\ndoesn't have to be part of your patch series here.\n\nSorting them also makes sense. It's another thing where I wish that we\nhad a tool to enforce this. clang-format supports this in theory, but\nit's disabled right now. And I'm not even sure whether it can be told to\ninclude e.g. \"git-compat-util.h\" first.\n\nThanks!\n\nPatrick\n"},{"id":"528508","messageId":"c4a03e23-1c14-05b7-25ad-a3d6998df876@gmx.de","threadId":"64281","inReplyTo":"aOjXjEJePkpCDoI3@pks.im","subject":"Re: [PATCH 0/2] Organize mingw includes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-10-10T13:55:55Z","receivedAt":"2025-10-10T13:55:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Patrick,\n\nOn Fri, 10 Oct 2025, Patrick Steinhardt wrote:\n\n> On Thu, Oct 09, 2025 at 07:45:59AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > Following in the footsteps of the many, many recent #include refactorings,\n> > this patch series orders the #include statements in compat/mingw.c.\n> \n> Both of these patches look good to me and I like the improved\n> consistency that they bring. I may also do the same for our code in\n> \"refs/\", where some of the files use relative includes, as well. Might\n> be worth to document this somewhere if it isn't already, but that\n> doesn't have to be part of your patch series here.\n\nThere's already such a lot of documentation that I fear it is intimidating\nto any new contributor (and if I was one, I'd be tempted to read it\nexclusively via AI summaries). I don't want to add to that amount.\n\n> Sorting them also makes sense. It's another thing where I wish that we\n> had a tool to enforce this. clang-format supports this in theory, but\n> it's disabled right now. And I'm not even sure whether it can be told to\n> include e.g. \"git-compat-util.h\" first.\n\nIn theory, I am totally with you: Sorting `#include`s is a job best left\nto tools. But then, I say the same about formatting, and I see a lot of\nappetite on this list for preventing that to become a tool-driven job, and\ninstead requiring developer time to be spent on it. Therefore, I don't\nwant to spend any effort trying to get this automated when I see small\nchances of success in the endeavor of freeing up cognitive cycles for\ntasks I find more intellectually rewarding than figuring out where a space\nor a line break goes.\n\nCiao,\nJohannes\n"},{"id":"528520","messageId":"xmqqfrbqafxj.fsf@gitster.g","threadId":"64281","inReplyTo":"c4a03e23-1c14-05b7-25ad-a3d6998df876@gmx.de","subject":"Re: [PATCH 0/2] Organize mingw includes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-10T16:18:32Z","receivedAt":"2025-10-10T16:18:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Sorting them also makes sense. It's another thing where I wish that we\n>> had a tool to enforce this. clang-format supports this in theory, but\n>> it's disabled right now. And I'm not even sure whether it can be told to\n>> include e.g. \"git-compat-util.h\" first.\n>\n> In theory, I am totally with you: Sorting `#include`s is a job best left\n> to tools. But then, I say the same about formatting,\n\nI am afraid that it is apples-to-oranges comparison. Nobody has to\nread the #include directives; they may have to see if a header they\ncare about (because they are planning to add a call to a function\nthat hasn't been used in the particular C source file, perhaps) is\nalready included, and sorted list of includes is a good tool to help\nthem. IOW, they do not read them, they scan in them. But the code,\nthe result of formatting, must be readable by people.\n"},{"id":"528563","messageId":"DB9P250MB06929F022ADC9A0544947BDAA5ECA@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM","threadId":"64281","inReplyTo":"484ef8b825e5d1d68a61f0f8cc2520457e8f4f3a.1759995961.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] mingw: avoid relative `#include`s","fromName":"Matthias Aßhauer","fromEmail":"mha1993@live.de","sentAt":"2025-10-11T09:03:04Z","receivedAt":"2025-10-11T09:03:15Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"On Thu, 9 Oct 2025, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> We want to make them relative to the top-level directory.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/mingw.c | 20 ++++++++++----------\n> 1 file changed, 10 insertions(+), 10 deletions(-)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 8538e3d172..da99473f56 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1,22 +1,22 @@\n> #define USE_THE_REPOSITORY_VARIABLE\n> #define DISABLE_SIGN_COMPARE_WARNINGS\n>\n> -#include \"../git-compat-util.h\"\n> +#include \"git-compat-util.h\"\n> #include \"win32.h\"\n\nIsn't this include also relative to compat/ and should become #include \n\"compat/win32.h\"?\n\n> #include \n> #include \n> #include \n> #include \n> -#include \"../strbuf.h\"\n> -#include \"../run-command.h\"\n> -#include \"../abspath.h\"\n> -#include \"../alloc.h\"\n> +#include \"strbuf.h\"\n> +#include \"run-command.h\"\n> +#include \"abspath.h\"\n> +#include \"alloc.h\"\n> #include \"win32/lazyload.h\"\n\nSame situation here.\n\nbest regards\n\nMatthias\n"},{"id":"528581","messageId":"e593886e-eeb4-440f-a317-a2959577e1e3@kdbg.org","threadId":"64281","inReplyTo":"484ef8b825e5d1d68a61f0f8cc2520457e8f4f3a.1759995961.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] mingw: avoid relative `#include`s","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-10-12T11:45:50Z","receivedAt":"2025-10-12T11:45:59Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.10.25 um 09:46 schrieb Johannes Schindelin via GitGitGadget:\n> From: Johannes Schindelin \n> \n> We want to make them relative to the top-level directory.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/mingw.c | 20 ++++++++++----------\n> 1 file changed, 10 insertions(+), 10 deletions(-)\n> \n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 8538e3d172..da99473f56 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1,22 +1,22 @@\n> #define USE_THE_REPOSITORY_VARIABLE\n> #define DISABLE_SIGN_COMPARE_WARNINGS\n> \n> -#include \"../git-compat-util.h\"\n> +#include \"git-compat-util.h\"\n> #include \"win32.h\"\n> #include \n> #include \n> #include \n> #include \n> -#include \"../strbuf.h\"\n> -#include \"../run-command.h\"\n> -#include \"../abspath.h\"\n> -#include \"../alloc.h\"\n> +#include \"strbuf.h\"\n> +#include \"run-command.h\"\n> +#include \"abspath.h\"\n> +#include \"alloc.h\"\n> #include \"win32/lazyload.h\"\n> -#include \"../config.h\"\n> -#include \"../environment.h\"\n> -#include \"../trace2.h\"\n> -#include \"../symlinks.h\"\n> -#include \"../wrapper.h\"\n> +#include \"config.h\"\n> +#include \"environment.h\"\n> +#include \"trace2.h\"\n> +#include \"symlinks.h\"\n> +#include \"wrapper.h\"\n> #include \"dir.h\"\n> #include \"gettext.h\"\n> #define SECURITY_WIN32\n\nWhy is this needed?\n\nWith #include \"foo\" it is quite clear that the file is first looked up\nfrom the directory of the file being processed. The changed code\nrequires that the top-level directory is among the -I directives of the\ncommand lines. Then it would be much more logical to use #include \ninstead. But that I wouldn't regard as desirable, either, because the\nincluded file isn't from a subordinate module or library.\n\nSo, IMO, the status quo is perfect and does not need this change.\n\n-- Hannes\n\n"},{"id":"528637","messageId":"xmqqa51u3guw.fsf@gitster.g","threadId":"64281","inReplyTo":"e593886e-eeb4-440f-a317-a2959577e1e3@kdbg.org","subject":"Re: [PATCH 1/2] mingw: avoid relative `#include`s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-13T16:29:27Z","receivedAt":"2025-10-13T16:29:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Why is this needed?\n\n;-)\n\nAs pointed out by Matthias, the changes in the posted patch are not\ncomplete/comprehensive.\n\n> With #include \"foo\" it is quite clear that the file is first looked up\n> from the directory of the file being processed. The changed code\n> requires that the top-level directory is among the -I directives of the\n> command lines. Then it would be much more logical to use #include \n> instead.\n\nI actually prefer that, but that is a taste thing I do not want to\nimpose on this project.\n\n> So, IMO, the status quo is perfect and does not need this change.\n\nI tend to agree, but it would be a waste of time to further discuss\non this. As long as it does not break compilation, I'll just let\nthe patch graduate.\n\nMy preference is to \n\n * always name custom headers using the path from the top-level (we\n use -I in BASIC_CFLAGS exactly for this purpose), \n e.g.\n\n\t#include \"compat/win32.h\" (good)\n\t#include \"win32.h\" (not good)\n\n * compat header that aims to replace system supplied headers like\n should use -I appropriately and appear as if they are\n from the system, e.g.\n\n\t#include (good)\n\t#include \"compat/regex/regex.h\" (not good)\n\nIf somebody truly wants to improve things once the dust settles from\nthese patches, I would appreciate they keep the above in mind.\n\nTHanks.\n"}]}