{"thread":{"id":"60898","subject":"[PATCH] unit-tests: do show relative file paths on non-Windows, too","startedAt":"2024-02-11T08:57:57Z","lastAt":"2024-02-13T20:49:04Z","messageCount":9,"participants":["Junio C Hamano","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"488384","messageId":"xmqqttmf9y46.fsf@gitster.g","threadId":"60898","inReplyTo":null,"subject":"[PATCH] unit-tests: do show relative file paths on non-Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-11T08:57:45Z","receivedAt":"2024-02-11T08:57:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are compilers other than Visual C that want to show absolute\npaths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\nshow relative file paths, 2023-09-25) so that it can also work with\na path that uses slash as the directory separator, and becomes\nalmost no-op once one-time preparation finds out that we are using a\ncompiler that already gives relative paths.  Incidentally, this also\nshould do the right thing on Windows with a compiler that shows\nrelative paths but with backslash as the directory separator (if\nsuch a thing exists and is used to build git).\n\nReported-by: Randall S. Becker <rsbecker@nexbridge.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Another change I made, which is not described in the proposed\n   commit log message, is that we now use fspathcmp() instead of\n   strcmp() to precompute the prefix length using a known needle[]\n   string, to be consistent with the runtime check done for each and\n   every path.\n\n   This is a belated follow-up on <f0b804129e8a21449cbb6f346473d3570182ddfa.1695640837.git.gitgitgadget@gmail.com>\n\n t/unit-tests/test-lib.c | 60 ++++++++++++++++++++++++++++++++---------\n 1 file changed, 47 insertions(+), 13 deletions(-)\n\ndiff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\nindex 7bf9dfdb95..83c9eb8c59 100644\n--- a/t/unit-tests/test-lib.c\n+++ b/t/unit-tests/test-lib.c\n@@ -21,12 +21,11 @@ static struct {\n \t.result = RESULT_NONE,\n };\n \n-#ifndef _MSC_VER\n-#define make_relative(location) location\n-#else\n /*\n  * Visual C interpolates the absolute Windows path for `__FILE__`,\n  * but we want to see relative paths, as verified by t0080.\n+ * There are other compilers that do the same, and are not for\n+ * Windows.\n  */\n #include \"dir.h\"\n \n@@ -34,32 +33,67 @@ static const char *make_relative(const char *location)\n {\n \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n \tstatic size_t prefix_len;\n+\tstatic int need_bs_to_fs = -1;\n \n-\tif (!prefix_len) {\n+\t/* one-time preparation */\n+\tif (need_bs_to_fs < 0) {\n \t\tsize_t len = strlen(prefix);\n-\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n+\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n \t\tsize_t needle_len = strlen(needle);\n \n-\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n+\t\tif (len < needle_len)\n+\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n+\n+\t\t/*\n+\t\t * The path could be relative (t/unit-tests/test-lib.c)\n+\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n+\t\t * Check the slash between \"t\" and \"unit-tests\".\n+\t\t */\n+\t\tprefix_len = len - needle_len;\n+\t\tif (prefix[prefix_len + 1] == '/') {\n+\t\t\t/* Oh, we're not Windows */\n+\t\t\tfor (size_t i = 0; i < needle_len; i++)\n+\t\t\t\tif (needle[i] == '\\\\')\n+\t\t\t\t\tneedle[i] = '/';\n+\t\t\tneed_bs_to_fs = 0;\n+\t\t} else {\n+\t\t\tneed_bs_to_fs = 1;\n+\t\t}\n+\n+\t\t/*\n+\t\t * prefix_len == 0 if the compiler gives paths relative\n+\t\t * to the root of the working tree.  Otherwise, we want\n+\t\t * to see that we did find the needle[] at a directory\n+\t\t * boundary.\n+\t\t */\n+\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n+\t\t    (prefix_len &&\n+\t\t     prefix[prefix_len - 1] != '/' &&\n+\t\t     prefix[prefix_len - 1] != '\\\\'))\n \t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n \n-\t\t/* let it end in a directory separator */\n-\t\tprefix_len = len - needle_len + 1;\n \t}\n \n+\t/*\n+\t * If our compiler gives relative paths and we do not need\n+\t * to munge directory separator, we can return location as-is.\n+\t */\n+\tif (!prefix_len && !need_bs_to_fs)\n+\t\treturn location;\n+\n \t/* Does it not start with the expected prefix? */\n \tif (fspathncmp(location, prefix, prefix_len))\n \t\treturn location;\n \n \tstrlcpy(buf, location + prefix_len, sizeof(buf));\n \t/* convert backslashes to forward slashes */\n-\tfor (p = buf; *p; p++)\n-\t\tif (*p == '\\\\')\n-\t\t\t*p = '/';\n-\n+\tif (need_bs_to_fs) {\n+\t\tfor (p = buf; *p; p++)\n+\t\t\tif (*p == '\\\\')\n+\t\t\t\t*p = '/';\n+\t}\n \treturn buf;\n }\n-#endif\n \n static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n {\n-- \n2.44.0-rc0\n\n"},{"id":"488385","messageId":"6872b42d-8763-44dc-9502-2362d1ed80a7@gmail.com","threadId":"60898","inReplyTo":"xmqqttmf9y46.fsf@gitster.g","subject":"Re: [PATCH] unit-tests: do show relative file paths on non-Windows, too","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-11T11:03:55Z","receivedAt":"2024-02-11T11:03:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 11/02/2024 08:57, Junio C Hamano wrote:\n> There are compilers other than Visual C that want to show absolute\n> paths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\n> show relative file paths, 2023-09-25) so that it can also work with\n> a path that uses slash as the directory separator, and becomes\n> almost no-op once one-time preparation finds out that we are using a\n> compiler that already gives relative paths.  Incidentally, this also\n> should do the right thing on Windows with a compiler that shows\n> relative paths but with backslash as the directory separator (if\n> such a thing exists and is used to build git).\n> \n> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>   * Another change I made, which is not described in the proposed\n>     commit log message, is that we now use fspathcmp() instead of\n>     strcmp() to precompute the prefix length using a known needle[]\n>     string, to be consistent with the runtime check done for each and\n>     every path.\n> \n>     This is a belated follow-up on <f0b804129e8a21449cbb6f346473d3570182ddfa.1695640837.git.gitgitgadget@gmail.com>\n\nThanks for putting this together - I was about to sit down and write \nsomething similar when I saw your patch. I've left one comment below but \nI don't think it is worth a re-roll, this looks good to me.\n\n> +\t\t/*\n> +\t\t * prefix_len == 0 if the compiler gives paths relative\n> +\t\t * to the root of the working tree.  Otherwise, we want\n> +\t\t * to see that we did find the needle[] at a directory\n> +\t\t * boundary.\n> +\t\t */\n> +\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n> +\t\t    (prefix_len &&\n> +\t\t     prefix[prefix_len - 1] != '/' &&\n> +\t\t     prefix[prefix_len - 1] != '\\\\'))\n\nWe know which separator we're expecting so we could replace  the last \ntwo comparisons with\n\n\t\tprefix[prefix_len -1] != needle[1]\n\nbut as I say I'm not sure that is worth re-rolling for\n\nBest Wishes\n\nPhillip\n"},{"id":"488396","messageId":"xmqqle7r9enn.fsf_-_@gitster.g","threadId":"60898","inReplyTo":"6872b42d-8763-44dc-9502-2362d1ed80a7@gmail.com","subject":"[PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-11T15:58:04Z","receivedAt":"2024-02-11T15:58:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">\n> We know which separator we're expecting so we could replace  the last\n> two comparisons with\n>\n> \t\tprefix[prefix_len -1] != needle[1]\n>\n> but as I say I'm not sure that is worth re-rolling for\n\nThere is a larger clean-up opportunity to drop the need for making a\ncopy, which probably is worth doing, so I folded the above into this\nversion.\n\n------- >8 ------------- >8 ------------- >8 ------------- >8 -------\n\nThere are compilers other than Visual C that want to show absolute\npaths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\nshow relative file paths, 2023-09-25) so that it can also work with\na path that uses slash as the directory separator, and becomes\nalmost no-op once one-time preparation finds out that we are using a\ncompiler that already gives relative paths.  Incidentally, this also\nshould do the right thing on Windows with a compiler that shows\nrelative paths but with backslash as the directory separator (if\nsuch a thing exists and is used to build git).\n\nReported-by: Randall S. Becker <rsbecker@nexbridge.com>\nHelped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n  * I found that the diff relative to the result of applying v1 was\n    easier to follow than the range-diff, so here it is.\n\n  diff --git c/t/unit-tests/test-lib.c w/t/unit-tests/test-lib.c\n  index 83c9eb8c59..66d6980ffb 100644\n  --- c/t/unit-tests/test-lib.c\n  +++ w/t/unit-tests/test-lib.c\n  @@ -64,34 +64,33 @@ static const char *make_relative(const char *location)\n   \t\t * prefix_len == 0 if the compiler gives paths relative\n   \t\t * to the root of the working tree.  Otherwise, we want\n   \t\t * to see that we did find the needle[] at a directory\n  -\t\t * boundary.\n  +\t\t * boundary.  Again we rely on that needle[] begins with\n  +\t\t * \"t\" followed by the directory separator.\n   \t\t */\n   \t\tif (fspathcmp(needle, prefix + prefix_len) ||\n  -\t\t    (prefix_len &&\n  -\t\t     prefix[prefix_len - 1] != '/' &&\n  -\t\t     prefix[prefix_len - 1] != '\\\\'))\n  +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n   \t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n  -\n   \t}\n   \n   \t/*\n  -\t * If our compiler gives relative paths and we do not need\n  -\t * to munge directory separator, we can return location as-is.\n  +\t * Does it not start with the expected prefix?\n  +\t * Return it as-is without making it worse.\n   \t */\n  -\tif (!prefix_len && !need_bs_to_fs)\n  +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n   \t\treturn location;\n   \n  -\t/* Does it not start with the expected prefix? */\n  -\tif (fspathncmp(location, prefix, prefix_len))\n  -\t\treturn location;\n  +\t/*\n  +\t * If we do not need to munge directory separator, we can return\n  +\t * the substring at the tail of the location.\n  +\t */\n  +\tif (!need_bs_to_fs)\n  +\t\treturn location + prefix_len;\n   \n  -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n   \t/* convert backslashes to forward slashes */\n  -\tif (need_bs_to_fs) {\n  -\t\tfor (p = buf; *p; p++)\n  -\t\t\tif (*p == '\\\\')\n  -\t\t\t\t*p = '/';\n  -\t}\n  +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n  +\tfor (p = buf; *p; p++)\n  +\t\tif (*p == '\\\\')\n  +\t\t\t*p = '/';\n   \treturn buf;\n   }\n   \n\n t/unit-tests/test-lib.c | 61 +++++++++++++++++++++++++++++++----------\n 1 file changed, 47 insertions(+), 14 deletions(-)\n\ndiff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\nindex 7bf9dfdb95..66d6980ffb 100644\n--- a/t/unit-tests/test-lib.c\n+++ b/t/unit-tests/test-lib.c\n@@ -21,12 +21,11 @@ static struct {\n \t.result = RESULT_NONE,\n };\n \n-#ifndef _MSC_VER\n-#define make_relative(location) location\n-#else\n /*\n  * Visual C interpolates the absolute Windows path for `__FILE__`,\n  * but we want to see relative paths, as verified by t0080.\n+ * There are other compilers that do the same, and are not for\n+ * Windows.\n  */\n #include \"dir.h\"\n \n@@ -34,32 +33,66 @@ static const char *make_relative(const char *location)\n {\n \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n \tstatic size_t prefix_len;\n+\tstatic int need_bs_to_fs = -1;\n \n-\tif (!prefix_len) {\n+\t/* one-time preparation */\n+\tif (need_bs_to_fs < 0) {\n \t\tsize_t len = strlen(prefix);\n-\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n+\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n \t\tsize_t needle_len = strlen(needle);\n \n-\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n-\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n+\t\tif (len < needle_len)\n+\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n+\n+\t\t/*\n+\t\t * The path could be relative (t/unit-tests/test-lib.c)\n+\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n+\t\t * Check the slash between \"t\" and \"unit-tests\".\n+\t\t */\n+\t\tprefix_len = len - needle_len;\n+\t\tif (prefix[prefix_len + 1] == '/') {\n+\t\t\t/* Oh, we're not Windows */\n+\t\t\tfor (size_t i = 0; i < needle_len; i++)\n+\t\t\t\tif (needle[i] == '\\\\')\n+\t\t\t\t\tneedle[i] = '/';\n+\t\t\tneed_bs_to_fs = 0;\n+\t\t} else {\n+\t\t\tneed_bs_to_fs = 1;\n+\t\t}\n \n-\t\t/* let it end in a directory separator */\n-\t\tprefix_len = len - needle_len + 1;\n+\t\t/*\n+\t\t * prefix_len == 0 if the compiler gives paths relative\n+\t\t * to the root of the working tree.  Otherwise, we want\n+\t\t * to see that we did find the needle[] at a directory\n+\t\t * boundary.  Again we rely on that needle[] begins with\n+\t\t * \"t\" followed by the directory separator.\n+\t\t */\n+\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n+\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n+\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n \t}\n \n-\t/* Does it not start with the expected prefix? */\n-\tif (fspathncmp(location, prefix, prefix_len))\n+\t/*\n+\t * Does it not start with the expected prefix?\n+\t * Return it as-is without making it worse.\n+\t */\n+\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n \t\treturn location;\n \n-\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n+\t/*\n+\t * If we do not need to munge directory separator, we can return\n+\t * the substring at the tail of the location.\n+\t */\n+\tif (!need_bs_to_fs)\n+\t\treturn location + prefix_len;\n+\n \t/* convert backslashes to forward slashes */\n+\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n \tfor (p = buf; *p; p++)\n \t\tif (*p == '\\\\')\n \t\t\t*p = '/';\n-\n \treturn buf;\n }\n-#endif\n \n static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n {\n-- \n2.44.0-rc0\n\n"},{"id":"488440","messageId":"c625239a-a847-475a-a228-9deb622c67bf@gmail.com","threadId":"60898","inReplyTo":"xmqqle7r9enn.fsf_-_@gitster.g","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-12T10:44:41Z","receivedAt":"2024-02-12T10:44:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 11/02/2024 15:58, Junio C Hamano wrote:\n>>\n>> We know which separator we're expecting so we could replace  the last\n>> two comparisons with\n>>\n>> \t\tprefix[prefix_len -1] != needle[1]\n>>\n>> but as I say I'm not sure that is worth re-rolling for\n> \n> There is a larger clean-up opportunity to drop the need for making a\n> copy, which probably is worth doing, so I folded the above into this\n> version.\n\nOoh, that's nice. This version looks good, I found the code comments \nvery helpful\n\nBest Wishes\n\nPhillip\n\n> ------- >8 ------------- >8 ------------- >8 ------------- >8 -------\n> \n> There are compilers other than Visual C that want to show absolute\n> paths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\n> show relative file paths, 2023-09-25) so that it can also work with\n> a path that uses slash as the directory separator, and becomes\n> almost no-op once one-time preparation finds out that we are using a\n> compiler that already gives relative paths.  Incidentally, this also\n> should do the right thing on Windows with a compiler that shows\n> relative paths but with backslash as the directory separator (if\n> such a thing exists and is used to build git).\n> \n> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>\n> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>    * I found that the diff relative to the result of applying v1 was\n>      easier to follow than the range-diff, so here it is.\n> \n>    diff --git c/t/unit-tests/test-lib.c w/t/unit-tests/test-lib.c\n>    index 83c9eb8c59..66d6980ffb 100644\n>    --- c/t/unit-tests/test-lib.c\n>    +++ w/t/unit-tests/test-lib.c\n>    @@ -64,34 +64,33 @@ static const char *make_relative(const char *location)\n>     \t\t * prefix_len == 0 if the compiler gives paths relative\n>     \t\t * to the root of the working tree.  Otherwise, we want\n>     \t\t * to see that we did find the needle[] at a directory\n>    -\t\t * boundary.\n>    +\t\t * boundary.  Again we rely on that needle[] begins with\n>    +\t\t * \"t\" followed by the directory separator.\n>     \t\t */\n>     \t\tif (fspathcmp(needle, prefix + prefix_len) ||\n>    -\t\t    (prefix_len &&\n>    -\t\t     prefix[prefix_len - 1] != '/' &&\n>    -\t\t     prefix[prefix_len - 1] != '\\\\'))\n>    +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n>     \t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>    -\n>     \t}\n>     \n>     \t/*\n>    -\t * If our compiler gives relative paths and we do not need\n>    -\t * to munge directory separator, we can return location as-is.\n>    +\t * Does it not start with the expected prefix?\n>    +\t * Return it as-is without making it worse.\n>     \t */\n>    -\tif (!prefix_len && !need_bs_to_fs)\n>    +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>     \t\treturn location;\n>     \n>    -\t/* Does it not start with the expected prefix? */\n>    -\tif (fspathncmp(location, prefix, prefix_len))\n>    -\t\treturn location;\n>    +\t/*\n>    +\t * If we do not need to munge directory separator, we can return\n>    +\t * the substring at the tail of the location.\n>    +\t */\n>    +\tif (!need_bs_to_fs)\n>    +\t\treturn location + prefix_len;\n>     \n>    -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>     \t/* convert backslashes to forward slashes */\n>    -\tif (need_bs_to_fs) {\n>    -\t\tfor (p = buf; *p; p++)\n>    -\t\t\tif (*p == '\\\\')\n>    -\t\t\t\t*p = '/';\n>    -\t}\n>    +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>    +\tfor (p = buf; *p; p++)\n>    +\t\tif (*p == '\\\\')\n>    +\t\t\t*p = '/';\n>     \treturn buf;\n>     }\n>     \n> \n>   t/unit-tests/test-lib.c | 61 +++++++++++++++++++++++++++++++----------\n>   1 file changed, 47 insertions(+), 14 deletions(-)\n> \n> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\n> index 7bf9dfdb95..66d6980ffb 100644\n> --- a/t/unit-tests/test-lib.c\n> +++ b/t/unit-tests/test-lib.c\n> @@ -21,12 +21,11 @@ static struct {\n>   \t.result = RESULT_NONE,\n>   };\n>   \n> -#ifndef _MSC_VER\n> -#define make_relative(location) location\n> -#else\n>   /*\n>    * Visual C interpolates the absolute Windows path for `__FILE__`,\n>    * but we want to see relative paths, as verified by t0080.\n> + * There are other compilers that do the same, and are not for\n> + * Windows.\n>    */\n>   #include \"dir.h\"\n>   \n> @@ -34,32 +33,66 @@ static const char *make_relative(const char *location)\n>   {\n>   \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n>   \tstatic size_t prefix_len;\n> +\tstatic int need_bs_to_fs = -1;\n>   \n> -\tif (!prefix_len) {\n> +\t/* one-time preparation */\n> +\tif (need_bs_to_fs < 0) {\n>   \t\tsize_t len = strlen(prefix);\n> -\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n> +\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n>   \t\tsize_t needle_len = strlen(needle);\n>   \n> -\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n> -\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n> +\t\tif (len < needle_len)\n> +\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n> +\n> +\t\t/*\n> +\t\t * The path could be relative (t/unit-tests/test-lib.c)\n> +\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n> +\t\t * Check the slash between \"t\" and \"unit-tests\".\n> +\t\t */\n> +\t\tprefix_len = len - needle_len;\n> +\t\tif (prefix[prefix_len + 1] == '/') {\n> +\t\t\t/* Oh, we're not Windows */\n> +\t\t\tfor (size_t i = 0; i < needle_len; i++)\n> +\t\t\t\tif (needle[i] == '\\\\')\n> +\t\t\t\t\tneedle[i] = '/';\n> +\t\t\tneed_bs_to_fs = 0;\n> +\t\t} else {\n> +\t\t\tneed_bs_to_fs = 1;\n> +\t\t}\n>   \n> -\t\t/* let it end in a directory separator */\n> -\t\tprefix_len = len - needle_len + 1;\n> +\t\t/*\n> +\t\t * prefix_len == 0 if the compiler gives paths relative\n> +\t\t * to the root of the working tree.  Otherwise, we want\n> +\t\t * to see that we did find the needle[] at a directory\n> +\t\t * boundary.  Again we rely on that needle[] begins with\n> +\t\t * \"t\" followed by the directory separator.\n> +\t\t */\n> +\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n> +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n> +\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>   \t}\n>   \n> -\t/* Does it not start with the expected prefix? */\n> -\tif (fspathncmp(location, prefix, prefix_len))\n> +\t/*\n> +\t * Does it not start with the expected prefix?\n> +\t * Return it as-is without making it worse.\n> +\t */\n> +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>   \t\treturn location;\n>   \n> -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n> +\t/*\n> +\t * If we do not need to munge directory separator, we can return\n> +\t * the substring at the tail of the location.\n> +\t */\n> +\tif (!need_bs_to_fs)\n> +\t\treturn location + prefix_len;\n> +\n>   \t/* convert backslashes to forward slashes */\n> +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>   \tfor (p = buf; *p; p++)\n>   \t\tif (*p == '\\\\')\n>   \t\t\t*p = '/';\n> -\n>   \treturn buf;\n>   }\n> -#endif\n>   \n>   static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n>   {\n\n"},{"id":"488476","messageId":"xmqqsf1x486b.fsf@gitster.g","threadId":"60898","inReplyTo":"c625239a-a847-475a-a228-9deb622c67bf@gmail.com","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-12T22:41:32Z","receivedAt":"2024-02-12T22:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> There is a larger clean-up opportunity to drop the need for making a\n>> copy, which probably is worth doing, so I folded the above into this\n>> version.\n>\n> Ooh, that's nice. This version looks good, I found the code comments\n> very helpful\n\nThanks.\n\nJudging from https://github.com/git/git/actions/runs/7878254534/job/21496314393#step:5:142\nI do not seen to have broken Windows with this change, so let's\nfast-track and merge it down to 'master' before -rc1.\n\n> Best Wishes\n>\n> Phillip\n>\n>> ------- >8 ------------- >8 ------------- >8 ------------- >8 -------\n>> There are compilers other than Visual C that want to show absolute\n>> paths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\n>> show relative file paths, 2023-09-25) so that it can also work with\n>> a path that uses slash as the directory separator, and becomes\n>> almost no-op once one-time preparation finds out that we are using a\n>> compiler that already gives relative paths.  Incidentally, this also\n>> should do the right thing on Windows with a compiler that shows\n>> relative paths but with backslash as the directory separator (if\n>> such a thing exists and is used to build git).\n>> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>\n>> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>> ---\n>>    * I found that the diff relative to the result of applying v1 was\n>>      easier to follow than the range-diff, so here it is.\n>>    diff --git c/t/unit-tests/test-lib.c w/t/unit-tests/test-lib.c\n>>    index 83c9eb8c59..66d6980ffb 100644\n>>    --- c/t/unit-tests/test-lib.c\n>>    +++ w/t/unit-tests/test-lib.c\n>>    @@ -64,34 +64,33 @@ static const char *make_relative(const char *location)\n>>     \t\t * prefix_len == 0 if the compiler gives paths relative\n>>     \t\t * to the root of the working tree.  Otherwise, we want\n>>     \t\t * to see that we did find the needle[] at a directory\n>>    -\t\t * boundary.\n>>    +\t\t * boundary.  Again we rely on that needle[] begins with\n>>    +\t\t * \"t\" followed by the directory separator.\n>>     \t\t */\n>>     \t\tif (fspathcmp(needle, prefix + prefix_len) ||\n>>    -\t\t    (prefix_len &&\n>>    -\t\t     prefix[prefix_len - 1] != '/' &&\n>>    -\t\t     prefix[prefix_len - 1] != '\\\\'))\n>>    +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n>>     \t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>>    -\n>>     \t}\n>>         \t/*\n>>    -\t * If our compiler gives relative paths and we do not need\n>>    -\t * to munge directory separator, we can return location as-is.\n>>    +\t * Does it not start with the expected prefix?\n>>    +\t * Return it as-is without making it worse.\n>>     \t */\n>>    -\tif (!prefix_len && !need_bs_to_fs)\n>>    +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>>     \t\treturn location;\n>>        -\t/* Does it not start with the expected prefix? */\n>>    -\tif (fspathncmp(location, prefix, prefix_len))\n>>    -\t\treturn location;\n>>    +\t/*\n>>    +\t * If we do not need to munge directory separator, we can return\n>>    +\t * the substring at the tail of the location.\n>>    +\t */\n>>    +\tif (!need_bs_to_fs)\n>>    +\t\treturn location + prefix_len;\n>>        -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>     \t/* convert backslashes to forward slashes */\n>>    -\tif (need_bs_to_fs) {\n>>    -\t\tfor (p = buf; *p; p++)\n>>    -\t\t\tif (*p == '\\\\')\n>>    -\t\t\t\t*p = '/';\n>>    -\t}\n>>    +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>    +\tfor (p = buf; *p; p++)\n>>    +\t\tif (*p == '\\\\')\n>>    +\t\t\t*p = '/';\n>>     \treturn buf;\n>>     }\n>>       t/unit-tests/test-lib.c | 61\n>> +++++++++++++++++++++++++++++++----------\n>>   1 file changed, 47 insertions(+), 14 deletions(-)\n>> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\n>> index 7bf9dfdb95..66d6980ffb 100644\n>> --- a/t/unit-tests/test-lib.c\n>> +++ b/t/unit-tests/test-lib.c\n>> @@ -21,12 +21,11 @@ static struct {\n>>   \t.result = RESULT_NONE,\n>>   };\n>>   -#ifndef _MSC_VER\n>> -#define make_relative(location) location\n>> -#else\n>>   /*\n>>    * Visual C interpolates the absolute Windows path for `__FILE__`,\n>>    * but we want to see relative paths, as verified by t0080.\n>> + * There are other compilers that do the same, and are not for\n>> + * Windows.\n>>    */\n>>   #include \"dir.h\"\n>>   @@ -34,32 +33,66 @@ static const char *make_relative(const char\n>> *location)\n>>   {\n>>   \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n>>   \tstatic size_t prefix_len;\n>> +\tstatic int need_bs_to_fs = -1;\n>>   -\tif (!prefix_len) {\n>> +\t/* one-time preparation */\n>> +\tif (need_bs_to_fs < 0) {\n>>   \t\tsize_t len = strlen(prefix);\n>> -\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n>> +\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n>>   \t\tsize_t needle_len = strlen(needle);\n>>   -\t\tif (len < needle_len || strcmp(needle, prefix + len -\n>> needle_len))\n>> -\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>> +\t\tif (len < needle_len)\n>> +\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n>> +\n>> +\t\t/*\n>> +\t\t * The path could be relative (t/unit-tests/test-lib.c)\n>> +\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n>> +\t\t * Check the slash between \"t\" and \"unit-tests\".\n>> +\t\t */\n>> +\t\tprefix_len = len - needle_len;\n>> +\t\tif (prefix[prefix_len + 1] == '/') {\n>> +\t\t\t/* Oh, we're not Windows */\n>> +\t\t\tfor (size_t i = 0; i < needle_len; i++)\n>> +\t\t\t\tif (needle[i] == '\\\\')\n>> +\t\t\t\t\tneedle[i] = '/';\n>> +\t\t\tneed_bs_to_fs = 0;\n>> +\t\t} else {\n>> +\t\t\tneed_bs_to_fs = 1;\n>> +\t\t}\n>>   -\t\t/* let it end in a directory separator */\n>> -\t\tprefix_len = len - needle_len + 1;\n>> +\t\t/*\n>> +\t\t * prefix_len == 0 if the compiler gives paths relative\n>> +\t\t * to the root of the working tree.  Otherwise, we want\n>> +\t\t * to see that we did find the needle[] at a directory\n>> +\t\t * boundary.  Again we rely on that needle[] begins with\n>> +\t\t * \"t\" followed by the directory separator.\n>> +\t\t */\n>> +\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n>> +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n>> +\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>>   \t}\n>>   -\t/* Does it not start with the expected prefix? */\n>> -\tif (fspathncmp(location, prefix, prefix_len))\n>> +\t/*\n>> +\t * Does it not start with the expected prefix?\n>> +\t * Return it as-is without making it worse.\n>> +\t */\n>> +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>>   \t\treturn location;\n>>   -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>> +\t/*\n>> +\t * If we do not need to munge directory separator, we can return\n>> +\t * the substring at the tail of the location.\n>> +\t */\n>> +\tif (!need_bs_to_fs)\n>> +\t\treturn location + prefix_len;\n>> +\n>>   \t/* convert backslashes to forward slashes */\n>> +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>   \tfor (p = buf; *p; p++)\n>>   \t\tif (*p == '\\\\')\n>>   \t\t\t*p = '/';\n>> -\n>>   \treturn buf;\n>>   }\n>> -#endif\n>>     static void msg_with_prefix(const char *prefix, const char\n>> *format, va_list ap)\n>>   {\n"},{"id":"488525","messageId":"fd8874bb-8e45-4b39-986c-5a96ccf0747f@gmail.com","threadId":"60898","inReplyTo":"xmqqsf1x486b.fsf@gitster.g","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-13T10:55:25Z","receivedAt":"2024-02-13T10:55:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 12/02/2024 22:41, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n>>> There is a larger clean-up opportunity to drop the need for making a\n>>> copy, which probably is worth doing, so I folded the above into this\n>>> version.\n>>\n>> Ooh, that's nice. This version looks good, I found the code comments\n>> very helpful\n> \n> Thanks.\n> \n> Judging from https://github.com/git/git/actions/runs/7878254534/job/21496314393#step:5:142\n> I do not seen to have broken Windows with this change, so let's\n> fast-track and merge it down to 'master' before -rc1.\n\nI think it was only the MSVC that needed the paths munging which we \ndon't test by default. I have tweaked our github actions to run those \ntests and they pass \nhttps://github.com/phillipwood/git/actions/runs/7885144920/job/21515922057#step:5:146\n\nBest Wishes\n\nPhillip\n\n\n>> Best Wishes\n>>\n>> Phillip\n>>\n>>> ------- >8 ------------- >8 ------------- >8 ------------- >8 -------\n>>> There are compilers other than Visual C that want to show absolute\n>>> paths.  Generalize the helper introduced by a2c5e294 (unit-tests: do\n>>> show relative file paths, 2023-09-25) so that it can also work with\n>>> a path that uses slash as the directory separator, and becomes\n>>> almost no-op once one-time preparation finds out that we are using a\n>>> compiler that already gives relative paths.  Incidentally, this also\n>>> should do the right thing on Windows with a compiler that shows\n>>> relative paths but with backslash as the directory separator (if\n>>> such a thing exists and is used to build git).\n>>> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>\n>>> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>>> ---\n>>>     * I found that the diff relative to the result of applying v1 was\n>>>       easier to follow than the range-diff, so here it is.\n>>>     diff --git c/t/unit-tests/test-lib.c w/t/unit-tests/test-lib.c\n>>>     index 83c9eb8c59..66d6980ffb 100644\n>>>     --- c/t/unit-tests/test-lib.c\n>>>     +++ w/t/unit-tests/test-lib.c\n>>>     @@ -64,34 +64,33 @@ static const char *make_relative(const char *location)\n>>>      \t\t * prefix_len == 0 if the compiler gives paths relative\n>>>      \t\t * to the root of the working tree.  Otherwise, we want\n>>>      \t\t * to see that we did find the needle[] at a directory\n>>>     -\t\t * boundary.\n>>>     +\t\t * boundary.  Again we rely on that needle[] begins with\n>>>     +\t\t * \"t\" followed by the directory separator.\n>>>      \t\t */\n>>>      \t\tif (fspathcmp(needle, prefix + prefix_len) ||\n>>>     -\t\t    (prefix_len &&\n>>>     -\t\t     prefix[prefix_len - 1] != '/' &&\n>>>     -\t\t     prefix[prefix_len - 1] != '\\\\'))\n>>>     +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n>>>      \t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>>>     -\n>>>      \t}\n>>>          \t/*\n>>>     -\t * If our compiler gives relative paths and we do not need\n>>>     -\t * to munge directory separator, we can return location as-is.\n>>>     +\t * Does it not start with the expected prefix?\n>>>     +\t * Return it as-is without making it worse.\n>>>      \t */\n>>>     -\tif (!prefix_len && !need_bs_to_fs)\n>>>     +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>>>      \t\treturn location;\n>>>         -\t/* Does it not start with the expected prefix? */\n>>>     -\tif (fspathncmp(location, prefix, prefix_len))\n>>>     -\t\treturn location;\n>>>     +\t/*\n>>>     +\t * If we do not need to munge directory separator, we can return\n>>>     +\t * the substring at the tail of the location.\n>>>     +\t */\n>>>     +\tif (!need_bs_to_fs)\n>>>     +\t\treturn location + prefix_len;\n>>>         -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>>      \t/* convert backslashes to forward slashes */\n>>>     -\tif (need_bs_to_fs) {\n>>>     -\t\tfor (p = buf; *p; p++)\n>>>     -\t\t\tif (*p == '\\\\')\n>>>     -\t\t\t\t*p = '/';\n>>>     -\t}\n>>>     +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>>     +\tfor (p = buf; *p; p++)\n>>>     +\t\tif (*p == '\\\\')\n>>>     +\t\t\t*p = '/';\n>>>      \treturn buf;\n>>>      }\n>>>        t/unit-tests/test-lib.c | 61\n>>> +++++++++++++++++++++++++++++++----------\n>>>    1 file changed, 47 insertions(+), 14 deletions(-)\n>>> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\n>>> index 7bf9dfdb95..66d6980ffb 100644\n>>> --- a/t/unit-tests/test-lib.c\n>>> +++ b/t/unit-tests/test-lib.c\n>>> @@ -21,12 +21,11 @@ static struct {\n>>>    \t.result = RESULT_NONE,\n>>>    };\n>>>    -#ifndef _MSC_VER\n>>> -#define make_relative(location) location\n>>> -#else\n>>>    /*\n>>>     * Visual C interpolates the absolute Windows path for `__FILE__`,\n>>>     * but we want to see relative paths, as verified by t0080.\n>>> + * There are other compilers that do the same, and are not for\n>>> + * Windows.\n>>>     */\n>>>    #include \"dir.h\"\n>>>    @@ -34,32 +33,66 @@ static const char *make_relative(const char\n>>> *location)\n>>>    {\n>>>    \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n>>>    \tstatic size_t prefix_len;\n>>> +\tstatic int need_bs_to_fs = -1;\n>>>    -\tif (!prefix_len) {\n>>> +\t/* one-time preparation */\n>>> +\tif (need_bs_to_fs < 0) {\n>>>    \t\tsize_t len = strlen(prefix);\n>>> -\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n>>> +\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n>>>    \t\tsize_t needle_len = strlen(needle);\n>>>    -\t\tif (len < needle_len || strcmp(needle, prefix + len -\n>>> needle_len))\n>>> -\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>>> +\t\tif (len < needle_len)\n>>> +\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n>>> +\n>>> +\t\t/*\n>>> +\t\t * The path could be relative (t/unit-tests/test-lib.c)\n>>> +\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n>>> +\t\t * Check the slash between \"t\" and \"unit-tests\".\n>>> +\t\t */\n>>> +\t\tprefix_len = len - needle_len;\n>>> +\t\tif (prefix[prefix_len + 1] == '/') {\n>>> +\t\t\t/* Oh, we're not Windows */\n>>> +\t\t\tfor (size_t i = 0; i < needle_len; i++)\n>>> +\t\t\t\tif (needle[i] == '\\\\')\n>>> +\t\t\t\t\tneedle[i] = '/';\n>>> +\t\t\tneed_bs_to_fs = 0;\n>>> +\t\t} else {\n>>> +\t\t\tneed_bs_to_fs = 1;\n>>> +\t\t}\n>>>    -\t\t/* let it end in a directory separator */\n>>> -\t\tprefix_len = len - needle_len + 1;\n>>> +\t\t/*\n>>> +\t\t * prefix_len == 0 if the compiler gives paths relative\n>>> +\t\t * to the root of the working tree.  Otherwise, we want\n>>> +\t\t * to see that we did find the needle[] at a directory\n>>> +\t\t * boundary.  Again we rely on that needle[] begins with\n>>> +\t\t * \"t\" followed by the directory separator.\n>>> +\t\t */\n>>> +\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n>>> +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n>>> +\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>>>    \t}\n>>>    -\t/* Does it not start with the expected prefix? */\n>>> -\tif (fspathncmp(location, prefix, prefix_len))\n>>> +\t/*\n>>> +\t * Does it not start with the expected prefix?\n>>> +\t * Return it as-is without making it worse.\n>>> +\t */\n>>> +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>>>    \t\treturn location;\n>>>    -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>> +\t/*\n>>> +\t * If we do not need to munge directory separator, we can return\n>>> +\t * the substring at the tail of the location.\n>>> +\t */\n>>> +\tif (!need_bs_to_fs)\n>>> +\t\treturn location + prefix_len;\n>>> +\n>>>    \t/* convert backslashes to forward slashes */\n>>> +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>>>    \tfor (p = buf; *p; p++)\n>>>    \t\tif (*p == '\\\\')\n>>>    \t\t\t*p = '/';\n>>> -\n>>>    \treturn buf;\n>>>    }\n>>> -#endif\n>>>      static void msg_with_prefix(const char *prefix, const char\n>>> *format, va_list ap)\n>>>    {\n\n"},{"id":"488542","messageId":"xmqq4jec8f3w.fsf@gitster.g","threadId":"60898","inReplyTo":"fd8874bb-8e45-4b39-986c-5a96ccf0747f@gmail.com","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-13T17:10:27Z","receivedAt":"2024-02-13T17:10:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> I think it was only the MSVC that needed the paths munging which we\n> don't test by default.\n\nAh, my sillies.\n\n> I have tweaked our github actions to run those\n> tests and they pass\n> https://github.com/phillipwood/git/actions/runs/7885144920/job/21515922057#step:5:146\n\nThanks.\n"},{"id":"488572","messageId":"0fb4e912-b25e-0240-c49f-aac5cbc3ef9e@gmx.de","threadId":"60898","inReplyTo":"xmqqle7r9enn.fsf_-_@gitster.g","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-02-13T19:58:53Z","receivedAt":"2024-02-13T19:59:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 11 Feb 2024, Junio C Hamano wrote:\n\n> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\n> index 7bf9dfdb95..66d6980ffb 100644\n> --- a/t/unit-tests/test-lib.c\n> +++ b/t/unit-tests/test-lib.c\n> @@ -21,12 +21,11 @@ static struct {\n>  \t.result = RESULT_NONE,\n>  };\n>\n> -#ifndef _MSC_VER\n> -#define make_relative(location) location\n> -#else\n>  /*\n>   * Visual C interpolates the absolute Windows path for `__FILE__`,\n>   * but we want to see relative paths, as verified by t0080.\n> + * There are other compilers that do the same, and are not for\n> + * Windows.\n>   */\n>  #include \"dir.h\"\n>\n> @@ -34,32 +33,66 @@ static const char *make_relative(const char *location)\n>  {\n>  \tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n>  \tstatic size_t prefix_len;\n> +\tstatic int need_bs_to_fs = -1;\n>\n> -\tif (!prefix_len) {\n> +\t/* one-time preparation */\n> +\tif (need_bs_to_fs < 0) {\n>  \t\tsize_t len = strlen(prefix);\n> -\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n> +\t\tchar needle[] = \"t\\\\unit-tests\\\\test-lib.c\";\n>  \t\tsize_t needle_len = strlen(needle);\n>\n> -\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n> -\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n> +\t\tif (len < needle_len)\n> +\t\t\tdie(\"unexpected prefix '%s'\", prefix);\n> +\n> +\t\t/*\n> +\t\t * The path could be relative (t/unit-tests/test-lib.c)\n> +\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n> +\t\t * Check the slash between \"t\" and \"unit-tests\".\n> +\t\t */\n> +\t\tprefix_len = len - needle_len;\n> +\t\tif (prefix[prefix_len + 1] == '/') {\n> +\t\t\t/* Oh, we're not Windows */\n> +\t\t\tfor (size_t i = 0; i < needle_len; i++)\n> +\t\t\t\tif (needle[i] == '\\\\')\n> +\t\t\t\t\tneedle[i] = '/';\n\nThis looks very similar to the `convert_slashes()` function that is\ndefined in `compat/mingw.h`. It might be a good opportunity to rename that\nfunction and move it to `git-compat-util.h`, then use it here and once\nmore below at the end of `make_relative()`.\n\nThat's just a nit, though. The patch looks good to me.\n\nThanks,\nJohannes\n\n> +\t\t\tneed_bs_to_fs = 0;\n> +\t\t} else {\n> +\t\t\tneed_bs_to_fs = 1;\n> +\t\t}\n>\n> -\t\t/* let it end in a directory separator */\n> -\t\tprefix_len = len - needle_len + 1;\n> +\t\t/*\n> +\t\t * prefix_len == 0 if the compiler gives paths relative\n> +\t\t * to the root of the working tree.  Otherwise, we want\n> +\t\t * to see that we did find the needle[] at a directory\n> +\t\t * boundary.  Again we rely on that needle[] begins with\n> +\t\t * \"t\" followed by the directory separator.\n> +\t\t */\n> +\t\tif (fspathcmp(needle, prefix + prefix_len) ||\n> +\t\t    (prefix_len && prefix[prefix_len - 1] != needle[1]))\n> +\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n>  \t}\n>\n> -\t/* Does it not start with the expected prefix? */\n> -\tif (fspathncmp(location, prefix, prefix_len))\n> +\t/*\n> +\t * Does it not start with the expected prefix?\n> +\t * Return it as-is without making it worse.\n> +\t */\n> +\tif (prefix_len && fspathncmp(location, prefix, prefix_len))\n>  \t\treturn location;\n>\n> -\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n> +\t/*\n> +\t * If we do not need to munge directory separator, we can return\n> +\t * the substring at the tail of the location.\n> +\t */\n> +\tif (!need_bs_to_fs)\n> +\t\treturn location + prefix_len;\n> +\n>  \t/* convert backslashes to forward slashes */\n> +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n>  \tfor (p = buf; *p; p++)\n>  \t\tif (*p == '\\\\')\n>  \t\t\t*p = '/';\n> -\n>  \treturn buf;\n>  }\n> -#endif\n>\n>  static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n>  {\n> --\n> 2.44.0-rc0\n>\n>\n>\n"},{"id":"488580","messageId":"xmqqzfw43xad.fsf@gitster.g","threadId":"60898","inReplyTo":"0fb4e912-b25e-0240-c49f-aac5cbc3ef9e@gmx.de","subject":"Re: [PATCH v2] unit-tests: do show relative file paths on non-Windows, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-13T20:48:58Z","receivedAt":"2024-02-13T20:49:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> +\t\t/*\n>> +\t\t * The path could be relative (t/unit-tests/test-lib.c)\n>> +\t\t * or full (/home/user/git/t/unit-tests/test-lib.c).\n>> +\t\t * Check the slash between \"t\" and \"unit-tests\".\n>> +\t\t */\n>> +\t\tprefix_len = len - needle_len;\n>> +\t\tif (prefix[prefix_len + 1] == '/') {\n>> +\t\t\t/* Oh, we're not Windows */\n>> +\t\t\tfor (size_t i = 0; i < needle_len; i++)\n>> +\t\t\t\tif (needle[i] == '\\\\')\n>> +\t\t\t\t\tneedle[i] = '/';\n>\n> This looks very similar to the `convert_slashes()` function that is\n> defined in `compat/mingw.h`.\n\nI lifted it from your later loop in the function, but given that\nmany of the things that needed on Windows came from you, it is not\nsurprising if you have another copy there ;-)\n\n> It might be a good opportunity to rename that\n> function and move it to `git-compat-util.h`, then use it here and once\n> more below at the end of `make_relative()`.\n\nRight.  But not as part of the -rc fix.  Let's leave it as\n#leftoverbits.\n\nThanks.\n"}]}