threads / bug / 60895

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

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

## tl;dr

9 messages between Feb 10, 2024 and Feb 12, 2024.

replies: 8people: 2as markdown or json

rsbecker@nexbridge.com· Feb 10, 2024, 18:14 UTC · lore

I encountered a new problem on git 2.44.0-rc0 for test t0080.1. Run very verbose (--verbose -x):

++ cat ++ /home/randall/git/t/unit-tests/bin/t-basic ++ test_cmp expect actual ++ test 2 -ne 2 ++ eval 'diff -u' '"$@"' +++ diff -u expect actual --- expect 2024-02-10 18:04:28 +0000 +++ actual 2024-02-10 18:04:28 +0000

@@ -1,43 +1,43 @@
 ok 1 - passing test
 ok 2 - passing test and assertion return 1
-# check "1 == 2" failed at t/unit-tests/t-basic.c:76
+# check "1 == 2" failed at /home/randall/git/t/unit-tests/t-basic.c:76
 #    left: 1
 #   right: 2
 not ok 3 - failing test
 ok 4 - failing test and assertion return 0
 not ok 5 - passing TEST_TODO() # TODO
 ok 6 - passing TEST_TODO() returns 1
-# todo check 'check(x)' succeeded at t/unit-tests/t-basic.c:25
+# todo check 'check(x)' succeeded at
/home/randall/git/t/unit-tests/t-basic.c:25
 not ok 7 - failing TEST_TODO()
 ok 8 - failing TEST_TODO() returns 0
-# check "0" failed at t/unit-tests/t-basic.c:30
+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:30
 # skipping test - missing prerequisite
-# skipping check '1' at t/unit-tests/t-basic.c:32
+# skipping check '1' at /home/randall/git/t/unit-tests/t-basic.c:32
 ok 9 - test_skip() # SKIP
 ok 10 - skipped test returns 1
 # skipping test - missing prerequisite
 ok 11 - test_skip() inside TEST_TODO() # SKIP
 ok 12 - test_skip() inside TEST_TODO() returns 1
-# check "0" failed at t/unit-tests/t-basic.c:48
+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:48
 not ok 13 - TEST_TODO() after failing check
 ok 14 - TEST_TODO() after failing check returns 0
-# check "0" failed at t/unit-tests/t-basic.c:56
+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:56
 not ok 15 - failing check after TEST_TODO()
 ok 16 - failing check after TEST_TODO() returns 0
-# check "!strcmp("\thello\\", "there\"\n")" failed at
t/unit-tests/t-basic.c:61
+# check "!strcmp("\thello\\", "there\"\n")" failed at
/home/randall/git/t/unit-tests/t-basic.c:61
 #    left: "\011hello\\"
 #   right: "there\"\012"
-# check "!strcmp("NULL", NULL)" failed at t/unit-tests/t-basic.c:62
+# check "!strcmp("NULL", NULL)" failed at
/home/randall/git/t/unit-tests/t-basic.c:62
 #    left: "NULL"
 #   right: NULL
-# check "'a' == '\n'" failed at t/unit-tests/t-basic.c:63
+# check "'a' == '\n'" failed at /home/randall/git/t/unit-tests/t-basic.c:63
 #    left: 'a'
 #   right: '\012'
-# check "'\\' == '\''" failed at t/unit-tests/t-basic.c:64
+# check "'\\' == '\''" failed at
/home/randall/git/t/unit-tests/t-basic.c:64
 #    left: '\\'
 #   right: '\''
 not ok 17 - messages from failing string and char comparison
-# BUG: test has no checks at t/unit-tests/t-basic.c:91
+# BUG: test has no checks at /home/randall/git/t/unit-tests/t-basic.c:91
 not ok 18 - test with no checks
 ok 19 - test with no checks returns 0
 1..19
error: last command exited with $?=1

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. It might be useful to pipe to sed to strip ${HOME}/ when building
the actual file, something like:

sed -i "1,\$s/${HOME}\//g" actual    # Not that that will actually work
because sed will process /. A different delimiter would work.

Randall

