Re: fairywren hung in pg_basebackup tests

On 2022-07-27 We 09:47, Andrew Dunstan wrote:
> On 2022-07-26 Tu 18:31, Thomas Munro wrote:
>> On Tue, Jul 26, 2022 at 4:03 AM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>> On 2022-07-25 Mo 11:24, Thomas Munro wrote:
>>>> On Tue, Jul 26, 2022 at 3:08 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>> Hmm ... an alternative theory is that the test is fine, and what
>>>>> it's telling us is that get_dirent_type() is still wrong on msys.
>>>>> Would that end in this symptom?
>>>> Hmm, possibly yes (if it sees a non-symlink, it'll skip it). If
>>>> someone can run the test on an msys system, perhaps they could put a
>>>> debugging elog() into the code modified by 9d3444dc to log d_name and
>>>> the d_type that is returned? I'm struggling to understand why msys
>>>> would change the answer though.
>>> I have no idea either. The link exists and it is a junction. I'll see
>>> about logging details.
>> >From the clues so far, it seems like pgwin32_is_junction(fullpath) was
>> returning true, but the following code in get_dirent_type(), which was
>> supposed to be equivalent, is not reached on MSYS (though it
>> apparently does on MSVC?):
>>
>> + if ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 &&
>> + (fd.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT))
>> d->ret.d_type = DT_LNK;
>>
>> pgwin32_is_junction() uses GetFileAttributes() and tests (attr &
>> FILE_ATTRIBUTE_REPARSE_POINT) == FILE_ATTRIBUTE_REPARSE_POINT, which
>> is like the first condition but lacks the dwReserved0 part. What is
>> that part doing, and why would it be doing something different in MSVC
>> and MSYS builds? That code came from 87e6ed7c, recently I was just
>> trying to fix it by reordering the checks; oh, there was some
>> discussion about that field[1].
>>
>> One idea is that something about dwReserved0 or
>> IO_REPARSE_TAG_MOUNT_POINT is different in the open source replacement
>> system headers supplied by the MinGW project used by MSYS builds
>> (right?), compared to the "real" Windows SDK's headers used by MSVC
>> builds.
>>
>> Or perhaps there is some other dumb mistake, or perhaps the reparse
>> point really is different, or ... I dunno, I'd probably shove a load
>> of log messages in there and see what's going on.
>>
>> [1] https://www.postgresql.org/message-id/flat/CABUevEzURN%3DwC95JHvTKFJtEy0eY9rWO42yU%3D59-q8xSwm-Dug%40mail.gmail.com#ac54acd782fc849c0fe6c2c05db101dc
>
> dirent.c is not used on msys, only on MSVC. msys is apparently using
> opendir and friends supplied by the system.
>
> What it does if there's a junction I'll try to find out, but it appears
> that 5344723755 was conceived under a misapprehension about the
> behaviour of msys.
>
>

The msys dirent.h doesn't have a d_type field at all in a struct dirent.
I can see a number of ways of dealing with this, but the simplest seems
to be just to revert 5344723755, at least for msys, along with a comment
about why it's necessary.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com