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

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

[rsbecker]

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

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

[rsbecker]

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
>(--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

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.

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

[Junio C Hamano]

<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, 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)
 {

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

[rsbecker]

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, 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

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

[rsbecker]

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.

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

[Junio C Hamano]

<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/

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

[rsbecker]

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.

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

[Junio C Hamano]

<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.

--- >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

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

[rsbecker]

On Monday, February 12, 2024 2:02 PM, Junio C Hamano wrote:
><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.