Horton

Re: Is this a bug in pg_current_logfile() on Windows?

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Thomas Kellerer <shammat@gmx.net>, pgsql-hackers@lists.postgresql.org
Date: 2020-07-08T23:29:57Z
Lists: pgsql-hackers, pgsql-general 
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 7/8/20 5:26 PM, Tom Lane wrote:
>> However ... I put in a test case to try to expose this failure, and
>> our Windows buildfarm critters remain perfectly happy.  So what's up
>> with that?  After some digging around, I believe the reason is that
>> PostgresNode::psql is stripping the \r from pg_current_logfile()'s
>> result, here:
>> 	$$stdout =~ s/\r//g if $TestLib::windows_os;
>> I'm slightly tempted to extend the test case by verifying on the
>> server side that the result ends in ".log" with no extra characters.
>> More generally, I wonder if the above behavior is really a good idea.
>> It seems to have been added in commit 33f3bbc6d as a hack to avoid
>> having to think too hard about mingw's behavior, but now I wonder if
>> it isn't masking other bugs too.  At the very least I think we ought
>> to tighten the coding to
>> 	$$stdout =~ s/\r\n/\n/g if $TestLib::windows_os;
>> so that it won't strip carriage returns at random.

> Seems reasonable. If we rip it out completely we'll have to find all the
> places it breaks and fix them. And we'll almost certainly get new
> breakage. If it's hiding a real bug we'll have to do that, but I'd be
> reluctant unless there's actual proof.

Hard to tell.  What I propose to do right now is change the \r filters
as shown above, and see if the test I added in 004_logrotate.pl starts
to show failures on Windows.  If it does, and no other place does,
I'm willing to be satisfied with that.  If we see *other* failures then
that'd prove that the problem is real, no?

			regards, tom lane

Commits

  1. Tighten up Windows CRLF conversion in our TAP test scripts.

  2. Fix pg_current_logfile() to not emit a carriage return on Windows.

  3. Further tighten Windows CRLF conversion in our TAP test scripts.

  4. Add test coverage for pg_current_logfile() function.

  5. Fix failures to ignore \r when reading Windows-style newlines.

  6. Fix TAP infrastructure to support Mingw better