Re: fairywren hung in pg_basebackup tests

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: fairywren hung in pg_basebackup tests
Date: 2022-07-28 00:41:20
Message-ID: CA+hUKGJHAxo6CtnEiYH4Q-Pm0J+dTwDT-fOjAm0cB3VAUbFAhg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 28, 2022 at 3:21 AM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> On 2022-07-27 We 10:58, Tom Lane wrote:
> > Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >> The alternative I thought of would be to switch msys to using our
> >> dirent.c. Probably not too hard, but certainly more work than reverting.

Thanks for figuring this out Andrew. Previously I thought of MSYS as
a way to use configure+make+gcc/clang but pure Windows C APIs (using
MinGW's replacement Windows headers), but today I learned that
MSYS/MinGW also supplies a small amount of POSIX stuff, including
readdir() etc, so we don't use our own emulation in that case.

I suppose we could consider using own dirent.h/c with MinGW (and
seeing if there are other similar hazards), to reduce the number of
Windows/POSIX API combinations we have to fret about, but not today.

Another thought for the future is that lstat() + S_ISLNK() could
probably be made to fire for junction points on Windows (all build
variants), and then get_dirent_type()'s fallback code for DT_UNKNOWN
would have Just Worked (no extra system call required), and we could
also probably remove calls to pgwin32_is_junction() everywhere.

> > If you ask me, the shortest-path general-purpose fix is to insert
> >
> > #if MSYS
> > if (pgwin32_is_junction(path))
> > return PGFILETYPE_DIR;
> > #endif
> >
> > at the start of get_dirent_type. (I'm not sure how to spell the
> > #if test.) We could look at using dirent.c later, but I think
> > right now it's important to un-break the buildfarm ASAP.
>
> +1. I think you spell it:
>
> #if defined(WIN32) && !defined(_MSC_VER)

I thought about putting it at the top, but don't we really only need
to make an extra system call if MinGW's stat() told us it saw a
directory? And what if you asked to look through symlinks? I thought
about putting it near the S_ISLNK() test, which is the usual pattern,
but then what if MinGW decides to add d_type support one day? Those
thoughts led to the attached formulation. Untested. I'll go and try
to see if I can run this with Melih's proposed MSYS CI support...

Attachment Content-Type Size
0001-Fix-get_dirent_type-for-simlinks-on-MinGW-MSYS.patch text/x-patch 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2022-07-28 01:06:41 Re: Introduce wait_for_subscription_sync for TAP tests
Previous Message David G. Johnston 2022-07-27 23:43:11 Re: Proposal: add a debug message about using geqo