Re: Checking pgwin32_is_junction() errors
On Thu, Aug 11, 2022 at 10:40 PM wrote:
> On 2022-08-11 07:55, Thomas Munro wrote:
> >> I checked a few variants:
> >>
> >> 21.07.2022 15:11 HOME [\??\Volume{GUID}\]
> >> 09.08.2022 15:06 Test1 [\\?\Volume{GUID}\]
> >> 09.08.2022 15:06 Test2 [\\.\Volume{GUID}\]
> >> 09.08.2022 15:17 Test3 [\??\Volume{GUID}\]
> >> 09.08.2022 15:27 Test4 [C:\temp\1]
> >> 09.08.2022 15:28 Test5 [C:\HOME\Temp\1]
> >
> > One more thing I wondered about, now that we're following junctions
> > outside PGDATA: can a junction point to another junction? If so, I
> > didn't allow for that: stat() gives up after one hop, because I
> > figured that was enough for the stuff we expect inside PGDATA and I
> > couldn't find any evidence in the man pages that referred to chains.
> > But if you *are* allowed to create a junction "c:\huey" that points to
> > junction "c:\dewey" that points to "c:\louie", and then you do initdb
> > -D c:\huey\pgdata, then I guess it would fail. Would you mind
> > checking if that is a real possibility, and if so, testing this
> > chain-following patch to see if it fixes it?
>
> I made some junctions and rechecked both patches.
>
> 11.08.2022 16:11 donald [C:\huey]
> 11.08.2022 13:23 huey [C:\dewey]
> 11.08.2022 13:23 dewey [C:\louie]
> 11.08.2022 16:57 louie
>
> With the small attached patch initdb succeeded in any of these
> "directories". If the junction chain is too long, initdb fails with
> "could not create directory" as expected.
Thanks for testing and for that fix! I do intend to push this, and a
nearby fix for unlink(), but first I want to have test coverage for
all this stuff so we can demonstrate comprehensively that it works via
automated testing, otherwise it's just impossible to maintain (at
least for me, Unix guy). I have a prototype test suite based on
writing TAP tests in C and I've already found more subtle ancient bugs
around the Windows porting layer... more soon.
Re: Checking pgwin32_is_junction() errors
On 2022-08-11 07:55, Thomas Munro wrote:
I checked a few variants:
21.07.2022 15:11 HOME [\??\Volume{GUID}\]
09.08.2022 15:06 Test1 [\\?\Volume{GUID}\]
09.08.2022 15:06 Test2 [\\.\Volume{GUID}\]
09.08.2022 15:17 Test3 [\??\Volume{GUID}\]
09.08.2022 15:27 Test4 [C:\temp\1]
09.08.2022 15:28 Test5 [C:\HOME\Temp\1]
One more thing I wondered about, now that we're following junctions
outside PGDATA: can a junction point to another junction? If so, I
didn't allow for that: stat() gives up after one hop, because I
figured that was enough for the stuff we expect inside PGDATA and I
couldn't find any evidence in the man pages that referred to chains.
But if you *are* allowed to create a junction "c:\huey" that points to
junction "c:\dewey" that points to "c:\louie", and then you do initdb
-D c:\huey\pgdata, then I guess it would fail. Would you mind
checking if that is a real possibility, and if so, testing this
chain-following patch to see if it fixes it?
I made some junctions and rechecked both patches.
11.08.2022 16:11 donald [C:\huey]
11.08.2022 13:23 huey [C:\dewey]
11.08.2022 13:23 dewey [C:\louie]
11.08.2022 16:57 louie
With the small attached patch initdb succeeded in any of these
"directories". If the junction chain is too long, initdb fails with
"could not create directory" as expected.
initdb -D huey/pgdata
...
Success.
initdb -N -D donald
...
Success.
11.08.2022 17:32 1
11.08.2022 17:32 2 [C:\1]
11.08.2022 17:32 3 [C:\2]
11.08.2022 17:32 4 [C:\3]
11.08.2022 17:32 5 [C:\4]
11.08.2022 17:32 6 [C:\5]
11.08.2022 17:32 7 [C:\6]
11.08.2022 17:32 8 [C:\7]
11.08.2022 17:32 9 [C:\8]
11.08.2022 17:32 10 [C:\9]
initdb -D 10/pgdata
...
creating directory 10/pgdata ... initdb: error: could not create
directory "10": File exists
initdb -D 9/pgdata
...
Success.
--- win32stat.c.bak 2022-08-11 17:15:10.775412600 +0700
+++ win32stat.c 2022-08-11 16:47:41.992805700 +0700
@@ -186,9 +186,12 @@
{
int loops = 0;
int ret;
+ char curr[MAXPGPATH];
ret = _pglstat64(name, buf);
+ strlcpy(curr, name, MAXPGPATH);
+
/* Do we need to follow a symlink (junction point)? */
while (ret == 0 && S_ISLNK(buf->st_mode))
{
@@ -211,7 +214,7 @@
* That could be optimized, but stat() on symlinks is probably rare
* and this way is simple.
*/
- size = readlink(name, next, sizeof(next));
+ size = readlink(curr, next, sizeof(next));
if (size < 0)
{
if (errno == EACCES &&
@@ -230,6 +233,7 @@
next[size] = 0;
ret = _pglstat64(next, buf);
+ strcpy(curr, next);
}
return ret;
Re: Checking pgwin32_is_junction() errors
On Tue, Aug 9, 2022 at 10:59 PM wrote:
> On 2022-08-09 05:44, Thomas Munro wrote:
> > On Tue, Aug 9, 2022 at 8:30 AM Thomas Munro
> > wrote:
> >> On Mon, Aug 8, 2022 at 8:23 PM wrote:
> >> > "C:/HOME" is the junction point to the second volume on my hard drive -
> >> > "\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
> >> > https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
> >
> >> ... Would it
> >> be better to say: if it doesn't begin with "\??\X:", where X could be
> >> any letter, then don't modify it?
> >
> > Concretely, I wonder if this is a good fix at least in the short term.
> > Does this work for you, and do the logic and explanation make sense?
>
> Yes, this patch works well with my junction points.
Thanks for testing! I did a bit more reading on this stuff, so that I
could update the comments with the correct terminology from Windows
APIs. I also realised that the pattern we could accept to symlink()
and expect to work is not just "C:..." (could be
RtlPathTypeDriveRelative, which wouldn't actually work in a junction
point) but "C:\..." (RtlPathTypeDriveAbsolute). I tweaked it a bit to
test for that.
> I checked a few variants:
>
> 21.07.2022 15:11 HOME [\??\Volume{GUID}\]
> 09.08.2022 15:06 Test1 [\\?\Volume{GUID}\]
> 09.08.2022 15:06 Test2 [\\.\Volume{GUID}\]
> 09.08.2022 15:17 Test3 [\??\Volume{GUID}\]
> 09.08.2022 15:27 Test4 [C:\temp\1]
> 09.08.2022 15:28 Test5 [C:\HOME\Temp\1]
One more thing I wondered about, now that we're following junctions
outside PGDATA: can a junction point to another junction? If so, I
didn't allow for that: stat() gives up after one hop, because I
figured that was enough for the stuff we expect inside PGDATA and I
couldn't find any evidence in the man pages that referred to chains.
But if you *are* allowed to create a junction "c:\huey" that points to
junction "c:\dewey" that points to "c:\louie", and then you do initdb
-D c:\huey\pgdata, then I guess it would fail. Would you mind
checking if that is a real possibility, and if so, testing this
chain-following patch to see if it fixes it?
> After hours of reading the documentation and debugging, it seems to me
> we can use REPARSE_GUID_DATA_BUFFER structure instead of our
> REPARSE_JUNCTION_DATA_BUFFER [1]. DataBuffer doesn't contain any
> prefixes,
> so we don't need to strip them. But we still need to construct a correct
> volume name if a junction point is a volume mount point. Is it worth to
> check this idea?
I don't know. I think I prefer our current approach, because it can
handle anything (raw/full NT paths) and doesn't try to be very clever,
and I don't want to change to a different scheme for no real
benefit...
From 6ed3edc77275aa51d1d0b1012c0a36db65735d39 Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Thu, 11 Aug 2022 10:42:13 +1200
Subject: [PATCH v2 1/2] Fix readlink() for general Windows junction points.
Our symlink() and readlink() replacements perform a naive transformation
from DOS paths to NT paths and back, as required by the junction point
APIs. This was enough for the "drive absolute" paths we expect users to
provide for tablespaces, for example "d:\my\fast\storage".
Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb
uses pg_mkdir_p(), and that examines parent directories, our humble
readlink() implementation can now be exposed to junction points not of
PostgreSQL origin. Those might be corrupted by our naive path mangling,
which doesn't really understand NT paths in general.
Simply decline to transform paths that don't look like a drive absolute
path. That means that readlink() returns the NT path directly when
checking a parent directory of PGDATA that happen to point to a drive
using "rooted" format. That works for the purposes of our stat()
emulation.
Reported-by: Roman Zharkov
Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru
---
src/port/dirmod.c | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 2818bfd2e9..53310baafb 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -199,7 +199,11 @@ pgsymlink(const char *oldpath, const char *newpath)
if (dirhandle == INVALID_HANDLE_VALUE)
return -1;
- /* make sure we have an unparsed native win32 path */
+ /*
+ * We expect either an NT path or a "drive absolute" path like "C:\..."
+ * that we convert to an NT path by bolting on a prefix and converting any
+ * Unix-style path delimiters to NT-style.
+ */
if (memcmp("\\??\\", oldpath, 4) != 0)
snprintf(nativeTarget, sizeof(nativeTarget), "\\??\\%s", oldpath);
else
@@ -351,10 +355,21 @@ pgreadlink(const char *path, char *buf, size_t size)
}
/*
- * If the path starts with "\??\", which it will do in most (all?) cases,
- * strip those out.
+ * If the path starts with "\??\"
Re: Checking pgwin32_is_junction() errors
On 2022-08-09 05:44, Thomas Munro wrote:
On Tue, Aug 9, 2022 at 8:30 AM Thomas Munro
wrote:
On Mon, Aug 8, 2022 at 8:23 PM wrote:
> "C:/HOME" is the junction point to the second volume on my hard drive -
> "\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
>
https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
... Would it
be better to say: if it doesn't begin with "\??\X:", where X could be
any letter, then don't modify it?
Concretely, I wonder if this is a good fix at least in the short term.
Does this work for you, and do the logic and explanation make sense?
Yes, this patch works well with my junction points.
I checked a few variants:
21.07.2022 15:11 HOME [\??\Volume{GUID}\]
09.08.2022 15:06 Test1 [\\?\Volume{GUID}\]
09.08.2022 15:06 Test2 [\\.\Volume{GUID}\]
09.08.2022 15:17 Test3 [\??\Volume{GUID}\]
09.08.2022 15:27 Test4 [C:\temp\1]
09.08.2022 15:28 Test5 [C:\HOME\Temp\1]
After hours of reading the documentation and debugging, it seems to me
we can use REPARSE_GUID_DATA_BUFFER structure instead of our
REPARSE_JUNCTION_DATA_BUFFER [1]. DataBuffer doesn't contain any
prefixes,
so we don't need to strip them. But we still need to construct a correct
volume name if a junction point is a volume mount point. Is it worth to
check this idea?
[1]
https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-reparse_guid_data_buffer
Re: Checking pgwin32_is_junction() errors
On 2022-08-09 03:30, Thomas Munro wrote:
Then our readlink() emulation removes it again, but in the case of
your \??\Volume{GUID} path, created by you, not our symlink()
emulation, removing "\??\" apparently makes it unopenable with
CreateFile() (I guess that's what fails? What's the error?). So your
patch just says: don't strip "\??\" if it's followed by "Volume".
Sorry, I thought wrong that everyone sees the backtrace on my screen.
Failes the CreateFile() function with fileName = "Volume{GUID}\" at [1].
And the GetLastError() returnes 2 (ERROR_FILE_NOT_FOUND).
Call Stack:
initdb.exe!pgwin32_open_handle(const char * fileName, ...) Line 111 C
initdb.exe!_pglstat64(const char * name, stat * buf) Line 128 C
initdb.exe!_pgstat64(const char * name, stat * buf) Line 221C
initdb.exe!pg_mkdir_p(char * path, int omode) Line 123 C
initdb.exe!create_data_directory() Line 2537C
initdb.exe!initialize_data_directory() Line 2696C
initdb.exe!main(int argc, char * * argv) Line 3102 C
I don't understand all the kinds of DOS, Windows and NT paths (let me
take a moment to say how much I love Unix), but here's a guess: could
it be that NT "\??\C:\foo" = DOS "C:\foo", but NT "\??\Volume..." =
DOS "\Volume..."? In other words, if it hasn't got a drive letter,
maybe it still needs an initial "\" (or if not that, then *something*
special, because otherwise it looks like a relative path).
It seems to me, when we call CreateFile() Windows Object Manager
searches
DOS devices (drive letters in our case) in DOS Device namespaces.
But it doesn't search the "Volume{GUID}" devices which must be named as
"\\?\Volume{GUID}\" [2].
Would it be better to say: if it doesn't begin with "\??\X:", where X
could be any letter, then don't modify it?
I think it will be better.
[1]
https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/open.c#L86
[2]
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-volume
Re: Checking pgwin32_is_junction() errors
On Tue, Aug 9, 2022 at 8:30 AM Thomas Munro wrote:
> On Mon, Aug 8, 2022 at 8:23 PM wrote:
> > "C:/HOME" is the junction point to the second volume on my hard drive -
> > "\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
> > https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
> ... Would it
> be better to say: if it doesn't begin with "\??\X:", where X could be
> any letter, then don't modify it?
Concretely, I wonder if this is a good fix at least in the short term.
Does this work for you, and do the logic and explanation make sense?
From 9772b2bf5a1b11605ea16cfd17b3b50e0274aadf Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Tue, 9 Aug 2022 10:29:08 +1200
Subject: [PATCH] Fix readlink() for more complex Windows paths.
Our symlink() and readlink() replacements perform a very naive
transformation from "DOS" paths to "NT" paths and back, as required by
the junction point APIs. This was enough for the simple paths we expect
users to provide for tablespaces, for example 'd:\my\fast\storage'.
Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb
uses pg_mkdir_p(), and that examines parent directories, our humble
readlink() implementation can now be exposed to junction points not of
PostgreSQL origin. Those might be corrupted by our naive path mangling,
which doesn't really understand NT paths in general.
It's possible that we should investigate using Rtl*() library routines
to do path transformation, but for now, simply decline to transform
paths that don't fit the simple drive-qualified pattern. That means
that readlink() returns an NT path, which works just as well as a DOS
path when following the link.
Reported-by: Roman Zharkov
Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru
---
src/port/dirmod.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 2818bfd2e9..821563c290 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -352,9 +352,11 @@ pgreadlink(const char *path, char *buf, size_t size)
/*
* If the path starts with "\??\", which it will do in most (all?) cases,
- * strip those out.
+ * strip those out, but only if it's of the simple drive-letter-qualified
+ * type that we know how to transform from NT to DOS format.
*/
- if (r > 4 && strncmp(buf, "\\??\\", 4) == 0)
+ if (r >= 6 && strncmp(buf, "\\??\\", 4) == 0 && isalpha(buf[4]) &&
+ buf[5] == ':')
{
memmove(buf, buf + 4, strlen(buf + 4) + 1);
r -= 4;
--
2.35.1
Re: Checking pgwin32_is_junction() errors
On Mon, Aug 8, 2022 at 8:23 PM wrote:
> initdb on my windows 10 system stopped working after the commit
> c5cb8f3b: "Provide lstat() for Windows."
> The error message is: creating directory C:/HOME/data ... initdb:
> error: could not create directory "C:/HOME": File exists
>
> "C:/HOME" is the junction point to the second volume on my hard drive -
> "\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
> https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
> So initdb could not stat the file with name "Volume{GUID}", tried to
> create it and failed.
> With the attached patch initdb works fine again.
-if (r > 4 && strncmp(buf, "\\??\\", 4) == 0)
+if (r > 4 && strncmp(buf, "\\??\\", 4) == 0 &&
+strncmp(buf, "\\??\\Volume", 10) != 0)
{
memmove(buf, buf + 4, strlen(buf + 4) + 1);
r -= 4;
Hmm. I suppose the problem must be in pg_mkdir_p(). Our symlink()
emulation usually adds this "\??\" prefix (making it an "NT object
path"?), because junction points only work if they are in that format.
Then our readlink() emulation removes it again, but in the case of
your \??\Volume{GUID} path, created by you, not our symlink()
emulation, removing "\??\" apparently makes it unopenable with
CreateFile() (I guess that's what fails? What's the error?). So your
patch just says: don't strip "\??\" if it's followed by "Volume".
I don't understand all the kinds of DOS, Windows and NT paths (let me
take a moment to say how much I love Unix), but here's a guess: could
it be that NT "\??\C:\foo" = DOS "C:\foo", but NT "\??\Volume..." =
DOS "\Volume..."? In other words, if it hasn't got a drive letter,
maybe it still needs an initial "\" (or if not that, then *something*
special, because otherwise it looks like a relative path). Would it
be better to say: if it doesn't begin with "\??\X:", where X could be
any letter, then don't modify it?
Maybe [1] has some clues. It seems to give the info in a higher
density form than the Windows docs (at least to the uninitiated like
me wanting a quick overview with examples). Hmm, I wonder if we could
get away from doing our own path mangling and use some of the proper
library calls mentioned on that page...
[1]
https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
Re: Checking pgwin32_is_junction() errors
On 2022-08-06 08:02, Thomas Munro wrote:
Pushed.
Hmm, this stuff could *really* use a little test framework that's run
by check-world, that exercises these various replacement operations.
But I also suspect that problems in this area are likely to be due to
concurrency. It's hard to make a simple test that simulates the case
where a file is unlinked between system calls within stat() and hits
the STATUS_DELETE_PENDING case. That check is code I cargo-culted in
this patch. So much of the stuff we've had in the tree relating to
that area has been wrong in the past...
Hello, hackers!
initdb on my windows 10 system stopped working after the commit
c5cb8f3b: "Provide lstat() for Windows."
The error message is: creating directory C:/HOME/data ... initdb:
error: could not create directory "C:/HOME": File exists
"C:/HOME" is the junction point to the second volume on my hard drive -
"\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
So initdb could not stat the file with name "Volume{GUID}", tried to
create it and failed.
With the attached patch initdb works fine again.
--
regards,
Romandiff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 2818bfd2e95..9d73620a6ba 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -354,7 +354,8 @@ pgreadlink(const char *path, char *buf, size_t size)
* If the path starts with "\??\", which it will do in most (all?) cases,
* strip those out.
*/
- if (r > 4 && strncmp(buf, "\\??\\", 4) == 0)
+ if (r > 4 && strncmp(buf, "\\??\\", 4) == 0 &&
+ strncmp(buf, "\\??\\Volume", 10) != 0)
{
memmove(buf, buf + 4, strlen(buf + 4) + 1);
r -= 4;
Re: Checking pgwin32_is_junction() errors
On Fri, Aug 5, 2022 at 9:17 PM Thomas Munro wrote: > Hmm, POSIX says st_link should contain the length of a symlink's > target path, so I suppose we should probably set that even though we > never consult it. Here's a version that does that. I also removed > the rest of the now redundant #ifdef S_ISLNK conditions. Pushed. Hmm, this stuff could *really* use a little test framework that's run by check-world, that exercises these various replacement operations. But I also suspect that problems in this area are likely to be due to concurrency. It's hard to make a simple test that simulates the case where a file is unlinked between system calls within stat() and hits the STATUS_DELETE_PENDING case. That check is code I cargo-culted in this patch. So much of the stuff we've had in the tree relating to that area has been wrong in the past...
Re: Checking pgwin32_is_junction() errors
On Thu, Aug 4, 2022 at 9:42 AM Thomas Munro wrote:
> On Thu, Aug 4, 2022 at 9:28 AM Andrew Dunstan wrote:
> > On 2022-08-01 Mo 16:06, Andrew Dunstan wrote:
> > > I'll try it out on fairywren/drongo.
>
> > They are happy with patches 2, 3, and 4.
>
> Thanks for testing!
>
> If there are no objections, I'll go ahead and commit these later today.
Hmm, POSIX says st_link should contain the length of a symlink's
target path, so I suppose we should probably set that even though we
never consult it. Here's a version that does that. I also removed
the rest of the now redundant #ifdef S_ISLNK conditions.
From 6e57c91b7b780ea51fa45e63cb201ff4240a416b Mon Sep 17 00:00:00 2001
From: Melih Mutlu
Date: Mon, 21 Feb 2022 14:46:05 +0300
Subject: [PATCH v4 1/4] Added Windows with MinGW environment in Cirrus CI
---
.cirrus.yml | 79 +++--
1 file changed, 65 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 81eb8a9996..9820915f35 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -23,7 +23,6 @@ env:
CHECKFLAGS: -Otarget
PROVE_FLAGS: --timer
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
- TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl
@@ -338,13 +337,30 @@ task:
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
+WINDOWS_ENVIRONMENT_BASE: _ENVIRONMENT_BASE
+env:
+ # Half the allowed per-user CPU cores
+ CPUS: 4
+ # The default working dir is in a directory msbuild complains about
+ CIRRUS_WORKING_DIR: "c:/cirrus"
+ TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
+
+ # Avoids port conflicts between concurrent tap test runs
+ PG_TEST_USE_UNIX_SOCKETS: 1
+
+only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*windows.*'
+
+sysinfo_script: |
+ chcp
+ systeminfo
+ powershell -Command get-psdrive -psprovider filesystem
+ set
+
task:
+ << : *WINDOWS_ENVIRONMENT_BASE
name: Windows - Server 2019, VS 2019
env:
-# Half the allowed per-user CPU cores
-CPUS: 4
-
# Our windows infrastructure doesn't have test concurrency above the level
# of a single vcregress test target. Due to that, it's useful to run prove
# with multiple jobs. For the other tasks it isn't, because two sources
@@ -354,15 +370,11 @@ task:
# likely can be improved upon further.
PROVE_FLAGS: -j10 --timer
-# The default cirrus working dir is in a directory msbuild complains about
-CIRRUS_WORKING_DIR: "c:/cirrus"
# Avoid re-installing over and over
NO_TEMP_INSTALL: 1
# git's tar doesn't deal with drive letters, see
# https://postgr.es/m/b6782dc3-a7b0-ed56-175f-f8f54cb08d67%40dunslane.net
TAR: "c:/windows/system32/tar.exe"
-# Avoids port conflicts between concurrent tap test runs
-PG_TEST_USE_UNIX_SOCKETS: 1
PG_REGRESS_SOCK_DIR: "c:/cirrus/"
# -m enables parallelism
# verbosity:minimal + Summary reduce verbosity, while keeping a summary of
@@ -393,12 +405,6 @@ task:
cpu: $CPUS
memory: 4G
- sysinfo_script: |
-chcp
-systeminfo
-powershell -Command get-psdrive -psprovider filesystem
-set
-
setup_additional_packages_script: |
REM choco install -y --no-progress ...
@@ -456,6 +462,51 @@ task:
path: "crashlog-*.txt"
type: text/plain
+task:
+ << : *WINDOWS_ENVIRONMENT_BASE
+ name: Windows - Server 2019, MinGW64
+ windows_container:
+image: $CONTAINER_REPO/windows_ci_mingw64:latest
+cpu: $CPUS
+memory: 4G
+ env:
+CCACHE_DIR: C:/msys64/ccache
+BUILD_DIR: "%CIRRUS_WORKING_DIR%/build"
+
+ ccache_cache:
+folder: ${CCACHE_DIR}
+
+ mingw_info_script:
+- C:\msys64\usr\bin\bash.exe -lc "where gcc"
+- C:\msys64\usr\bin\bash.exe -lc "gcc --version"
+- C:\msys64\usr\bin\bash.exe -lc "where perl"
+- C:\msys64\usr\bin\bash.exe -lc "perl --version"
+
+ configure_script:
+- C:\msys64\usr\bin\bash.exe -lc "mkdir %BUILD_DIR% &&
+ cd %BUILD_DIR% &&
+ %CIRRUS_WORKING_DIR%/configure
+--host=x86_64-w64-mingw32
+--enable-cassert
+--enable-tap-tests
+--with-icu
+--with-libxml
+--with-libxslt
+--with-lz4
+--enable-debug
+CC='ccache gcc'
+CXX='ccache g++'"
+
+ build_script:
+C:\msys64\usr\bin\bash.exe -lc "cd %BUILD_DIR% && make -s world-bin -j${CPUS}"
+
+ upload_caches: ccache
+
+ tests_script:
+ - set "NoDefaultCurrentDirectoryInExePath=0"
+ - C:\msys64\usr\bin\bash.exe -lc "cd %BUILD_DIR% && make -s ${CHECK} ${CHECKFLAGS} -j${CPUS} TMPDIR=%BUILD_DIR%/tmp_install"
+
+ on_failure: *on_failure
task:
name: CompilerWarnings
--
2.37.1
From f8be96a1c712b7ffe42445108022b9978825ce97 Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Fri, 5 Aug 2022 16:41:34 +1200
Subject: [PATCH v4
Re: Checking pgwin32_is_junction() errors
On Thu, Aug 4, 2022 at 9:28 AM Andrew Dunstan wrote: > On 2022-08-01 Mo 16:06, Andrew Dunstan wrote: > > I'll try it out on fairywren/drongo. > They are happy with patches 2, 3, and 4. Thanks for testing! If there are no objections, I'll go ahead and commit these later today.
Re: Checking pgwin32_is_junction() errors
On 2022-08-01 Mo 16:06, Andrew Dunstan wrote:
> On 2022-08-01 Mo 01:09, Thomas Munro wrote:
>> On Thu, Jul 28, 2022 at 9:31 PM Thomas Munro wrote:
>>> There's one curious change in the draft patch attached: you can't
>>> unlink() a junction point, you have to rmdir() it. Previously, things
>>> that traverse directories without ever calling pgwin32_is_junction()
>>> would see junction points as S_ISDIR() and call rmdir(), which was OK,
>>> but now they see S_ISLNK() and call unlink(). So I taught unlink() to
>>> try both things. Which is kinda weird, and not beautiful, especially
>>> when combined with the existing looping weirdness.
>> Here's a new attempt at unlink(), this time in its own patch. This
>> version is a little more careful about calling rmdir() only after
>> checking that it is a junction point, so that unlink("a directory")
>> fails just like on Unix (well, POSIX says that that should fail with
>> EPERM, not EACCES, and implementations are allowed to make it work
>> anyway, but it doesn't seem helpful to allow it to work there when
>> every OS I know of fails with EPERM or EISDIR). That check is racy,
>> but should be good enough for our purposes, no (see comment for a note
>> on that)?
>>
>> Longer term, I wonder if we should get rid of our use of symlinks, and
>> instead just put paths in a file and do our own path translation. But
>> for now, this patch set completes the set of junction point-based
>> emulations, and, IMHO, cleans up a confusing aspect of our code.
>>
>> As before, 0001 is just for cfbot to add an MSYS checkmark.
>
>
> I'll try it out on fairywren/drongo.
>
>
They are happy with patches 2, 3, and 4.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: Checking pgwin32_is_junction() errors
On 2022-08-01 Mo 01:09, Thomas Munro wrote:
> On Thu, Jul 28, 2022 at 9:31 PM Thomas Munro wrote:
>> There's one curious change in the draft patch attached: you can't
>> unlink() a junction point, you have to rmdir() it. Previously, things
>> that traverse directories without ever calling pgwin32_is_junction()
>> would see junction points as S_ISDIR() and call rmdir(), which was OK,
>> but now they see S_ISLNK() and call unlink(). So I taught unlink() to
>> try both things. Which is kinda weird, and not beautiful, especially
>> when combined with the existing looping weirdness.
> Here's a new attempt at unlink(), this time in its own patch. This
> version is a little more careful about calling rmdir() only after
> checking that it is a junction point, so that unlink("a directory")
> fails just like on Unix (well, POSIX says that that should fail with
> EPERM, not EACCES, and implementations are allowed to make it work
> anyway, but it doesn't seem helpful to allow it to work there when
> every OS I know of fails with EPERM or EISDIR). That check is racy,
> but should be good enough for our purposes, no (see comment for a note
> on that)?
>
> Longer term, I wonder if we should get rid of our use of symlinks, and
> instead just put paths in a file and do our own path translation. But
> for now, this patch set completes the set of junction point-based
> emulations, and, IMHO, cleans up a confusing aspect of our code.
>
> As before, 0001 is just for cfbot to add an MSYS checkmark.
I'll try it out on fairywren/drongo.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: Checking pgwin32_is_junction() errors
On Thu, Jul 28, 2022 at 9:31 PM Thomas Munro wrote:
> There's one curious change in the draft patch attached: you can't
> unlink() a junction point, you have to rmdir() it. Previously, things
> that traverse directories without ever calling pgwin32_is_junction()
> would see junction points as S_ISDIR() and call rmdir(), which was OK,
> but now they see S_ISLNK() and call unlink(). So I taught unlink() to
> try both things. Which is kinda weird, and not beautiful, especially
> when combined with the existing looping weirdness.
Here's a new attempt at unlink(), this time in its own patch. This
version is a little more careful about calling rmdir() only after
checking that it is a junction point, so that unlink("a directory")
fails just like on Unix (well, POSIX says that that should fail with
EPERM, not EACCES, and implementations are allowed to make it work
anyway, but it doesn't seem helpful to allow it to work there when
every OS I know of fails with EPERM or EISDIR). That check is racy,
but should be good enough for our purposes, no (see comment for a note
on that)?
Longer term, I wonder if we should get rid of our use of symlinks, and
instead just put paths in a file and do our own path translation. But
for now, this patch set completes the set of junction point-based
emulations, and, IMHO, cleans up a confusing aspect of our code.
As before, 0001 is just for cfbot to add an MSYS checkmark.
From c89ee0d78944e05fa28c1d6c2939d94b6e39987a Mon Sep 17 00:00:00 2001
From: Melih Mutlu
Date: Mon, 21 Feb 2022 14:46:05 +0300
Subject: [PATCH v3 1/4] Added Windows with MinGW environment in Cirrus CI
---
.cirrus.yml | 79 +++--
1 file changed, 65 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 72735d225a..8decef68e8 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -23,7 +23,6 @@ env:
CHECKFLAGS: -Otarget
PROVE_FLAGS: --timer
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
- TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl
@@ -338,13 +337,30 @@ task:
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
+WINDOWS_ENVIRONMENT_BASE: _ENVIRONMENT_BASE
+env:
+ # Half the allowed per-user CPU cores
+ CPUS: 4
+ # The default working dir is in a directory msbuild complains about
+ CIRRUS_WORKING_DIR: "c:/cirrus"
+ TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
+
+ # Avoids port conflicts between concurrent tap test runs
+ PG_TEST_USE_UNIX_SOCKETS: 1
+
+only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*windows.*'
+
+sysinfo_script: |
+ chcp
+ systeminfo
+ powershell -Command get-psdrive -psprovider filesystem
+ set
+
task:
+ << : *WINDOWS_ENVIRONMENT_BASE
name: Windows - Server 2019, VS 2019
env:
-# Half the allowed per-user CPU cores
-CPUS: 4
-
# Our windows infrastructure doesn't have test concurrency above the level
# of a single vcregress test target. Due to that, it's useful to run prove
# with multiple jobs. For the other tasks it isn't, because two sources
@@ -354,15 +370,11 @@ task:
# likely can be improved upon further.
PROVE_FLAGS: -j10 --timer
-# The default cirrus working dir is in a directory msbuild complains about
-CIRRUS_WORKING_DIR: "c:/cirrus"
# Avoid re-installing over and over
NO_TEMP_INSTALL: 1
# git's tar doesn't deal with drive letters, see
# https://postgr.es/m/b6782dc3-a7b0-ed56-175f-f8f54cb08d67%40dunslane.net
TAR: "c:/windows/system32/tar.exe"
-# Avoids port conflicts between concurrent tap test runs
-PG_TEST_USE_UNIX_SOCKETS: 1
PG_REGRESS_SOCK_DIR: "c:/cirrus/"
# -m enables parallelism
# verbosity:minimal + Summary reduce verbosity, while keeping a summary of
@@ -393,12 +405,6 @@ task:
cpu: $CPUS
memory: 4G
- sysinfo_script: |
-chcp
-systeminfo
-powershell -Command get-psdrive -psprovider filesystem
-set
-
setup_additional_packages_script: |
REM choco install -y --no-progress ...
@@ -456,6 +462,51 @@ task:
path: "crashlog-*.txt"
type: text/plain
+task:
+ << : *WINDOWS_ENVIRONMENT_BASE
+ name: Windows - Server 2019, MinGW64
+ windows_container:
+image: $CONTAINER_REPO/windows_ci_mingw64:latest
+cpu: $CPUS
+memory: 4G
+ env:
+CCACHE_DIR: C:/msys64/ccache
+BUILD_DIR: "%CIRRUS_WORKING_DIR%/build"
+
+ ccache_cache:
+folder: ${CCACHE_DIR}
+
+ mingw_info_script:
+- C:\msys64\usr\bin\bash.exe -lc "where gcc"
+- C:\msys64\usr\bin\bash.exe -lc "gcc --version"
+- C:\msys64\usr\bin\bash.exe -lc "where perl"
+- C:\msys64\usr\bin\bash.exe -lc "perl --version"
+
+ configure_script:
+- C:\msys64\usr\bin\bash.exe -lc "mkdir %BUILD_DIR% &&
+ cd %BUILD_DIR% &&
+
Re: Checking pgwin32_is_junction() errors
Here's a better idea, now that I'm emboldened by having working CI for
Windows frankenbuilds, and since I broke some stuff in this area on
MSYS[1], which caused me to look more closely at this area.
Why don't we just nuke pgwin32_is_junction() from orbit, and teach
Windows how to lstat()? We're already defining our own replacement
stat() used in both MSVC and MSYS builds along with our own junction
point-based symlink() and readlink() functions, and lstat() was
already suggested in a comment in win32stat.c.
There's one curious change in the draft patch attached: you can't
unlink() a junction point, you have to rmdir() it. Previously, things
that traverse directories without ever calling pgwin32_is_junction()
would see junction points as S_ISDIR() and call rmdir(), which was OK,
but now they see S_ISLNK() and call unlink(). So I taught unlink() to
try both things. Which is kinda weird, and not beautiful, especially
when combined with the existing looping weirdness.
0001 is a copy of v2 of Melih Mutlu's CI patch[2] to show cfbot how to
test this on MSYS (alongside the normal MSVC result), but that's not
part of this submission.
[1]
https://www.postgresql.org/message-id/flat/b9ddf605-6b36-f90d-7c30-7b3e95c46276%40dunslane.net
[2]
https://www.postgresql.org/message-id/flat/CAGPVpCSKS9E0An4%3De7ZDnme%2By%3DWOcQFJYJegKO8kE9%3Dgh8NJKQ%40mail.gmail.com
From ec732571a1fa88fefcf9834987114716eb9c5998 Mon Sep 17 00:00:00 2001
From: Melih Mutlu
Date: Mon, 21 Feb 2022 14:46:05 +0300
Subject: [PATCH 1/3] Added Windows with MinGW environment in Cirrus CI
---
.cirrus.yml | 79 +++--
1 file changed, 65 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f23d6cae55..3d8c4cd813 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -23,7 +23,6 @@ env:
CHECKFLAGS: -Otarget
PROVE_FLAGS: --timer
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
- TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl
@@ -338,13 +337,30 @@ task:
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
+WINDOWS_ENVIRONMENT_BASE: _ENVIRONMENT_BASE
+env:
+ # Half the allowed per-user CPU cores
+ CPUS: 4
+ # The default working dir is in a directory msbuild complains about
+ CIRRUS_WORKING_DIR: "c:/cirrus"
+ TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
+
+ # Avoids port conflicts between concurrent tap test runs
+ PG_TEST_USE_UNIX_SOCKETS: 1
+
+only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*windows.*'
+
+sysinfo_script: |
+ chcp
+ systeminfo
+ powershell -Command get-psdrive -psprovider filesystem
+ set
+
task:
+ << : *WINDOWS_ENVIRONMENT_BASE
name: Windows - Server 2019, VS 2019
env:
-# Half the allowed per-user CPU cores
-CPUS: 4
-
# Our windows infrastructure doesn't have test concurrency above the level
# of a single vcregress test target. Due to that, it's useful to run prove
# with multiple jobs. For the other tasks it isn't, because two sources
@@ -354,15 +370,11 @@ task:
# likely can be improved upon further.
PROVE_FLAGS: -j10 --timer
-# The default cirrus working dir is in a directory msbuild complains about
-CIRRUS_WORKING_DIR: "c:/cirrus"
# Avoid re-installing over and over
NO_TEMP_INSTALL: 1
# git's tar doesn't deal with drive letters, see
# https://postgr.es/m/b6782dc3-a7b0-ed56-175f-f8f54cb08d67%40dunslane.net
TAR: "c:/windows/system32/tar.exe"
-# Avoids port conflicts between concurrent tap test runs
-PG_TEST_USE_UNIX_SOCKETS: 1
PG_REGRESS_SOCK_DIR: "c:/cirrus/"
# -m enables parallelism
# verbosity:minimal + Summary reduce verbosity, while keeping a summary of
@@ -393,12 +405,6 @@ task:
cpu: $CPUS
memory: 4G
- sysinfo_script: |
-chcp
-systeminfo
-powershell -Command get-psdrive -psprovider filesystem
-set
-
setup_additional_packages_script: |
REM choco install -y --no-progress ...
@@ -456,6 +462,51 @@ task:
path: "crashlog-*.txt"
type: text/plain
+task:
+ << : *WINDOWS_ENVIRONMENT_BASE
+ name: Windows - Server 2019, MinGW64
+ windows_container:
+image: $CONTAINER_REPO/windows_ci_mingw64:latest
+cpu: $CPUS
+memory: 4G
+ env:
+CCACHE_DIR: C:/msys64/ccache
+BUILD_DIR: "%CIRRUS_WORKING_DIR%/build"
+
+ ccache_cache:
+folder: ${CCACHE_DIR}
+
+ mingw_info_script:
+- C:\msys64\usr\bin\bash.exe -lc "where gcc"
+- C:\msys64\usr\bin\bash.exe -lc "gcc --version"
+- C:\msys64\usr\bin\bash.exe -lc "where perl"
+- C:\msys64\usr\bin\bash.exe -lc "perl --version"
+
+ configure_script:
+- C:\msys64\usr\bin\bash.exe -lc "mkdir %BUILD_DIR% &&
+ cd %BUILD_DIR% &&
+ %CIRRUS_WORKING_DIR%/configure
+--host=x86_64-w64-mingw32
Re: Checking pgwin32_is_junction() errors
On Thu, Mar 24, 2022 at 04:30:26PM +1300, Thomas Munro wrote: > I think it'd be better to add missing_ok and elevel parameters, > following existing patterns. Unfortunately, it can't use the generic > frontend logging to implement elevel in frontend code from its current > location, because pgport can't call pgcommon. For now I came up with > a kludge to work around that problem, but I don't like it, and would > need to come up with something better... The only barrier reason why elevel if needed is because of pg_wal in SyncDataDirectory() that cannot fail hard. I don't have a great idea here, except using a bits32 with some bitwise flags to control the behavior of the routine, aka something close to a MISSING_OK and a FAIL_HARD_ON_ERROR. This pattern exists already in some of the *Extended() routines. -- Michael signature.asc Description: PGP signature
Checking pgwin32_is_junction() errors
Hi,
The comment for pgwin32_is_junction() says "Assumes the file exists,
so will return false if it doesn't (since a nonexistent file is not a
junction)". In fact that's the behaviour for any kind of error, and
although we set errno in that case, no caller ever checks it.
I think it'd be better to add missing_ok and elevel parameters,
following existing patterns. Unfortunately, it can't use the generic
frontend logging to implement elevel in frontend code from its current
location, because pgport can't call pgcommon. For now I came up with
a kludge to work around that problem, but I don't like it, and would
need to come up with something better...
Sketch code attached.
From 8e0e51359c0191cf673c3ddc87a3262b13f530a4 Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Wed, 16 Mar 2022 23:05:39 +1300
Subject: [PATCH] Raise errors in pgwin32_is_junction().
XXX It's not very nice to use elevel like this in frontend code
---
src/backend/access/transam/xlog.c| 2 +-
src/backend/replication/basebackup.c | 4 ++--
src/backend/storage/file/fd.c| 2 +-
src/backend/utils/adt/misc.c | 2 +-
src/bin/pg_checksums/pg_checksums.c | 2 +-
src/bin/pg_rewind/file_ops.c | 2 +-
src/common/file_utils.c | 5 +++-
src/include/port.h | 2 +-
src/include/port/win32_port.h| 2 +-
src/port/dirmod.c| 36 +---
10 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4ac3871c74..819b05177d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8314,7 +8314,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
* directories directly under pg_tblspc, which would fail below.
*/
#ifdef WIN32
- if (!pgwin32_is_junction(fullpath))
+ if (!pgwin32_is_junction(fullpath, false, ERROR))
continue;
#else
if (get_dirent_type(fullpath, de, false, ERROR) != PGFILETYPE_LNK)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 6884cad2c0..4be9090445 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1352,7 +1352,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
#ifndef WIN32
S_ISLNK(statbuf.st_mode)
#else
- pgwin32_is_junction(pathbuf)
+ pgwin32_is_junction(pathbuf, true, ERROR)
#endif
)
{
@@ -1840,7 +1840,7 @@ convert_link_to_directory(const char *pathbuf, struct stat *statbuf)
#ifndef WIN32
if (S_ISLNK(statbuf->st_mode))
#else
- if (pgwin32_is_junction(pathbuf))
+ if (pgwin32_is_junction(pathbuf, true, ERROR))
#endif
statbuf->st_mode = S_IFDIR | pg_dir_create_mode;
}
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 14b77f2861..ec2f49fefa 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -3453,7 +3453,7 @@ SyncDataDirectory(void)
xlog_is_symlink = true;
}
#else
- if (pgwin32_is_junction("pg_wal"))
+ if (pgwin32_is_junction("pg_wal", false, LOG))
xlog_is_symlink = true;
#endif
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 89690be2ed..723504ab10 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -317,7 +317,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
* found, a relative path to the data directory is returned.
*/
#ifdef WIN32
- if (!pgwin32_is_junction(sourcepath))
+ if (!pgwin32_is_junction(sourcepath, false, ERROR))
PG_RETURN_TEXT_P(cstring_to_text(sourcepath));
#else
if (lstat(sourcepath, ) < 0)
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 5f0f5ee62d..5720907d33 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -404,7 +404,7 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly)
#ifndef WIN32
else if (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))
#else
- else if (S_ISDIR(st.st_mode) || pgwin32_is_junction(fn))
+ else if (S_ISDIR(st.st_mode) || pgwin32_is_junction(fn, false, 1))
#endif
{
/*
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 6cb288f099..1eda2baf2f 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -434,7 +434,7 @@ recurse_dir(const char *datadir, const char *parentpath,
#ifndef WIN32
else if (S_ISLNK(fst.st_mode))
#else
- else if (pgwin32_is_junction(fullpath))
+ else if (pgwin32_is_junction(fullpath, false, 1))
#endif
{
#if defined(HAVE_READLINK) || defined(WIN32)
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 7138068633..2db909a3aa 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -88,8 +88,11 @@ fsync_pgdata(const char *pg_data,
else if (S_ISLNK(st.st_mode))
xlog_is_symlink = true;
}
+#elif defined(FRONTEND)
