Clean up some old cruft related to Windows

Started by Michael Paquierabout 6 years ago16 messages
#1Michael Paquier
michael@paquier.xyz
1 attachment(s)

Hi all,

As discussed here, there is in the tree a couple of things related to
past versions of Windows:
/messages/by-id/20191218021954.GE1836@paquier.xyz

So I have been looking at that more closely, and found more:
- MIN_WINNT can be removed from win32.h thanks to d9dd406 which has
added a requirement on C99 with Windows 7 as minimum platform
supported. (The issue mentioned previously.)
- pipe_read_line(), used when finding another binary for a given
installation via find_other_exec() has some special handling related
to Windows 2000 and older versions.
- When trying to load getaddrinfo(), we try to load it from
wship6.ddl, which was something needed in Windows 2000, but newer
Windows versions include it in ws2_32.dll.
- A portion of the docs still refer to Windows 98.

Thoughts?
--
Michael

Attachments:

win32-old-cleanup.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index bb2f7540b3..728d6badb5 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -9,27 +9,6 @@
 #define WIN32
 #endif
 
-/*
- * 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
- * the minimum version is Windows XP (0x0501).
- */
-#if defined(_MSC_VER) && _MSC_VER >= 1900
-#define MIN_WINNT 0x0600
-#else
-#define MIN_WINNT 0x0501
-#endif
-
-#if defined(_WIN32_WINNT) && _WIN32_WINNT < MIN_WINNT
-#undef _WIN32_WINNT
-#endif
-
-#ifndef _WIN32_WINNT
-#define _WIN32_WINNT MIN_WINNT
-#endif
-
 /*
  * We need to prevent <crtdefs.h> from defining a symbol conflicting with
  * our errcode() function.  Since it's likely to get included by standard
diff --git a/src/common/exec.c b/src/common/exec.c
index 92dc3134a1..3f038d9214 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -364,7 +364,6 @@ find_other_exec(const char *argv0, const char *target,
 static char *
 pipe_read_line(char *cmd, char *line, int maxsize)
 {
-#ifndef WIN32
 	FILE	   *pgver;
 
 	/* flush output buffers in case popen does not... */
@@ -393,130 +392,6 @@ pipe_read_line(char *cmd, char *line, int maxsize)
 		return NULL;
 
 	return line;
-#else							/* WIN32 */
-
-	SECURITY_ATTRIBUTES sattr;
-	HANDLE		childstdoutrd,
-				childstdoutwr,
-				childstdoutrddup;
-	PROCESS_INFORMATION pi;
-	STARTUPINFO si;
-	char	   *retval = NULL;
-
-	sattr.nLength = sizeof(SECURITY_ATTRIBUTES);
-	sattr.bInheritHandle = TRUE;
-	sattr.lpSecurityDescriptor = NULL;
-
-	if (!CreatePipe(&childstdoutrd, &childstdoutwr, &sattr, 0))
-		return NULL;
-
-	if (!DuplicateHandle(GetCurrentProcess(),
-						 childstdoutrd,
-						 GetCurrentProcess(),
-						 &childstdoutrddup,
-						 0,
-						 FALSE,
-						 DUPLICATE_SAME_ACCESS))
-	{
-		CloseHandle(childstdoutrd);
-		CloseHandle(childstdoutwr);
-		return NULL;
-	}
-
-	CloseHandle(childstdoutrd);
-
-	ZeroMemory(&pi, sizeof(pi));
-	ZeroMemory(&si, sizeof(si));
-	si.cb = sizeof(si);
-	si.dwFlags = STARTF_USESTDHANDLES;
-	si.hStdError = childstdoutwr;
-	si.hStdOutput = childstdoutwr;
-	si.hStdInput = INVALID_HANDLE_VALUE;
-
-	if (CreateProcess(NULL,
-					  cmd,
-					  NULL,
-					  NULL,
-					  TRUE,
-					  0,
-					  NULL,
-					  NULL,
-					  &si,
-					  &pi))
-	{
-		/* Successfully started the process */
-		char	   *lineptr;
-
-		ZeroMemory(line, maxsize);
-
-		/* Try to read at least one line from the pipe */
-		/* This may require more than one wait/read attempt */
-		for (lineptr = line; lineptr < line + maxsize - 1;)
-		{
-			DWORD		bytesread = 0;
-
-			/* Let's see if we can read */
-			if (WaitForSingleObject(childstdoutrddup, 10000) != WAIT_OBJECT_0)
-				break;			/* Timeout, but perhaps we got a line already */
-
-			if (!ReadFile(childstdoutrddup, lineptr, maxsize - (lineptr - line),
-						  &bytesread, NULL))
-				break;			/* Error, but perhaps we got a line already */
-
-			lineptr += strlen(lineptr);
-
-			if (!bytesread)
-				break;			/* EOF */
-
-			if (strchr(line, '\n'))
-				break;			/* One or more lines read */
-		}
-
-		if (lineptr != line)
-		{
-			/* OK, we read some data */
-			int			len;
-
-			/* If we got more than one line, cut off after the first \n */
-			lineptr = strchr(line, '\n');
-			if (lineptr)
-				*(lineptr + 1) = '\0';
-
-			len = strlen(line);
-
-			/*
-			 * If EOL is \r\n, convert to just \n. Because stdout is a
-			 * text-mode stream, the \n output by the child process is
-			 * received as \r\n, so we convert it to \n.  The server main.c
-			 * sets setvbuf(stdout, NULL, _IONBF, 0) which has the effect of
-			 * disabling \n to \r\n expansion for stdout.
-			 */
-			if (len >= 2 && line[len - 2] == '\r' && line[len - 1] == '\n')
-			{
-				line[len - 2] = '\n';
-				line[len - 1] = '\0';
-				len--;
-			}
-
-			/*
-			 * We emulate fgets() behaviour. So if there is no newline at the
-			 * end, we add one...
-			 */
-			if (len == 0 || line[len - 1] != '\n')
-				strcat(line, "\n");
-
-			retval = line;
-		}
-
-		CloseHandle(pi.hProcess);
-		CloseHandle(pi.hThread);
-	}
-
-	CloseHandle(childstdoutwr);
-	CloseHandle(childstdoutrddup);
-
-	return retval;
-#endif							/* WIN32 */
 }
 
 
diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c
index 9e5829b6dd..4dce138a09 100644
--- a/src/port/getaddrinfo.c
+++ b/src/port/getaddrinfo.c
@@ -69,10 +69,9 @@ haveNativeWindowsIPv6routines(void)
 		return (getaddrinfo_ptr != NULL);
 
 	/*
-	 * For Windows XP and Windows 2003 (and longhorn/vista), the IPv6 routines
-	 * are present in the WinSock 2 library (ws2_32.dll). Try that first
+	 * For Windows XP and later versions, the IPv6 routines are present in
+	 * the WinSock 2 library (ws2_32.dll).
 	 */
-
 	hLibrary = LoadLibraryA("ws2_32");
 
 	if (hLibrary == NULL || GetProcAddress(hLibrary, "getaddrinfo") == NULL)
@@ -83,13 +82,6 @@ haveNativeWindowsIPv6routines(void)
 		 */
 		if (hLibrary != NULL)
 			FreeLibrary(hLibrary);
-
-		/*
-		 * In Windows 2000, there was only the IPv6 Technology Preview look in
-		 * the IPv6 WinSock library (wship6.dll).
-		 */
-
-		hLibrary = LoadLibraryA("wship6");
 	}
 
 	/* If hLibrary is null, we couldn't find a dll with functions */
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 08556b6ebe..178228b152 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -42,13 +42,8 @@
   <productname>MinGW-w64</productname>. These tools can also be used to
   cross-compile for 32 bit and 64 bit <productname>Windows</productname>
   targets on other hosts, such as <productname>Linux</productname> and
-  <productname>macOS</productname>.
-  <productname>Cygwin</productname> is not recommended for running a
-  production server, and it should only be used for running on
-  older versions of <productname>Windows</productname> where
-  the native build does not work, such as
-  <productname>Windows 98</productname>. The official
-  binaries are built using <productname>Visual Studio</productname>.
+  <productname>macOS</productname>. The official binaries are built using
+  <productname>Visual Studio</productname>.
  </para>
 
  <para>
#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Michael Paquier (#1)
Re: Clean up some old cruft related to Windows

At Thu, 19 Dec 2019 11:15:26 +0900, Michael Paquier <michael@paquier.xyz> wrote in

Hi all,

As discussed here, there is in the tree a couple of things related to
past versions of Windows:
/messages/by-id/201912180219SUSv254.GE1836@paquier.xyz

So I have been looking at that more closely, and found more:
- MIN_WINNT can be removed from win32.h thanks to d9dd406 which has
added a requirement on C99 with Windows 7 as minimum platform
supported. (The issue mentioned previously.)
- pipe_read_line(), used when finding another binary for a given
installation via find_other_exec() has some special handling related
to Windows 2000 and older versions.
- When trying to load getaddrinfo(), we try to load it from
wship6.ddl, which was something needed in Windows 2000, but newer
Windows versions include it in ws2_32.dll.
- A portion of the docs still refer to Windows 98.