--
Randall S. Becker
NSGit Chief Architect
Nexbridge Inc.
+1.416.984.9826
rsbecker@nexbridge.com· Feb 10, 2024, 23:22 UTC · re: rsbecker@nexbridge.com · lore

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

On Saturday, February 10, 2024 1:14 PM, I wrote:
>I encountered a new problem on git 2.44.0-rc0 for test t0080.1. Run very
verbose
Show 13 quoted lines
>(--verbose -x):
>
>++ cat
>++ /home/randall/git/t/unit-tests/bin/t-basic
>++ test_cmp expect actual
>++ test 2 -ne 2
>++ eval 'diff -u' '"$@"'
>+++ diff -u expect actual
>--- expect      2024-02-10 18:04:28 +0000
>+++ actual      2024-02-10 18:04:28 +0000
>@@ -1,43 +1,43 @@
> ok 1 - passing test
> ok 2 - passing test and assertion return 1 -# check "1 == 2" failed at
t/unit-tests/t-
Show 6 quoted lines
>basic.c:76
>+# check "1 == 2" failed at /home/randall/git/t/unit-tests/t-basic.c:76
> #    left: 1
> #   right: 2
> not ok 3 - failing test
> ok 4 - failing test and assertion return 0  not ok 5 - passing TEST_TODO()
# TODO
>ok 6 - passing TEST_TODO() returns 1 -# todo check 'check(x)' succeeded at
t/unit-
Show 17 quoted lines
>tests/t-basic.c:25
>+# todo check 'check(x)' succeeded at
>/home/randall/git/t/unit-tests/t-basic.c:25
> not ok 7 - failing TEST_TODO()
> ok 8 - failing TEST_TODO() returns 0
>-# check "0" failed at t/unit-tests/t-basic.c:30
>+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:30
> # skipping test - missing prerequisite
>-# skipping check '1' at t/unit-tests/t-basic.c:32
>+# skipping check '1' at /home/randall/git/t/unit-tests/t-basic.c:32
> ok 9 - test_skip() # SKIP
> ok 10 - skipped test returns 1
> # skipping test - missing prerequisite
> ok 11 - test_skip() inside TEST_TODO() # SKIP  ok 12 - test_skip() inside
>TEST_TODO() returns 1 -# check "0" failed at t/unit-tests/t-basic.c:48
>+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:48
> not ok 13 - TEST_TODO() after failing check  ok 14 - TEST_TODO() after
failing check
>returns 0 -# check "0" failed at t/unit-tests/t-basic.c:56
>+# check "0" failed at /home/randall/git/t/unit-tests/t-basic.c:56
> not ok 15 - failing check after TEST_TODO()  ok 16 - failing check after
TEST_TODO()
Show 22 quoted lines
>returns 0 -# check "!strcmp("\thello\\", "there\"\n")" failed at
>t/unit-tests/t-basic.c:61
>+# check "!strcmp("\thello\\", "there\"\n")" failed at
>/home/randall/git/t/unit-tests/t-basic.c:61
> #    left: "\011hello\\"
> #   right: "there\"\012"
>-# check "!strcmp("NULL", NULL)" failed at t/unit-tests/t-basic.c:62
>+# check "!strcmp("NULL", NULL)" failed at
>/home/randall/git/t/unit-tests/t-basic.c:62
> #    left: "NULL"
> #   right: NULL
>-# check "'a' == '\n'" failed at t/unit-tests/t-basic.c:63
>+# check "'a' == '\n'" failed at
>+/home/randall/git/t/unit-tests/t-basic.c:63
> #    left: 'a'
> #   right: '\012'
>-# check "'\\' == '\''" failed at t/unit-tests/t-basic.c:64
>+# check "'\\' == '\''" failed at
>/home/randall/git/t/unit-tests/t-basic.c:64
> #    left: '\\'
> #   right: '\''
> not ok 17 - messages from failing string and char comparison -# BUG: test
has no
Show 9 quoted lines
>checks at t/unit-tests/t-basic.c:91
>+# BUG: test has no checks at
>+/home/randall/git/t/unit-tests/t-basic.c:91
> not ok 18 - test with no checks
> ok 19 - test with no checks returns 0
> 1..19
>error: last command exited with $?=1
>
>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. It might be useful to pipe to sed
to strip
Show 6 quoted lines
>${HOME}/ when building the actual file, something like:
>
>sed -i "1,\$s/${HOME}\//g" actual    # Not that that will actually work
>because sed will process /. A different delimiter would work.
>
>Randall
Putting the sed command in, as follows:
diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh
index 6657c114a3..eaf25f2ddc 100755
--- a/t/t0080-unit-test-output.sh
+++ b/t/t0080-unit-test-output.sh
@@ -52,7 +52,7 @@ test_expect_success 'TAP output from unit tests' '
        1..19
        EOF

