On Thu, Jul 28, 2022 at 3:21 AM Andrew Dunstan <and...@dunslane.net> wrote: > On 2022-07-27 We 10:58, Tom Lane wrote: > > Andrew Dunstan <and...@dunslane.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...
From 33005aaad52a550ba028277dfb634d3d6fbfdd7f Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Thu, 28 Jul 2022 11:45:31 +1200 Subject: [PATCH] Fix get_dirent_type() for simlinks on MinGW/MSYS. On Windows with MSVC, get_dirent_type() was recently made to return DT_LNK for junction points by commit 9d3444dc, which fixed some defective dirent.c code. On Windows with Cygwin, get_dirent_type() already worked for symlinks, as it does on POSIX systems, because Cygwin has its own fake symlinks that behave like POSIX (on closer inspection, Cygwin's dirent has the BSD d_type extension but it's probably always DT_UNKNOWN, so we fall back to lstat(), which understands Cygwin symlinks with S_ISLNK()). On Windows with MinGW/MSYS, we need extra code, because the MinGW runtime has its own readdir() without d_type, and the lstat()-based fallback has no knowledge of our convention for treating junctions as symlinks. Reported-by: Andrew Dunstan <and...@dunslane.net> Discussion: https://postgr.es/m/b9ddf605-6b36-f90d-7c30-7b3e95c46276%40dunslane.net --- src/common/file_utils.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/common/file_utils.c b/src/common/file_utils.c index 19d308ad1f..210b2c4dfd 100644 --- a/src/common/file_utils.c +++ b/src/common/file_utils.c @@ -465,5 +465,19 @@ get_dirent_type(const char *path, #endif } +#if defined(WIN32) && !defined(_MSC_VER) + /* + * If we're on native Windows (not Cygwin, which has its own POSIX + * symlinks), but not using the MSVC compiler, then we're using a readdir() + * emulation provided by the compiler's runtime that has no concept of + * symlinks. It sees junction points as directories, so we need an extra + * system call to recognize them as symlinks, following our convention. + */ + if (result == PGFILETYPE_DIR && + !look_through_symlinks && + pgwin32_is_junction(path)) + result = PGFILETYPE_LNK; +#endif + return result; } -- 2.36.1
