Thank you,
In this variant:
1) renamed file win10.manifest to windows.manifest
2) renamed function pgrename_win10 to pgrename_windows_posix_semantics
3) Function pgrename returns result of pgrename_windows_posix_semantics
function and not contiue run old version of function.
4) Added call GetLastError() after error MultiByteToWideChar fuction.
+typedef struct _FILE_RENAME_INFO_VVS {
+ union {
+ BOOLEAN ReplaceIfExists; // FileRenameInfo
+ DWORD Flags; // FileRenameInfoEx
+ } DUMMYUNIONNAME;
+ HANDLE RootDirectory;
+ DWORD FileNameLength;
+ WCHAR FileName[MAX_PATH];
+} FILE_RENAME_INFO_VVS;
Why can't we use a system header[2] for this?
I have a dynamic memory allocation version in the first patch.
len = wcslen(to_w);
rename_info = (FILE_RENAME_INFO*)malloc(sizeof(FILE_RENAME_INFO) +
(len + 1) * sizeof(wchar_t));
rename_info->ReplaceIfExists = TRUE;
rename_info->Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS |
FILE_RENAME_FLAG_REPLACE_IF_EXISTS;
rename_info->RootDirectory = NULL;
rename_info->FileNameLength = len;
memcpy(rename_info->FileName, to_w, (len + 1) * sizeof(wchar_t));
Is this code better? Maybe there is another correct method?
I checked the pgrename_windows_posix_semantics() function on Windows 7.
It returns error 87: Parameter is incorrect. Hence, it is necessary to
check the Windows version and call the old pgrename function for old
Windows.
Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
07.09.2021 4:40, Thomas Munro пишет:
On Tue, Sep 7, 2021 at 5:44 AM Victor Spirin <v.spi...@postgrespro.ru> wrote:
I have changed the way I add the manifest to projects. I used the
AdditionalManifestFiles option for a VS project.
Hi Victor,
Thanks for working on this!
I wonder if POSIX-style rename is used automatically on recent
Windows, based on the new clue that DeleteFile() has started
defaulting to POSIX semantics[1] (maybe it would require ReplaceFile()
instead of MoveFileEx(), but I have no idea.) If so, one question is
whether we'd still want to do this explicit POSIX rename dance, or
whether we should just wait a bit longer for it to happen
automatically on all relevant systems (plus tweak to use ReplaceFile()
if necessary). If not, we might want to do what you're proposing
anyway, especially if ReplaceFile() is required, because its interface
is weird (it only works if the other file exists?). Hmm, that alone
would be a good reason to go with your plan regardless, and of course
it would be good to see this fixed everywhere ASAP.
We still have to answer that question for pgunlink(). I was
contemplating that over in that other thread, because unlink() ->
EACCES is blocking something I'm working on. I found a partial
solution to that that works even on old and non-NTFS systems, and I
was thinking that would be enough for now and we could just patiently
wait until automatic POSIX semantics to arrives on all relevant
systems as the real long term solution, so I didn't need to expend
energy doing an intermediate explicit POSIX-mode wrapper like what
you're proposing. But then it seems strange to make a different
choice about that for rename() and unlink(). So... do you think it
would make sense to extend your patch to cover unlink() too?
It would be great to have a tool in the tree that tests directory
entry semantics, called something like src/bin/pg_test_dirmod, so that
it becomes very clear when POSIX semantics are being used. It could
test various interesting unlink and rename scenarios through our
wrappers (concurrent file handles, creating a new file with the name
of the old one, unlinking the containing directory, ...). It could
run on the build farm animals, and we could even ask people to run it
when they report problems, to try to lift the fog of bizarro Windows
file system semantics.
How exactly does the function fail on a file system that doesn't
support the new POSIX semantics? Assuming there is something like
ENOSUPP to report "file system says no", do we want to keep trying
every time, or remember that it doesn't work? I guess the answer may
vary per mount point, which makes it hard to track when you only have
an fd...
If it fails because of a sharing violation, it seems strange that we
immediately fall back to the old code to do the traditional (and
horrible) sleep/retry loop. That means that in rare conditions we can
still get the old behaviour that leads to problems, just because of a
transient condition. Hmm. Would it make more sense to say: fall back
to the traditional behaviour only for ENOSUPP (if there is such a
thing), cope with transient sharing violations without giving up on
POSIX semantics, and report all other failures immediately?
I agree that the existing enum CHECKSUM_TYPE_NONE + friends should be
renamed to something less collision-prone and more consistent with the
name of the enum ("pg_checksum_type"), so I'd vote for adding a PG_
prefix, in a separate patch.
+ <Manifest>
+
<AdditionalManifestFiles>src/port/win10.manifest</AdditionalManifestFiles>
+ </Manifest>
I have no opinion on how you're supposed to test for OS versions, but
one trivial observation is that that file declares support for many
Windows releases, and I guess pretty soon you'll need to add 11, and
then we'll wonder why it says 10 in the file name. Would it be better
as "windows.manifest" or something?
+pgrename_win10(const char *from, const char *to)
Same thought on the name: this'll age badly. What about something
like pgrename_windows_posix_semantics?
+typedef struct _FILE_RENAME_INFO_VVS {
+ union {
+ BOOLEAN ReplaceIfExists; // FileRenameInfo
+ DWORD Flags; // FileRenameInfoEx
+ } DUMMYUNIONNAME;
+ HANDLE RootDirectory;
+ DWORD FileNameLength;
+ WCHAR FileName[MAX_PATH];
+} FILE_RENAME_INFO_VVS;
Why can't we use a system header[2] for this?
+ if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)from, -1,
(LPWSTR)from_w, MAX_PATH) == 0) return -1;
+ if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)to, -1,
(LPWSTR)rename_info.FileName, MAX_PATH) == 0) return -1;
Don't these need _dosmaperr(GetLastError())?
[1]
https://www.postgresql.org/message-id/20210905214437.y25j42yigwnbdvtg%40alap3.anarazel.de
[2]
https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_rename_info
diff --git a/src/include/common/checksum_helper.h
b/src/include/common/checksum_helper.h
index cac7570ea1..2d533c93a6 100644
--- a/src/include/common/checksum_helper.h
+++ b/src/include/common/checksum_helper.h
@@ -26,6 +26,13 @@
* MD5 here because any new that does need a cryptographically strong checksum
* should use something better.
*/
+
+ /*
+ * CHECKSUM_TYPE_NONE defined in the winioctl.h when _WIN32_WINNT >=
_WIN32_WINNT_WIN10
+ */
+#ifdef CHECKSUM_TYPE_NONE
+#undef CHECKSUM_TYPE_NONE
+#endif
typedef enum pg_checksum_type
{
CHECKSUM_TYPE_NONE,
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index d8ae49e22d..d91555f5c0 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -12,12 +12,13 @@
/*
* Make sure _WIN32_WINNT has the minimum required value.
* Leave a higher value in place. When building with at least Visual
- * Studio 2015 the minimum requirement is Windows Vista (0x0600) to
- * get support for GetLocaleInfoEx() with locales. For everything else
+ * Studio 2015 the minimum requirement is Windows 10 (0x0A00) to get support
for SetFileInformationByHandle.
+ * The minimum requirement is Windows Vista (0x0600) get support for
GetLocaleInfoEx() with locales.
+ * For everything else
* the minimum version is Windows XP (0x0501).
*/
#if defined(_MSC_VER) && _MSC_VER >= 1900
-#define MIN_WINNT 0x0600
+#define MIN_WINNT 0x0A00
#else
#define MIN_WINNT 0x0501
#endif
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 763bc5f915..0db3d16396 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -39,6 +39,89 @@
#endif
#endif
+
+#if defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) &&
_WIN32_WINNT >= _WIN32_WINNT_WIN10
+
+#include <versionhelpers.h>
+
+typedef struct _FILE_RENAME_INFO_VVS {
+ union {
+ BOOLEAN ReplaceIfExists; // FileRenameInfo
+ DWORD Flags; // FileRenameInfoEx
+ } DUMMYUNIONNAME;
+ HANDLE RootDirectory;
+ DWORD FileNameLength;
+ WCHAR FileName[MAX_PATH];
+} FILE_RENAME_INFO_VVS;
+
+/*
+ * pgrename_windows_posix_semantics - uses SetFileInformationByHandle
function with FILE_RENAME_FLAG_POSIX_SEMANTICS flag for atomic rename file
+ * working only on Windows 10 or later and _WIN32_WINNT must be >=
_WIN32_WINNT_WIN10
+ */
+static int
+pgrename_windows_posix_semantics(const char *from, const char *to)
+{
+ int err;
+ FILE_RENAME_INFO_VVS rename_info;
+ HANDLE f_handle;
+ wchar_t from_w[MAX_PATH];
+
+
+ if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)from, -1, (LPWSTR)from_w,
MAX_PATH) == 0) {
+ err = GetLastError();
+ _dosmaperr(err);
+ return -1;
+ }
+ if (MultiByteToWideChar(CP_ACP, 0, (LPCCH)to, -1,
(LPWSTR)rename_info.FileName, MAX_PATH) == 0) {
+ err = GetLastError();
+ _dosmaperr(err);
+ return -1;
+ }
+
+ f_handle = CreateFileW(from_w,
+ GENERIC_READ | GENERIC_WRITE | DELETE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ NULL,
+ OPEN_EXISTING,
+ FILE_ATTRIBUTE_NORMAL,
+ NULL);
+
+
+ if (f_handle == INVALID_HANDLE_VALUE)
+ {
+ err = GetLastError();
+
+ _dosmaperr(err);
+
+ return -1;
+ }
+
+
+ rename_info.ReplaceIfExists = TRUE;
+ rename_info.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS |
FILE_RENAME_FLAG_REPLACE_IF_EXISTS;
+
+ rename_info.RootDirectory = NULL;
+ rename_info.FileNameLength = wcslen(rename_info.FileName);;
+
+ if (!SetFileInformationByHandle(f_handle, FileRenameInfoEx,
&rename_info, sizeof(FILE_RENAME_INFO_VVS)))
+ {
+ err = GetLastError();
+
+ _dosmaperr(err);
+ CloseHandle(f_handle);
+
+ return -1;
+ }
+
+ CloseHandle(f_handle);
+
+ return 0;
+
+}
+
+#endif /* #if
defined(WIN32) && !defined(__CYGWIN__) && defined(_WIN32_WINNT_WIN10) &&
_WIN32_WINNT >= _WIN32_WINNT_WIN10 */
+
+
#if defined(WIN32) || defined(__CYGWIN__)
/*
@@ -49,6 +132,16 @@ pgrename(const char *from, const char *to)
{
int loops = 0;
+ /*
+ * Calls pgrename_windows_posix_semantics on Windows 10 and later when
_WIN32_WINNT >= _WIN32_WINNT_WIN10.
+ */
+#if defined(_WIN32_WINNT_WIN10) && _WIN32_WINNT >= _WIN32_WINNT_WIN10 &&
!defined(__CYGWIN__)
+ if (IsWindows10OrGreater()) {
+ return pgrename_windows_posix_semantics(from, to);
+ }
+#endif
+
+
/*
* We need to loop because even though PostgreSQL uses flags that allow
* rename while the file is open, other applications might have the file
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index ebe6530ba5..2ecb3c5638 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -378,6 +378,9 @@ EOF
<ResourceCompile>
<AdditionalIncludeDirectories>src\\include;\%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
</ResourceCompile>
+ <Manifest>
+
<AdditionalManifestFiles>src/port/windows.manifest</AdditionalManifestFiles>
+ </Manifest>
EOF
if ($self->{builddef})
{
diff --git a/src/port/windows.manifest b/src/port/windows.manifest
new file mode 100644
index 00000000000..fd3a344bee0
--- /dev/null
+++ b/src/port/windows.manifest
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1"
xmlns:asmv3="urn:schemas-microsoft-com:asm.v3" manifestVersion="1.0">
+
+ <!-- Enable use of version 6 of the common controls (Win XP and later) -->
+ <dependency>
+ <dependentAssembly>
+ <assemblyIdentity type="win32"
+ name="Microsoft.Windows.Common-Controls"
+ version="6.0.0.0"
+ processorArchitecture="*"
+ publicKeyToken="6595b64144ccf1df"
+ language="*" />
+ </dependentAssembly>
+ </dependency>
+
+ <!-- Indicate UAC compliance, with no need for elevated privileges (Win
Vista and later) -->
+ <!-- (if you need enhanced privileges, set the level to "highestAvailable"
or "requireAdministrator") -->
+ <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+ <security>
+ <requestedPrivileges>
+ <requestedExecutionLevel level="asInvoker" uiAccess="false" />
+ </requestedPrivileges>
+ </security>
+ </trustInfo>
+
+ <!-- Indicate high API awareness (Win Vista and later) -->
+ <!-- (if you support per-monitor high DPI, set this to "True/PM") -->
+ <application xmlns="urn:schemas-microsoft-com:asm.v3">
+ <windowsSettings>
+ <dpiAware
xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings";>false</dpiAware>
+ </windowsSettings>
+ </application>
+
+ <!-- Declare support for various versions of Windows -->
+ <ms_compatibility:compatibility
xmlns:ms_compatibility="urn:schemas-microsoft-com:compatibility.v1"
xmlns="urn:schemas-microsoft-com:compatibility.v1">
+ <ms_compatibility:application>
+ <!-- Windows Vista/Server 2008 -->
+ <ms_compatibility:supportedOS
Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}" />
+ <!-- Windows 7/Server 2008 R2 -->
+ <ms_compatibility:supportedOS
Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}" />
+ <!-- Windows 8/Server 2012 -->
+ <ms_compatibility:supportedOS
Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}" />
+ <!-- Windows 8.1/Server 2012 R2 -->
+ <ms_compatibility:supportedOS
Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}" />
+ <!-- Windows 10 -->
+ <ms_compatibility:supportedOS
Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}" />
+ </ms_compatibility:application>
+ </ms_compatibility:compatibility>
+
+</assembly>