-       ! "$GIT_BUILD_DIR"/t/unit-tests/bin/t-basic >actual &&
+       ! "$GIT_BUILD_DIR"/t/unit-tests/bin/t-basic | sed "s?/.*/t/?t/?"
>actual &&
        test_cmp expect actual
 '

still results in a problem. The ! in the above line is being interpreted
incorrectly:

diff expect actual
27c27
< # check "!strcmp("\thello\\", "there"\n")" failed at
t/unit-tests/t-basic.c:61
---
> # check "!strcmp("\thello\\", "there\"\n")" failed at
t/unit-tests/t-basic.c:61
29c29
< #   right: "there"\012"
---
> #   right: "there\"\012"

I am not convinced the test is working properly. The ! appears to be causing
/bin/ksh to be invoked instead of bash. If I remove !, the test works
correctly. Did we not have issues with using ! in the past?

--Randall

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.
Junio C Hamano· Feb 11, 2024, 02:05 UTC · re: rsbecker@nexbridge.com · lore

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

<rsbecker@nexbridge.com> writes:
Show 5 quoted lines
> 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, location + 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)
 {
rsbecker@nexbridge.com· Feb 11, 2024, 02:47 UTC · re: Junio C Hamano · lore

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

On Saturday, February 10, 2024 9:06 PM, Junio C Hamano wrote:
Show 9 quoted lines
><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
+++++++++++++++++++++++++++++++++++++---------
Show 36 quoted lines
>---
> 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))
Show 62 quoted lines
>+		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, location + 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.
--Randall
rsbecker@nexbridge.com· Feb 12, 2024, 16:05 UTC · re: rsbecker@nexbridge.com · lore

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

On Saturday, February 10, 2024 9:47 PM, Junio C Hamano wrote:
Show 123 quoted lines
>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.
Junio C Hamano· Feb 12, 2024, 16:43 UTC · re: rsbecker@nexbridge.com · lore

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

<rsbecker@nexbridge.com> writes:
>>This looks like a good plan.
>
> This might be trivial, but I cannot tell. The #ifndef should be changed as
> follows:
https://lore.kernel.org/git/xmqqttmf9y46.fsf@gitster.g/
rsbecker@nexbridge.com· Feb 12, 2024, 17:59 UTC · re: Junio C Hamano · lore

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

On Monday, February 12, 2024 11:43 AM, Junio C Hamano wrote:
Show 5 quoted lines
><rsbecker@nexbridge.com> writes:
>
>>>This looks like a good plan.
>>
>> This might be trivial, but I cannot tell. The #ifndef should be changed
as
>> follows:
>
>https://lore.kernel.org/git/xmqqttmf9y46.fsf@gitster.g/

I applied this fix but there is no improvement in the result from the last report. actual just has two lines. expect looks reasonable. I still had to modify the #ifndef.

I have tried cherry-picking the change (no effect), building on master, next... am lost.

Junio C Hamano· Feb 12, 2024, 19:02 UTC · re: rsbecker@nexbridge.com · lore

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

<rsbecker@nexbridge.com> writes:
Show 17 quoted lines
> On Monday, February 12, 2024 11:43 AM, Junio C Hamano wrote:
>><rsbecker@nexbridge.com> writes:
>>
>>>>This looks like a good plan.
>>>
>>> This might be trivial, but I cannot tell. The #ifndef should be changed
> as
>>> follows:
>>
>>https://lore.kernel.org/git/xmqqttmf9y46.fsf@gitster.g/
>
> I applied this fix but there is no improvement in the result from the last
> report. actual just has two lines. expect looks reasonable. I still had to
> modify the #ifndef.
>
> I have tried cherry-picking the change (no effect), building on master,
> next... am lost.

