Re: ssl tests fail on windows / slurp_file() offset doesn't work on win

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Daniel Gustafsson <daniel@yesql.se>, Michael Paquier <michael.paquier@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>, Andrew Dunstan <andrew@dunslane.net>
Date: 2021-10-03T17:30:38Z
Lists: pgsql-hackers

Attachments

Hi,

On 2021-10-03 10:18:31 -0700, Andres Freund wrote:
> As you can see in the test output, every mismatch prints the whole file,
> despite only intending to show the tail. Which appears to be because the
> windows portion of 3c5b0685b921 doesn't actually work.  The reason for that in
> turn is that afaict the setFilePointer doesn't change the file position in a
> way that affects perl.
> 
> Consequently, if I force the !win32 path, the tests pass.
> 
> At first I assumed the cause of this is that while the setFilePointer() modifies the
> state of the underlying handle, it doesn't actually let perl know about
> that. Due to buffering etc perl likely has its own bookeeping about the
> position in the file. There's some pretty clear hints in
> https://perldoc.perl.org/functions/seek
> 
> But the problem turns out to be that it's bogus to pass $fh to
> setFilePointer(). That's a perl handle, not an win32 handle. Fixing that seems
> to make the tests pass.

It does (I only let it run to the ssl test, then pushed a newer revision):
https://cirrus-ci.com/task/5345293928497152?logs=ssl#L5


> Why did 3c5b0685b921 choose to use setFilePointer() in the first place? At
> this point it's a perl filehandle, so we should just use perl seek?
> 
> 
> Leaving the concrete breakage aside, I'm somewhat unhappy that there's not a
> single comment explaining why TestLib.pm is trying to use native windows
> APIs.
> 
> Isn't the code as-is also "leaking" an open IO::Handle? There's a
> CloseHandle($fHandle), but nothing is done to $fh. But perhaps there's some
> perl magic cleaning things up? Even if so, loks like just closing $fh will
> close the handle as well...

I think something roughly like the attached might be a good idea. Runs locally
on linux, and hopefully still on windows

https://cirrus-ci.com/build/4857291573821440

Greetings,

Andres Freund

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. ci: enable zstd where available.

  2. ci: compile with -Og where applicable.

  3. ci: include hints how to install OS packages.

  4. ci: fix copy-paste mistake in 16eb8231d1b.

  5. ci: macos: align sysinfo_script to other tasks.

  6. ci: Only use one artifact instruction for logs.

  7. ci: s/CCACHE_SIZE/CCACHE_MAXSIZE/.

  8. pg_basebackup: Skip a few more fsyncs if --no-sync is specified.

  9. TAP tests: check for postmaster.pid anyway when "pg_ctl start" fails.

  10. Don't enable fsync in src/test/recovery/t/008_fsm_truncation.pl.

  11. ci: windows: run initdb with --no-sync.

  12. ci: windows: enable build summary to make it easier to spot warnings / errors.

  13. ci: Add continuous integration for github repositories via cirrus-ci.

  14. Fix TestLib::slurp_file() with offset on windows.