Hello Tom,
27.10.2024 20:41, Tom Lane wrote:
I wrote:
In the no-good-deed-goes-unpunished department: buildfarm member
hamerkop doesn't like this patch [1]. The diffs look like
...
So what I'd like to do to fix this is to change
- if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL)
+ if ((file = AllocateFile(filename, "r")) == NULL)
Well, that didn't fix it :-(. I went so far as to extract the raw log
files from the buildfarm database, and what they show is that there is
absolutely no difference between the lines diff is claiming are
different:
-QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n
+QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n
It's the same both before and after 924e03917, which made the code
change depicted above, so that didn't help.
So I'm pretty baffled. I suppose the expected and result files must
actually be different, and something in subsequent processing is
losing the difference before it gets to the buildfarm database.
But I don't have the ability to debug that from here. Does anyone
with access to hamerkop want to poke into this?
Without additional information, the only thing I can think of that
I have any confidence will eliminate these failures is to reformat
the affected test cases so that they produce just a single line of
output. That's kind of annoying from a functionality-coverage point
of view, but I'm not sure avoiding it is worth moving mountains for.
I've managed to reproduce the issue with the core.autocrlf=true git setting
(which sets CR+LF line ending in test_ext7--2.0--2.1bad.sql) and with diff
from msys:
C:\msys64\usr\bin\diff.exe --version
diff (GNU diffutils) 3.8
(Gnu/Win32 Diff [1] doesn't detect those EOL differences and thus the test
doesn't fail.)
I can really see different line endings with hexdump:
hexdump -C ...testrun\test_extensions\regress\regression.diffs
00000230 20 20 20 20 20 20 20 20 5e 0a 2d 51 55 45 52 59 | ^.-QUERY|
00000240 3a 20 20 43 52 45 41 54 45 20 46 55 4e 43 54 49 |: CREATE FUNCTI|
00000250 4e 20 6d 79 5f 65 72 72 6f 6e 65 6f 75 73 5f 66 |N my_erroneous_f|
00000260 75 6e 63 28 69 6e 74 29 20 52 45 54 55 52 4e 53 |unc(int) RETURNS|
00000270 20 69 6e 74 20 4c 41 4e 47 55 41 47 45 20 53 51 | int LANGUAGE SQ|
00000280 4c 0a 2b 51 55 45 52 59 3a 20 20 43 52 45 41 54 |L.+QUERY: CREAT|
00000290 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 72 |E FUNCTIN my_err|
000002a0 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 29 |oneous_func(int)|
000002b0 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 4e | RETURNS int LAN|
000002c0 47 55 41 47 45 20 53 51 4c 0d 0a 20 41 53 20 24 |GUAGE SQL.. AS $|
hexdump -C .../testrun/test_extensions/regress/results/test_extensions.out |
grep -C5 FUNCTIN
00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA|
00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er|
00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int|
00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA|
00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0d 0a 41 53 20 |NGUAGE SQL...AS |
whilst
hexdump -C .../src/test/modules/test_extensions/expected/test_extensions.out |
grep -C5 FUNCTIN
00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA|
00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er|
00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int|
00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA|
00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0a 41 53 20 24 |NGUAGE SQL..AS $|
It looks like --strip-trailing-cr doesn't work as desired for this diff version.
I've also dumped buf in read_whole_file() and found that in both
PG_BINARY_R and "r" modes the 0d 0a ending is preserved. But it changed
to 0a with the "rt" mode (see [1]), and it makes the test (and the whole
`meson test`) pass for me.
[1] https://gnuwin32.sourceforge.net/packages/diffutils.htm
[2]
https://learn.microsoft.com/en-us/cpp/c-runtime-library/file-translation-constants?view=msvc-170
Best regards,
Alexander