We seem to be looking at something totally different. The later patch in question (not the "looks like a good plan" outline) does not have any #ifndef and applies the make_relative() logic everywhere.

I would suspect that cherry-picking f6628636 (unit-tests: do show relative file paths on non-Windows, too, 2024-02-11) would be the simplest.

--- >8 ---
Subject: [PATCH] unit-tests: do show relative file paths on non-Windows, too

There are compilers other than Visual C that want to show absolute paths. Generalize the helper introduced by a2c5e294 (unit-tests: do show relative file paths, 2023-09-25) so that it can also work with a path that uses slash as the directory separator, and becomes almost no-op once one-time preparation finds out that we are using a compiler that already gives relative paths. Incidentally, this also should do the right thing on Windows with a compiler that shows relative paths but with backslash as the directory separator (if such a thing exists and is used to build git).

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/unit-tests/test-lib.c | 61 +++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c
index 7bf9dfdb95..66d6980ffb 100644
--- a/t/unit-tests/test-lib.c
+++ b/t/unit-tests/test-lib.c
@@ -21,12 +21,11 @@ static struct {
 	.result = RESULT_NONE,
 };
 
-#ifndef _MSC_VER
-#define make_relative(location) location
-#else
 /*
  * 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"
 
@@ -34,32 +33,66 @@ 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))
-			die("unexpected suffix of '%s'", prefix);
+		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;
+		}
 
-		/* let it end in a directory separator */
-		prefix_len = len - needle_len + 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.  Again we rely on that needle[] begins with
+		 * "t" followed by the directory separator.
+		 */
+		if (fspathcmp(needle, prefix + prefix_len) ||
+		    (prefix_len && prefix[prefix_len - 1] != needle[1]))
+			die("unexpected suffix of '%s'", prefix);
 	}
 
-	/* Does it not start with the expected prefix? */
-	if (fspathncmp(location, prefix, prefix_len))
+	/*
+	 * Does it not start with the expected prefix?
+	 * Return it as-is without making it worse.
+	 */
+	if (prefix_len && fspathncmp(location, prefix, prefix_len))
 		return location;
 
-	strlcpy(buf, location + prefix_len, sizeof(buf));
+	/*
+	 * If we do not need to munge directory separator, we can return
+	 * the substring at the tail of the location.
+	 */
+	if (!need_bs_to_fs)
+		return location + prefix_len;
+
 	/* convert backslashes to forward slashes */
+	strlcpy(buf, location + prefix_len, sizeof(buf));
 	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)
 {

-- 
2.44.0-rc0
rsbecker@nexbridge.com· Feb 12, 2024, 19:26 UTC · re: Junio C Hamano · lore

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

On Monday, February 12, 2024 2:02 PM, Junio C Hamano wrote:
Show 22 quoted lines
><rsbecker@nexbridge.com> writes:
>
>> On Monday, February 12, 2024 11:43 AM, Junio C Hamano wrote:
>>><rsbecker@nexbridge.com> writes:
>>>
>>>>>This looks like a good plan.
>>>>
>>>> This might be trivial, but I cannot tell. The #ifndef should be
>>>> changed
>> as
>>>> follows:
>>>
>>>https://lore.kernel.org/git/xmqqttmf9y46.fsf@gitster.g/
>>
>> I applied this fix but there is no improvement in the result from the
>> last report. actual just has two lines. expect looks reasonable. I
>> still had to modify the #ifndef.
>>
>> I have tried cherry-picking the change (no effect), building on
>> master, next... am lost.
>
>We seem to be looking at something totally different.  The later patch in
question
>(not the "looks like a good plan" outline) does not have any #ifndef and
applies the
>make_relative() logic everywhere.
>
>I would suspect that cherry-picking f6628636 (unit-tests: do show relative
file
>paths on non-Windows, too, 2024-02-11) would be the simplest.

Would you mind pushing that commit to github, please? It is not in the current history.

← back to recent threads