Thoughts?

I think MIN_WINNT is definitely emovable.

popen already has the plantform-dependent implement so I think it can
be removed irrelevantly for the C99 discussion.

I found some similar places by grep'ing for windows version names the
whole source tree.

- The comment for trapsig is mentioning win98/Me/NT/2000/XP.

- We don't need the (only) caller site of IsWindows7OrGreater()?

- The comment for AddUserToTokenDacl() is mentioning "XP/2K3,
Vista/2008".

- InitializeLDAPConnection dynamically loads WLDAP32.DLL for Windows
2000. It could either be statically loaded or could be left as it
is, but the comment seems to need a change in either case.

- The comment for IsoLocaleName mentioning Vista and Visual Studio
2012.

- install-windows.sgml is mentioning "XP and later" around line 117.

- installation.sgml is mentioning NT/2000/XP as platforms that don't
support adduser/su, command.

- "of Windows 2000 or later" is found at installation.sgml:2467

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#3Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Kyotaro Horiguchi (#2)
Re: Clean up some old cruft related to Windows

On Thu, Dec 19, 2019 at 5:47 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

At Thu, 19 Dec 2019 11:15:26 +0900, Michael Paquier <michael@paquier.xyz>
wrote in

Hi all,

As discussed here, there is in the tree a couple of things related to
past versions of Windows:

/messages/by-id/201912180219SUSv254.GE1836@paquier.xyz

So I have been looking at that more closely, and found more:
- MIN_WINNT can be removed from win32.h thanks to d9dd406 which has
added a requirement on C99 with Windows 7 as minimum platform
supported. (The issue mentioned previously.)
- pipe_read_line(), used when finding another binary for a given
installation via find_other_exec() has some special handling related
to Windows 2000 and older versions.
- When trying to load getaddrinfo(), we try to load it from
wship6.ddl, which was something needed in Windows 2000, but newer
Windows versions include it in ws2_32.dll.
- A portion of the docs still refer to Windows 98.

Thoughts?

I think MIN_WINNT is definitely emovable.

This is probably not an issue for the supported MSVC and their SDK, but
current MinGW defaults to Windows 2003 [1]https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/sdkddkver.h. So I would suggest a logic like:

#define WINNTVER(ver) ((ver) >> 16)
#define NTDDI_VERSION 0x06000100
#define _WIN32_WINNT WINNTVER(NTDDI_VERSION)

[1]: https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/sdkddkver.h
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/sdkddkver.h

Regards,

Juan José Santamaría Flecha

#4Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#3)
Re: Clean up some old cruft related to Windows

On Thu, Dec 19, 2019 at 08:09:45PM +0100, Juan José Santamaría Flecha wrote:

This is probably not an issue for the supported MSVC and their SDK, but
current MinGW defaults to Windows 2003 [1]. So I would suggest a logic like:

#define WINNTVER(ver) ((ver) >> 16)
#define NTDDI_VERSION 0x06000100
#define _WIN32_WINNT WINNTVER(NTDDI_VERSION)

[1]
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/sdkddkver.h

You're right, thanks for the pointer. This is this part of the
header:
#define NTDDI_VERSION NTDDI_WS03

Thinking more about that, the changes in win32.h are giving me cold
feet.
--
Michael

