RE: [BUG] git 2.44.0-rc0 t0080.1 Breaks on NonStop x86 and ia64

[<rsbecker@nexbridge.com>]

On Saturday, February 10, 2024 9:47 PM, Junio C Hamano wrote:
>On Saturday, February 10, 2024 9:06 PM, Junio C Hamano wrote:
>><rsbecker@nexbridge.com> writes:
>>
>>> The diff appears to have failed because of an assumption of how paths
>>> are resolved during compilation. The assumption is that files remain
>>> partially qualified, which is not the case in all C compilers. This
>>> is c99. My experience with gcc is that it qualifies names differently
>>> than other compilers.
>>
>>I just found this bit of gem in t/unit-tests/test-lib.c.  I guess
>>Visual C
>falls into the
>>same category as yours, while GCC and Clang are from different camps?
>>
>>The easiest way forward may be to build on top of it, perhaps like so,
>>to
>generalize
>>the make_relative() to cover your case, too?
>>
>> t/unit-tests/test-lib.c | 63
>+++++++++++++++++++++++++++++++++++++---------
>>---
>> 1 file changed, 48 insertions(+), 15 deletions(-)
>>
>>diff --git c/t/unit-tests/test-lib.c w/t/unit-tests/test-lib.c index
>>7bf9dfdb95..69dbe1bbc7 100644
>>--- c/t/unit-tests/test-lib.c
>>+++ w/t/unit-tests/test-lib.c
>>@@ -21,45 +21,78 @@ static struct {
>> 	.result = RESULT_NONE,
>> };
>>
>>-#ifndef _MSC_VER
>>-#define make_relative(location) location -#else
>>+#include "dir.h"
>> /*
>>  * Visual C interpolates the absolute Windows path for `__FILE__`,
>>  * but we want to see relative paths, as verified by t0080.
>>+ * There are other compilers that do the same, and are not for
>>+ * Windows.
>>  */
>>-#include "dir.h"
>>-
>> static const char *make_relative(const char *location)  {
>> 	static char prefix[] = __FILE__, buf[PATH_MAX], *p;
>> 	static size_t prefix_len;
>>+	static int need_bs_to_fs = -1;
>>
>>-	if (!prefix_len) {
>>+	/* one-time preparation */
>>+	if (need_bs_to_fs < 0) {
>> 		size_t len = strlen(prefix);
>>-		const char *needle = "\\t\\unit-tests\\test-lib.c";
>>+		char needle[] = "t\\unit-tests\\test-lib.c";
>> 		size_t needle_len = strlen(needle);
>>
>>-		if (len < needle_len || strcmp(needle, prefix + len -
>needle_len))
>>+		if (len < needle_len)
>>+			die("unexpected prefix '%s'", prefix);
>>+
>>+		/*
>>+		 * The path could be relative (t/unit-tests/test-lib.c)
>>+		 * or full (/home/user/git/t/unit-tests/test-lib.c).
>>+		 * Check the slash between "t" and "unit-tests".
>>+		 */
>>+		prefix_len = len - needle_len;
>>+		if (prefix[prefix_len + 1] == '/') {
>>+			/* Oh, we're not Windows */
>>+			for (size_t i = 0; i < needle_len; i++)
>>+				if (needle[i] == '\\')
>>+					needle[i] = '/';
>>+			need_bs_to_fs = 0;
>>+		} else {
>>+			need_bs_to_fs = 1;
>>+		}
>>+
>>+		/*
>>+		 * prefix_len == 0 if the compiler gives paths relative
>>+		 * to the root of the working tree.  Otherwise, we want
>>+		 * to see that we did find the needle[] at a directory
>>+		 * boundary.
>>+		 */
>>+		if (fspathcmp(needle, prefix + prefix_len) ||
>>+		    (prefix_len &&
>>+		     prefix[prefix_len - 1] != '/' &&
>>+		     prefix[prefix_len - 1] != '\\'))
>> 			die("unexpected suffix of '%s'", prefix);
>>
>>-		/* let it end in a directory separator */
>>-		prefix_len = len - needle_len + 1;
>> 	}
>>
>>+	/*
>>+	 * If our compiler gives relative paths and we do not need
>>+	 * to munge directory separator, we can return location as-is.
>>+	 */
>>+	if (!prefix_len && !need_bs_to_fs)
>>+		return location;
>>+
>> 	/* Does it not start with the expected prefix? */
>> 	if (fspathncmp(location, prefix, prefix_len))
>> 		return location;
>>
>> 	strlcpy(buf, lnocation + prefix_len, sizeof(buf));
>> 	/* convert backslashes to forward slashes */
>>-	for (p = buf; *p; p++)
>>-		if (*p == '\\')
>>-			*p = '/';
>>-
>>+	if (need_bs_to_fs) {
>>+		for (p = buf; *p; p++)
>>+			if (*p == '\\')
>>+				*p = '/';
>>+	}
>> 	return buf;
>> }
>>-#endif
>>
>> static void msg_with_prefix(const char *prefix, const char *format,
>va_list ap)  {
>
>This looks like a good plan.

This might be trivial, but I cannot tell. The #ifndef should be changed as
follows:

diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c
index 7bf9dfdb95..2d1f6b7648 100644
--- a/t/unit-tests/test-lib.c
+++ b/t/unit-tests/test-lib.c
@@ -21,7 +21,7 @@ static struct {
        .result = RESULT_NONE,
 };

-#ifndef _MSC_VER
+#if !defined(_MSC_VER) && !defined(__TANDEM)
 #define make_relative(location) location
 #else
 /*

However, if this goes beyond Windows and NonStop, which I suspect it does,
we could add a separate knob in config.mak.uname to pull this in. However,
this does not correct the problem. actual ends up with only:

ok 1 - passing test
ok 2 - passing test and assertion return 1

which fails the test_cmp at the end of the test.