#5Michael Paquier
michael@paquier.xyz
In reply to: Kyotaro Horiguchi (#2)
1 attachment(s)
Re: Clean up some old cruft related to Windows

On Thu, Dec 19, 2019 at 01:46:33PM +0900, Kyotaro Horiguchi wrote:

I found some similar places by grep'ing for windows version names the
whole source tree.

- The comment for trapsig is mentioning win98/Me/NT/2000/XP.

Let's refresh the comment here, as per the following:
https://docs.microsoft.com/en-us/previous-versions/xdkz3x12(v=vs.140)

- We don't need the (only) caller site of IsWindows7OrGreater()?

The compiled code can still run with Windows Server 2008.

- The comment for AddUserToTokenDacl() is mentioning "XP/2K3,
Vista/2008".

Keeping some context is still good here IMO.

- InitializeLDAPConnection dynamically loads WLDAP32.DLL for Windows
2000. It could either be statically loaded or could be left as it
is, but the comment seems to need a change in either case.

Looks safer to me to keep it.

- The comment for IsoLocaleName mentioning Vista and Visual Studio
2012.

It is good to keep some history in this context.

- install-windows.sgml is mentioning "XP and later" around line 117.

But this still applies to XP, even if compilation is supported from
Windows 7.

- installation.sgml is mentioning NT/2000/XP as platforms that don't
support adduser/su, command.

No objections to simplify that a bit.

Attached is a simplified version. It is smaller than the previous
one, but that's already a good cut. I have also done some testing
with the service manager to check after pipe_read_line(), and that
works.

Thoughts?
--
Michael

Attachments:

win32-old-cleanup-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/common/exec.c b/src/common/exec.c
index 359442cf0c..9b20195ce1 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -364,7 +364,6 @@ find_other_exec(const char *argv0, const char *target,
 static char *
 pipe_read_line(char *cmd, char *line, int maxsize)
 {
-#ifndef WIN32
 	FILE	   *pgver;
 
 	/* flush output buffers in case popen does not... */
@@ -393,130 +392,6 @@ pipe_read_line(char *cmd, char *line, int maxsize)
 		return NULL;
 
 	return line;
-#else							/* WIN32 */
-
-	SECURITY_ATTRIBUTES sattr;
-	HANDLE		childstdoutrd,
-				childstdoutwr,
-				childstdoutrddup;
-	PROCESS_INFORMATION pi;
-	STARTUPINFO si;
-	char	   *retval = NULL;
-
-	sattr.nLength = sizeof(SECURITY_ATTRIBUTES);
-	sattr.bInheritHandle = TRUE;
-	sattr.lpSecurityDescriptor = NULL;
-
-	if (!CreatePipe(&childstdoutrd, &childstdoutwr, &sattr, 0))
-		return NULL;
-
-	if (!DuplicateHandle(GetCurrentProcess(),
-						 childstdoutrd,
-						 GetCurrentProcess(),
-						 &childstdoutrddup,
-						 0,
-						 FALSE,
-						 DUPLICATE_SAME_ACCESS))
-	{
-		CloseHandle(childstdoutrd);
-		CloseHandle(childstdoutwr);
-		return NULL;
-	}
-
-	CloseHandle(childstdoutrd);
-
-	ZeroMemory(&pi, sizeof(pi));
-	ZeroMemory(&si, sizeof(si));
-	si.cb = sizeof(si);
-	si.dwFlags = STARTF_USESTDHANDLES;
-	si.hStdError = childstdoutwr;
-	si.hStdOutput = childstdoutwr;
-	si.hStdInput = INVALID_HANDLE_VALUE;
-
-	if (CreateProcess(NULL,
-					  cmd,
-					  NULL,
-					  NULL,
-					  TRUE,
-					  0,
-					  NULL,
-					  NULL,
-					  &si,
-					  &pi))
-	{
-		/* Successfully started the process */
-		char	   *lineptr;
-
-		ZeroMemory(line, maxsize);
-
-		/* Try to read at least one line from the pipe */
-		/* This may require more than one wait/read attempt */
-		for (lineptr = line; lineptr < line + maxsize - 1;)
-		{
-			DWORD		bytesread = 0;
-
-			/* Let's see if we can read */
-			if (WaitForSingleObject(childstdoutrddup, 10000) != WAIT_OBJECT_0)
-				break;			/* Timeout, but perhaps we got a line already */
-
-			if (!ReadFile(childstdoutrddup, lineptr, maxsize - (lineptr - line),
-						  &bytesread, NULL))
-				break;			/* Error, but perhaps we got a line already */
-
-			lineptr += strlen(lineptr);
-
-			if (!bytesread)
-				break;			/* EOF */
-
-			if (strchr(line, '\n'))
-				break;			/* One or more lines read */
-		}
-
-		if (lineptr != line)
-		{
-			/* OK, we read some data */
-			int			len;
-
-			/* If we got more than one line, cut off after the first \n */
-			lineptr = strchr(line, '\n');
-			if (lineptr)
-				*(lineptr + 1) = '\0';
-
-			len = strlen(line);
-
-			/*
-			 * If EOL is \r\n, convert to just \n. Because stdout is a
-			 * text-mode stream, the \n output by the child process is
-			 * received as \r\n, so we convert it to \n.  The server main.c
-			 * sets setvbuf(stdout, NULL, _IONBF, 0) which has the effect of
-			 * disabling \n to \r\n expansion for stdout.
-			 */
-			if (len >= 2 && line[len - 2] == '\r' && line[len - 1] == '\n')
-			{
-				line[len - 2] = '\n';
-				line[len - 1] = '\0';
-				len--;
-			}
-
-			/*
-			 * We emulate fgets() behaviour. So if there is no newline at the
-			 * end, we add one...
-			 */
-			if (len == 0 || line[len - 1] != '\n')
-				strcat(line, "\n");
-
-			retval = line;
-		}
-
-		CloseHandle(pi.hProcess);
-		CloseHandle(pi.hThread);
-	}
-
-	CloseHandle(childstdoutwr);
-	CloseHandle(childstdoutrddup);
-
-	return retval;
-#endif							/* WIN32 */
 }
 
 
diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c
index b8b7827356..3892fafb8c 100644
--- a/src/port/getaddrinfo.c
+++ b/src/port/getaddrinfo.c
@@ -69,10 +69,9 @@ haveNativeWindowsIPv6routines(void)
 		return (getaddrinfo_ptr != NULL);
 
 	/*
-	 * For Windows XP and Windows 2003 (and longhorn/vista), the IPv6 routines
-	 * are present in the WinSock 2 library (ws2_32.dll). Try that first
+	 * For Windows XP and later versions, the IPv6 routines are present in
+	 * the WinSock 2 library (ws2_32.dll).
 	 */
-
 	hLibrary = LoadLibraryA("ws2_32");
 
 	if (hLibrary == NULL || GetProcAddress(hLibrary, "getaddrinfo") == NULL)
@@ -83,13 +82,6 @@ haveNativeWindowsIPv6routines(void)
 		 */
 		if (hLibrary != NULL)
 			FreeLibrary(hLibrary);
-
-		/*
-		 * In Windows 2000, there was only the IPv6 Technology Preview look in
-		 * the IPv6 WinSock library (wship6.dll).
-		 */
-
-		hLibrary = LoadLibraryA("wship6");
 	}
 
 	/* If hLibrary is null, we couldn't find a dll with functions */
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 7f1534aebb..bb6656e406 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2016,11 +2016,10 @@ make_postgres(FILE *cmdfd)
  * exit() directly.
  *
  * Also note the behaviour of Windows with SIGINT, which says this:
- *	 Note	SIGINT is not supported for any Win32 application, including
- *	 Windows 98/Me and Windows NT/2000/XP. When a CTRL+C interrupt occurs,
- *	 Win32 operating systems generate a new thread to specifically handle
- *	 that interrupt. This can cause a single-thread application such as UNIX,
- *	 to become multithreaded, resulting in unexpected behavior.
+ *  SIGINT is not supported for any Win32 application. When a CTRL+C interrupt
+ *  occurs, Win32 operating systems generate a new thread to specifically
+ *  handle that interrupt. This can cause a single-thread application, such as
+ *  one in UNIX, to become multithreaded and cause unexpected behavior.
  *
  * I have no idea how to handle this. (Strange they call UNIX an application!)
  * So this will need some testing on Windows.
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 08556b6ebe..e492d8957c 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -46,8 +46,7 @@
   <productname>Cygwin</productname> is not recommended for running a
   production server, and it should only be used for running on
   older versions of <productname>Windows</productname> where
-  the native build does not work, such as
-  <productname>Windows 98</productname>. The official
+  the native build does not work. The official
   binaries are built using <productname>Visual Studio</productname>.
  </para>
 
#6Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Michael Paquier (#5)
Re: Clean up some old cruft related to Windows

Hello.

I understand that this is not for back-patching.

At Tue, 18 Feb 2020 16:44:59 +0900, Michael Paquier <michael@paquier.xyz> wrote in

On Thu, Dec 19, 2019 at 01:46:33PM +0900, Kyotaro Horiguchi wrote:

I found some similar places by grep'ing for windows version names the
whole source tree.

- The comment for trapsig is mentioning win98/Me/NT/2000/XP.

Let's refresh the comment here, as per the following:
https://docs.microsoft.com/en-us/previous-versions/xdkz3x12(v=vs.140)

  * The Windows runtime docs at
  * http://msdn.microsoft.com/library/en-us/vclib/html/_crt_signal.asp
...
- *	 Win32 operating systems generate a new thread to specifically handle
- *	 that interrupt. This can cause a single-thread application such as UNIX,
- *	 to become multithreaded, resulting in unexpected behavior.
+ *  SIGINT is not supported for any Win32 application. When a CTRL+C interrupt
+ *  occurs, Win32 operating systems generate a new thread to specifically
+ *  handle that interrupt. This can cause a single-thread application, such as
+ *  one in UNIX, to become multithreaded and cause unexpected behavior.
  *
  * I have no idea how to handle this. (Strange they call UNIX an application!)
  * So this will need some testing on Windows.

The unmodified section just above is griping that "Strange they call
UNIX an application". The expression "application such as UNIX" seems
corresponding to the gripe. I tried to find the soruce of the phrase
but the above URL (.._crt_signal.asp) sent me "We're sorry, the page
you requested cannot be found.":(

Thank you for checking the belows.

- We don't need the (only) caller site of IsWindows7OrGreater()?

The compiled code can still run with Windows Server 2008.

Do we let the new PG version for already-unsupported platforms? If I
don't missing anything Windows Server 2008 is already
End-Of-Extended-Support (2020/1/14) along with Windows 7.

- The comment for AddUserToTokenDacl() is mentioning "XP/2K3,
Vista/2008".

Keeping some context is still good here IMO.

I'm fine with that.

- InitializeLDAPConnection dynamically loads WLDAP32.DLL for Windows
2000. It could either be statically loaded or could be left as it
is, but the comment seems to need a change in either case.

Looks safer to me to keep it.

If it is still possible that the file is missing on Windows 8/ Server
2012 or later, the comment should be updatd accordingly.

- The comment for IsoLocaleName mentioning Vista and Visual Studio
2012.

It is good to keep some history in this context.

Agreed.

- install-windows.sgml is mentioning "XP and later" around line 117.

But this still applies to XP, even if compilation is supported from
Windows 7.

Hmm. "/xp" can be the reason to preserve it.

By the way that pharse is considering Windows environment and perhaps
cmd.exe. So the folloinwg description:

https://www.postgresql.org/docs/current/install-windows-full.html

In recent SDK versions you can change the targeted CPU architecture,
build type, and target OS by using the setenv command, e.g. setenv
/x86 /release /xp to target Windows XP or later with a 32-bit
release build. See /? for other options to setenv. All commands
should be run from the src\tools\msvc directory.

AFAICS we cannot use "setenv command" on cmd.exe, or no such command
found in the msvc directory.

- installation.sgml is mentioning NT/2000/XP as platforms that don't
support adduser/su, command.

No objections to simplify that a bit.

Sorry for the ambiguity. I meant the following part

installation.sgml

<para>
<productname>PostgreSQL</productname> can be expected to work on these operating
systems: Linux (all recent distributions), Windows (Win2000 SP4 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Other Unix-like systems may also work but are not currently
being tested. In most cases, all CPU architectures supported by

(The coming version of) PostgreSQL doesn't support Win2000 SP4.

Attached is a simplified version. It is smaller than the previous
one, but that's already a good cut. I have also done some testing
with the service manager to check after pipe_read_line(), and that
works.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#7Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#4)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 7:54 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Dec 19, 2019 at 08:09:45PM +0100, Juan José Santamaría Flecha
wrote:

This is probably not an issue for the supported MSVC and their SDK, but
current MinGW defaults to Windows 2003 [1]. So I would suggest a logic

like:

#define WINNTVER(ver) ((ver) >> 16)
#define NTDDI_VERSION 0x06000100
#define _WIN32_WINNT WINNTVER(NTDDI_VERSION)

[1]

https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/sdkddkver.h

You're right, thanks for the pointer. This is this part of the
header:
#define NTDDI_VERSION NTDDI_WS03

Thinking more about that, the changes in win32.h are giving me cold
feet.

Maybe this needs a specific thread, as it is not quite cruft but something
that will require maintenance.

Regards,

Juan José Santamaría Flecha

#8Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Kyotaro Horiguchi (#6)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 9:56 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

https://www.postgresql.org/docs/current/install-windows-full.html

In recent SDK versions you can change the targeted CPU architecture,
build type, and target OS by using the setenv command, e.g. setenv
/x86 /release /xp to target Windows XP or later with a 32-bit
release build. See /? for other options to setenv. All commands
should be run from the src\tools\msvc directory.

AFAICS we cannot use "setenv command" on cmd.exe, or no such command
found in the msvc directory.

I cannot point when SetEnv.bat was exactly dropped, probably Windows 7 SDK
was the place where it was included [1]https://docs.microsoft.com/en-us/previous-versions/visualstudio/windows-sdk/ff660764(v=vs.100)?redirectedfrom=MSDN, so that needs to be updated.

Using VS2019 and VS2017 this would be done using VsDevCmd.bat [2]https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/how-to-set-environment-variables-for-the-visual-studio-command-line, while
VS2015 and VS2013 use VSVARS32.bat.

[1]: https://docs.microsoft.com/en-us/previous-versions/visualstudio/windows-sdk/ff660764(v=vs.100)?redirectedfrom=MSDN
https://docs.microsoft.com/en-us/previous-versions/visualstudio/windows-sdk/ff660764(v=vs.100)?redirectedfrom=MSDN
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/how-to-set-environment-variables-for-the-visual-studio-command-line
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/how-to-set-environment-variables-for-the-visual-studio-command-line

Regards,

Juan José Santamaría Flecha

#9Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Juan José Santamaría Flecha (#8)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 12:26 PM Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

[Edit] ... probably Windows 7 SDK was the *last* place where it was

included [1]...

Show quoted text

Regards,

Juan José Santamaría Flecha

#10Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#7)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 12:05:42PM +0100, Juan José Santamaría Flecha wrote:

Maybe this needs a specific thread, as it is not quite cruft but something
that will require maintenance.

Makes sense. I have discarded that part for now.
--
Michael

#11Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#8)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 12:26:06PM +0100, Juan José Santamaría Flecha wrote:

I cannot point when SetEnv.bat was exactly dropped, probably Windows 7 SDK
was the place where it was included [1], so that needs to be updated.

Using VS2019 and VS2017 this would be done using VsDevCmd.bat [2], while
VS2015 and VS2013 use VSVARS32.bat.

Would you like to write a patch for that part?
--
Michael

#12Michael Paquier
michael@paquier.xyz
In reply to: Kyotaro Horiguchi (#6)
Re: Clean up some old cruft related to Windows

On Tue, Feb 18, 2020 at 05:54:43PM +0900, Kyotaro Horiguchi wrote:

I understand that this is not for back-patching.

Cleanups don't go to back-branches.

At Tue, 18 Feb 2020 16:44:59 +0900, Michael Paquier <michael@paquier.xyz> wrote in
The unmodified section just above is griping that "Strange they call
UNIX an application". The expression "application such as UNIX" seems
corresponding to the gripe. I tried to find the soruce of the phrase
but the above URL (.._crt_signal.asp) sent me "We're sorry, the page
you requested cannot be found.":(

Yes, we should use that instead:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal

Do we let the new PG version for already-unsupported platforms? If I
don't missing anything Windows Server 2008 is already
End-Of-Extended-Support (2020/1/14) along with Windows 7.

Windows is known for keeping things backward compatible, so I don't
see any reason to not allow Postgres to run on those versions.
Outdated of course, still they could be used at runtime even if they
cannot compile the code.

By the way that pharse is considering Windows environment and perhaps
cmd.exe. So the folloinwg description:

https://www.postgresql.org/docs/current/install-windows-full.html

Let's tackle that as a separate patch as this is MSVC-dependent.

<para>
<productname>PostgreSQL</productname> can be expected to work on these operating
systems: Linux (all recent distributions), Windows (Win2000 SP4 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Other Unix-like systems may also work but are not currently
being tested. In most cases, all CPU architectures supported by

(The coming version of) PostgreSQL doesn't support Win2000 SP4.

Right, per the change for src/common/exec.c. I am wondering though if
we don't have more portability issues if we try to run Postgres on
something older than XP as there has been many changes in the last
couple of years, and we have no more buildfarm members that old.
Anyway, that's not worth the cost. For now I have applied to the tree
the smaller version as that's still a good cut.
--
Michael

#13Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#11)
1 attachment(s)
Re: Clean up some old cruft related to Windows

On Wed, Feb 19, 2020 at 4:49 AM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Feb 18, 2020 at 12:26:06PM +0100, Juan José Santamaría Flecha
wrote:

I cannot point when SetEnv.bat was exactly dropped, probably Windows 7

SDK

was the place where it was *last* included [1], so that needs to be

updated.

Using VS2019 and VS2017 this would be done using VsDevCmd.bat [2], while
VS2015 and VS2013 use VSVARS32.bat.

Would you like to write a patch for that part?

Please find a patched for so. I have tried to make it more version neutral.

Regards,

Juan José Santamaría Flecha

Attachments:

install-windows_doc_cleanup.patchapplication/octet-stream; name=install-windows_doc_cleanup.patchDownload
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index e492d89..7b60bfd 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -111,12 +111,11 @@
   the command, and vice versa.
   In the <productname>Microsoft Windows SDK</productname>, start the
   <application>CMD shell</application> listed under the SDK on the Start Menu.
-  In recent SDK versions you can change the targeted CPU architecture, build
-  type, and target OS by using the <command>setenv</command> command, e.g.
-  <command>setenv /x86 /release /xp</command> to target Windows XP or later
-  with a 32-bit release build. See <command>/?</command> for other options to
-  <command>setenv</command>.  All commands should be run from the
-  <filename>src\tools\msvc</filename> directory.
+  You can change certain build options, such as the targeted CPU
+  architecture, build type, and the selection of the SDK by using either
+  <command>VSVARS32.bat</command> or <command>VsDevCmd.bat</command> depending
+  on your <productname>Visual Studio</productname> release. All commands
+  should be run from the <filename>src\tools\msvc</filename> directory.
  </para>
 
  <para>
#14Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#13)
Re: Clean up some old cruft related to Windows

On Wed, Feb 19, 2020 at 07:02:55PM +0100, Juan José Santamaría Flecha wrote:

Please find a patched for so. I have tried to make it more version
neutral.

+  You can change certain build options, such as the targeted CPU
+  architecture, build type, and the selection of the SDK by using either
+  <command>VSVARS32.bat</command> or <command>VsDevCmd.bat</command> depending
+  on your <productname>Visual Studio</productname> release. All commands
+  should be run from the <filename>src\tools\msvc</filename> directory.

Both commands have different ways of doing things, and don't really
shine with their documentation, so it could save time to the reader to
add more explicit details of how to switch to the 32-bit mode, like
with "VsDevCmd -arch=x86". And I am not actually sure which
environment variable to touch when using VSVARS32.bat or
VSVARSALL.bat with MSVC <= 2017.
--
Michael

#15Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#14)
1 attachment(s)
Re: Clean up some old cruft related to Windows

On Thu, Feb 20, 2020 at 4:23 AM Michael Paquier <michael@paquier.xyz> wrote:

+  You can change certain build options, such as the targeted CPU
+  architecture, build type, and the selection of the SDK by using either
+  <command>VSVARS32.bat</command> or <command>VsDevCmd.bat</command>
depending
+  on your <productname>Visual Studio</productname> release. All commands
+  should be run from the <filename>src\tools\msvc</filename> directory.

I think more parts of this paragraph need tuning, like:

"In Visual Studio, start the Visual Studio Command Prompt. If you wish to
build a 64-bit version, you must use the 64-bit version of the command, and
vice versa."

This is what VsDevCmd.bat does, seting up the Visual Studio Command Prompt,
but from the command-line.

Also the following:

"In the Microsoft Windows SDK, start the CMD shell listed under the SDK on
the Start Menu."

This is not the case, you would be working in the CMD setup previously from
Visual Studio.

And I am not actually sure which
environment variable to touch when using VSVARS32.bat or
VSVARSALL.bat with MSVC <= 2017.

Actually, you can still use the vcvars% scripts to configure architecture,
platform_type and winsdk_version with current VS [1]https://docs.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=vs-2019.

Both commands have different ways of doing things, and don't really

shine with their documentation

I hear you.

Please find attached a new version that addresses these issues.

[1]: https://docs.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=vs-2019
https://docs.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=vs-2019

Regards,

Juan José Santamaría Flecha

Attachments:

install-windows_doc_cleanup-v2.patchapplication/octet-stream; name=install-windows_doc_cleanup-v2.patchDownload
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index e492d89..ea35de3 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -109,13 +109,15 @@
   <application>Visual Studio Command Prompt</application>.
   If you wish to build a 64-bit version, you must use the 64-bit version of
   the command, and vice versa.
-  In the <productname>Microsoft Windows SDK</productname>, start the
-  <application>CMD shell</application> listed under the SDK on the Start Menu.
-  In recent SDK versions you can change the targeted CPU architecture, build
-  type, and target OS by using the <command>setenv</command> command, e.g.
-  <command>setenv /x86 /release /xp</command> to target Windows XP or later
-  with a 32-bit release build. See <command>/?</command> for other options to
-  <command>setenv</command>.  All commands should be run from the
+  Starting with <productname>Visual Studio 2017</productname> this can be done from the
+  command line using <command>VsDevCmd.bat</command>, see <command>-help</command>
+  for the available options and their default values.
+  From the <application>Visual Studio Command Prompt</application> you can
+  change the targeted CPU architecture, build type, and target OS by using the
+  <command>vcvarsall.bat</command> command, e.g.
+  <command>vcvarsall.bat x64 10.0.10240.0</command> to target Windows 10
+  with a 64-bit release build. See <command>-help</command> for other options to
+  <command>vcvarsall.bat</command>.  All commands should be run from the
   <filename>src\tools\msvc</filename> directory.
  </para>
 
#16Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#15)
Re: Clean up some old cruft related to Windows

On Thu, Feb 20, 2020 at 12:39:56PM +0100, Juan José Santamaría Flecha wrote:

Actually, you can still use the vcvars% scripts to configure architecture,
platform_type and winsdk_version with current VS [1].

We still support the build down to MSVC 2013, so I think that it is
good to mention the options available for 2013 and 2015 as well, as
noted here:
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/how-to-set-environment-variables-for-the-visual-studio-command-line
"Visual Studio 2015 and earlier versions used VSVARS32.bat, not
VsDevCmd.bat for the same purpose."

Please find attached a new version that addresses these issues.

[1]
https://docs.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=vs-2019

Thanks, applied after tweaking the text a bit. I have applied that
down to 12 where we support MSVC from 2013.
--
Michael