Thread: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
Hi all, On the thread about the removal of VS 2013, Jose (in CC) has mentioned that bumping MIN_WINNT independently would make sense, as the simplication of locales would expose under MinGW some code for GetLocaleInfoEx(): https://www.postgresql.org/message-id/CAC+AXB3himFH+-pGRO1cYju6zF2hLH6VmwPbf5RAytF1UBm_nw@mail.gmail.com Attached is a patch to set MIN_WINNT, the minimal version of Windows allowed at run-time to 0x0600 for all environments, aka Vista. This results in removing the support for XP at run-time when compiling with anything else than VS >= 2015 (VS 2013, MinGW, etc.). We could cut things more, I hope, but this bump makes sense in itself with the business related to locales. What I would like to do is to apply that at the beginning of the dev cycle for v16, in parallel of the removal of VS 2013. This move is rather independent of the other thread, which is why I am spawning a new one here. And it is better than having to dig into the other thread for a change like that. Thoughts or opinions? -- Michael
Attachment
On Thu, May 26, 2022 at 2:59 PM Michael Paquier <michael@paquier.xyz> wrote: > On the thread about the removal of VS 2013, Jose (in CC) has mentioned > that bumping MIN_WINNT independently would make sense, as the > simplication of locales would expose under MinGW some code for > GetLocaleInfoEx(): > https://www.postgresql.org/message-id/CAC+AXB3himFH+-pGRO1cYju6zF2hLH6VmwPbf5RAytF1UBm_nw@mail.gmail.com > > Attached is a patch to set MIN_WINNT, the minimal version of Windows > allowed at run-time to 0x0600 for all environments, aka Vista. This > results in removing the support for XP at run-time when compiling with > anything else than VS >= 2015 (VS 2013, MinGW, etc.). We could cut > things more, I hope, but this bump makes sense in itself with the > business related to locales. > > What I would like to do is to apply that at the beginning of the dev > cycle for v16, in parallel of the removal of VS 2013. This move is > rather independent of the other thread, which is why I am spawning a > new one here. And it is better than having to dig into the other > thread for a change like that. > > Thoughts or opinions? I think we should drop everything older than Win 10 for PG16, as argued in various threads where various pain points came up. For one thing, that would make a lot of future work simpler (ie not needing to test alternative code paths on dead computers without CI or BF, AKA dead code), and also I don't think we really help anyone by allowing new database deployments on operating systems that aren't receiving vendor patches on the world's most attacked operating system. Doing it incrementally is fine by me, too, though, if it makes the patches and discussions easier...
On Thu, May 26, 2022 at 04:16:40PM +1200, Thomas Munro wrote: > I think we should drop everything older than Win 10 for PG16, as > argued in various threads where various pain points came up. For one > thing, that would make a lot of future work simpler (ie not needing to > test alternative code paths on dead computers without CI or BF, AKA > dead code), and also I don't think we really help anyone by allowing > new database deployments on operating systems that aren't receiving > vendor patches on the world's most attacked operating system. Doing > it incrementally is fine by me, too, though, if it makes the patches > and discussions easier... Is there anything posted recently that would require that? Perhaps the async work? FWIW, I agree to be much more aggressive, but there is nothing in the tree now that depends on _WIN32_WINNT, except one change for the locales. -- Michael
Attachment
Re: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
From
Juan José Santamaría Flecha
Date:
On Thu, May 26, 2022 at 6:27 AM Michael Paquier <michael@paquier.xyz> wrote:
Is there anything posted recently that would require that? Perhaps
the async work? FWIW, I agree to be much more aggressive, but there
is nothing in the tree now that depends on _WIN32_WINNT, except one
change for the locales.
There have been a couple of discussions involving not only Windows version10, but also the Release id:
Maybe this thread can push those others forward.
Regards,
Juan José Santamaría Flecha
On Thu, May 26, 2022 at 4:27 PM Michael Paquier <michael@paquier.xyz> wrote: > Perhaps the async work? (Checks code...) Looks like the experimental Windows native AIO code we have today, namely io_method=windows_iocp, only needs Vista. That's for GetQueueCompletionStatusEx() (before that you had to call GetQueuedCompletionStatus() in a loop to read multiple IO completion events from an IOCP), and otherwise it's all just ancient Windows "overlapped" stuff. Not sure if we'll propose that io_method, or skip directly to the new io_uring-style API that appeared in Win 11 (not yet tried), or propose both as long as Win 10 is around, or ... I dunno.
On Thu, May 26, 2022 at 10:17:08AM +0200, Juan José Santamaría Flecha wrote: > There have been a couple of discussions involving not only Windows > version10, but also the Release id: > > https://commitfest.postgresql.org/38/3347/ This mentions 0x0A00, aka Windows 10, for atomic rename support. > https://commitfest.postgresql.org/38/3530/ Similarly 0x0A00, aka Windows 10, for fdatasync(). > https://www.postgresql.org/message-id/6389b5a88e114bee85593af2853c08cd%40dental-vision.de And Windows 10 1703, for large pages. > Maybe this thread can push those others forward. Post Windows 8, the most annoying part is that manifests are required to be able to check at run-time on which version you are running with routines like IsWindowsXXOrGreater() if you compiled with a threshold of MIN_WINNT lower than the version you expect compatibility for. And each one of those things would mean cutting a lot of past support if we want to eliminate the manifest part. Windows 8 ends its support in 2023, it seems, so that sounds short even for PG16. -- Michael
Attachment
On Fri, May 27, 2022 at 3:53 PM Michael Paquier <michael@paquier.xyz> wrote: > Windows 8 ends its support > in 2023, it seems, so that sounds short even for PG16. I guess you meant 8.1 here, and corresponding server release 2012 R2. These will come to the end of their "extended" support phase in 2023, before PG16 comes out. If I understand correctly (and I'm not a Windows user, I just googled this), they will start showing blue full-screen danger-Will-Robinson alerts about viruses and malware. Why would we have explicit support for that in a new release? Do we want people putting their users' data in such a system? Can you go to GDPR jail for that in Europe? (Joking, I think). We should go full Marie Kondo on EOL'd OSes that are not in our CI or build farm, IMHO.
> On 27 May 2022, at 23:07, Thomas Munro <thomas.munro@gmail.com> wrote: > We should go full Marie Kondo on EOL'd OSes that are not in our CI or > build farm, IMHO. FWIW, +1 -- Daniel Gustafsson https://vmware.com/
On Sat, May 28, 2022 at 05:30:51PM +0200, Daniel Gustafsson wrote: > On 27 May 2022, at 23:07, Thomas Munro <thomas.munro@gmail.com> wrote: >> We should go full Marie Kondo on EOL'd OSes that are not in our CI or >> build farm, IMHO. > > FWIW, +1 Okay, I am outnumbered, and that would mean bumping MIN_WINNT to 0x0A00. So, ready to move to this version at full speed for 16? We still have a couple of weeks ahead before the next dev cycle begins, so feel free to comment, of course. -- Michael
Attachment
On Mon, May 30, 2022 at 03:59:52PM +0900, Michael Paquier wrote: > Okay, I am outnumbered, and that would mean bumping MIN_WINNT to > 0x0A00. So, ready to move to this version at full speed for 16? We > still have a couple of weeks ahead before the next dev cycle begins, > so feel free to comment, of course. And attached is an updated patch to do exactly that. -- Michael
Attachment
On Thu, Jun 9, 2022 at 3:55 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, May 30, 2022 at 03:59:52PM +0900, Michael Paquier wrote:
> > Okay, I am outnumbered, and that would mean bumping MIN_WINNT to
> > 0x0A00. So, ready to move to this version at full speed for 16? We
> > still have a couple of weeks ahead before the next dev cycle begins,
> > so feel free to comment, of course.
>
> And attached is an updated patch to do exactly that.
<productname>PostgreSQL</productname> can be expected to work on
these operating
- systems: Linux (all recent distributions), Windows (XP and later),
+ systems: Linux (all recent distributions), Windows (10 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Cool. (I'm not sure what "all recent distributions" contributes but
that's not from your patch...).
The Cygwin stuff in installation.sgml also mentions NT, 2000, XP, but
it's not clear from the phrasing if it meant "and later" or "and
earlier", so I'm not sure if it needs adjusting or removing...
While looking for more stuff to vacuum, I found this:
<title>Special Considerations for 64-Bit Windows</title>
<para>
PostgreSQL will only build for the x64 architecture on 64-bit Windows, there
is no support for Itanium processors.
</para>
I think we can drop mention of Itanium (RIP): the ancient versions of
Windows that could run on that arch are desupported with your patch.
It might be more relevant to say that we can't yet run on ARM, and
Windows 11 is untested by us, but let's fix those problems instead of
documenting them :-)
Is the mention of "64-Bit" in that title sounds a little anachronistic
to me (isn't that the norm now?) but I'm not sure what to suggest.
There are more mentions of older Windows releases near the compiler
stuff but perhaps your MSVC version vacuuming work will take care of
those.
On Thu, Jun 09, 2022 at 04:55:45PM +1200, Thomas Munro wrote: > The Cygwin stuff in installation.sgml also mentions NT, 2000, XP, but > it's not clear from the phrasing if it meant "and later" or "and > earlier", so I'm not sure if it needs adjusting or removing... Right. We could just remove the entire mention to "NT, 2000 or XP" instead? There would be no loss in clarity IMO. > I think we can drop mention of Itanium (RIP): the ancient versions of > Windows that could run on that arch are desupported with your patch. > It might be more relevant to say that we can't yet run on ARM, and > Windows 11 is untested by us, but let's fix those problems instead of > documenting them :-) Okay to remove the Itanium part for me. > Is the mention of "64-Bit" in that title sounds a little anachronistic > to me (isn't that the norm now?) but I'm not sure what to suggest. Not sure. I think that I would leave this part alone for now. > There are more mentions of older Windows releases near the compiler > stuff but perhaps your MSVC version vacuuming work will take care of > those. Yes, I have a few changes like the one in main.c for _M_AMD64. Are you referring to something else? -- Michael
Attachment
On Thu, Jun 09, 2022 at 02:47:36PM +0900, Michael Paquier wrote: > On Thu, Jun 09, 2022 at 04:55:45PM +1200, Thomas Munro wrote: >> I think we can drop mention of Itanium (RIP): the ancient versions of >> Windows that could run on that arch are desupported with your patch. >> It might be more relevant to say that we can't yet run on ARM, and >> Windows 11 is untested by us, but let's fix those problems instead of >> documenting them :-) > > Okay to remove the Itanium part for me. install-windows.sgml has one extra spot mentioning Windows 7 and Server 2008 that can be simplified on top of that. >> There are more mentions of older Windows releases near the compiler >> stuff but perhaps your MSVC version vacuuming work will take care of >> those. > > Yes, I have a few changes like the one in main.c for _M_AMD64. Are > you referring to something else? Actually, this can go with the bump of MIN_WINNT as it uses one of the IsWindows*OrGreater() macros as a runtime check. And there are two more places in pg_ctl.c that can be similarly cleaned up. It is possible that I have missed some spots, of course. -- Michael
Attachment
On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote: > Actually, this can go with the bump of MIN_WINNT as it uses one of the > IsWindows*OrGreater() macros as a runtime check. And there are two > more places in pg_ctl.c that can be similarly cleaned up. > > It is possible that I have missed some spots, of course. It does not seem to be the case on a second look. The buildfarm animals running Windows are made of: - hamerkop, Windows server 2016 (based on Win10 AFAIK) - drongo, Windows server 2019 - bowerbird, Windows 10 pro - jacana, Windows 10 - fairywren, Msys server 2019 - bichir, Ubuntu/Windows 10 mix Now that v16 is open for business, any objections to move on with this patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items for large pages and more. -- Michael
Attachment
On Wed, Jul 6, 2022 at 7:28 PM Michael Paquier <michael@paquier.xyz> wrote: > On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote: > > Actually, this can go with the bump of MIN_WINNT as it uses one of the > > IsWindows*OrGreater() macros as a runtime check. And there are two > > more places in pg_ctl.c that can be similarly cleaned up. > > > > It is possible that I have missed some spots, of course. > > It does not seem to be the case on a second look. The buildfarm > animals running Windows are made of: > - hamerkop, Windows server 2016 (based on Win10 AFAIK) > - drongo, Windows server 2019 > - bowerbird, Windows 10 pro > - jacana, Windows 10 > - fairywren, Msys server 2019 > - bichir, Ubuntu/Windows 10 mix > > Now that v16 is open for business, any objections to move on with this > patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items > for large pages and more. +1 for proceeding. This will hopefully unblock a few things, and it's good to update our claims to match the reality of what we are actually testing and able to debug. The build farm also has frogmouth and currawong, 32 bit systems running Windows XP, but they are only testing REL_10_STABLE so I assume Andrew will decommission them in November.
On 2022-07-06 We 16:46, Thomas Munro wrote: > On Wed, Jul 6, 2022 at 7:28 PM Michael Paquier <michael@paquier.xyz> wrote: >> On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote: >>> Actually, this can go with the bump of MIN_WINNT as it uses one of the >>> IsWindows*OrGreater() macros as a runtime check. And there are two >>> more places in pg_ctl.c that can be similarly cleaned up. >>> >>> It is possible that I have missed some spots, of course. >> It does not seem to be the case on a second look. The buildfarm >> animals running Windows are made of: >> - hamerkop, Windows server 2016 (based on Win10 AFAIK) >> - drongo, Windows server 2019 >> - bowerbird, Windows 10 pro >> - jacana, Windows 10 >> - fairywren, Msys server 2019 >> - bichir, Ubuntu/Windows 10 mix >> >> Now that v16 is open for business, any objections to move on with this >> patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items >> for large pages and more. > +1 for proceeding. This will hopefully unblock a few things, and it's > good to update our claims to match the reality of what we are actually > testing and able to debug. > > The build farm also has frogmouth and currawong, 32 bit systems > running Windows XP, but they are only testing REL_10_STABLE so I > assume Andrew will decommission them in November. Yeah, it's not capable of supporting anything newer, so it will finally go to sleep this year. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Wed, Jul 06, 2022 at 05:13:27PM -0400, Andrew Dunstan wrote: > On 2022-07-06 We 16:46, Thomas Munro wrote: >> The build farm also has frogmouth and currawong, 32 bit systems >> running Windows XP, but they are only testing REL_10_STABLE so I >> assume Andrew will decommission them in November. > > Yeah, it's not capable of supporting anything newer, so it will finally > go to sleep this year. Okay, thanks for confirming. I think that I'll give it a try today then, my schedule would fit nicely with the buildfarm monitoring. -- Michael
Attachment
On Thu, Jul 07, 2022 at 09:11:57AM +0900, Michael Paquier wrote: > Okay, thanks for confirming. I think that I'll give it a try today > then, my schedule would fit nicely with the buildfarm monitoring. And I have applied that, after noticing that the MinGW was complaining because _WIN32_WINNT was not getting set like previously and removing _WIN32_WINNT as there is no need for it anymore. The CI has reported green for all my tests, so I am rather confident to not have messed up something. Now, let's see what the buildfarm members tell. This should take a couple of hours.. -- Michael
Attachment
On Thu, Jul 07, 2022 at 01:56:39PM +0900, Michael Paquier wrote: > And I have applied that, after noticing that the MinGW was complaining > because _WIN32_WINNT was not getting set like previously and removing > _WIN32_WINNT as there is no need for it anymore. The CI has reported > green for all my tests, so I am rather confident to not have messed up > something. Now, let's see what the buildfarm members tell. This > should take a couple of hours.. Since this has been applied, all the Windows members have reported a green state except for jacana and bowerbird. Based on their environment, I would not expect any issues though I may be wrong. Andrew, is something happening on those environments? Is 495ed0e causing problems? -- Michael
Attachment
On Thu, Jul 07, 2022 at 01:56:39PM +0900, Michael Paquier wrote: > On Thu, Jul 07, 2022 at 09:11:57AM +0900, Michael Paquier wrote: > > Okay, thanks for confirming. I think that I'll give it a try today > > then, my schedule would fit nicely with the buildfarm monitoring. > > And I have applied that, after noticing that the MinGW was complaining > because _WIN32_WINNT was not getting set like previously and removing > _WIN32_WINNT as there is no need for it anymore. The CI has reported > green for all my tests, so I am rather confident to not have messed up > something. Now, let's see what the buildfarm members tell. This > should take a couple of hours.. If I'm not wrong, there's some lingering comments which could be removed since 495ed0ef2. src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4. src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the required functions, will src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have CreateRestrictedToken, so just call ordinary src/port/dirmod.c: * Win32 (NT4 and newer). src/backend/port/win32/socket.c: /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */ -- Justin
On Fri, Aug 26, 2022 at 06:26:37AM -0500, Justin Pryzby wrote: > If I'm not wrong, there's some lingering comments which could be removed since > 495ed0ef2. It seems to me that you are right. I have not thought about looking at references to NT. Good catches! > src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4. > src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the required functions, will > src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have CreateRestrictedToken, so just call ordinary > src/port/dirmod.c: * Win32 (NT4 and newer). > src/backend/port/win32/socket.c: /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */ There is also a reference to Nt4 in win32.c, for a comment that is irrelevant now, so it can be IMO removed. There may be a point in enforcing CreateProcess() if CreateRestrictedToken() cannot be loaded, but that would be a security issue if Windows goes crazy as we should always expect the function, so this had better return an error. So, what do you think about the attached? -- Michael
Attachment
On Sat, Aug 27, 2022 at 02:35:25PM +0900, Michael Paquier wrote: > There may be a point in enforcing CreateProcess() if > CreateRestrictedToken() cannot be loaded, but that would be a security > issue if Windows goes crazy as we should always expect the function, > so this had better return an error. And applied as of b1ec7f4. -- Michael
Attachment
On Tue, Aug 30, 2022 at 12:57 PM Michael Paquier <michael@paquier.xyz> wrote: > On Sat, Aug 27, 2022 at 02:35:25PM +0900, Michael Paquier wrote: > > There may be a point in enforcing CreateProcess() if > > CreateRestrictedToken() cannot be loaded, but that would be a security > > issue if Windows goes crazy as we should always expect the function, > > so this had better return an error. > > And applied as of b1ec7f4. This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with a direct function call. Can you just call all these functions directly these days?
On Tue, Aug 30, 2022 at 01:29:24PM +1200, Thomas Munro wrote: > This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with > a direct function call. Can you just call all these functions > directly these days? Hmm. Some tests in the CI show that attempting to call directly MiniDumpWriteDump() causes a linking failure at compilation. Anyway, in the same fashion, I can get some simplifications done right for pg_ctl.c, auth.c and restricted_token.c. And I am seeing all these functions listed in the headers of MinGW, meaning that all these should work out of the box in this case, no? The others are library-dependent, and I not really confident about ldap_start_tls_sA(). So, at the end, I am finishing with the attached, what do you think? This cuts some code, which is nice: 3 files changed, 48 insertions(+), 159 deletions(-) -- Michael
Attachment
On Thu, Sep 08, 2022 at 05:55:40PM +0900, Michael Paquier wrote: > On Tue, Aug 30, 2022 at 01:29:24PM +1200, Thomas Munro wrote: > > This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with > > a direct function call. Can you just call all these functions > > directly these days? > > Hmm. Some tests in the CI show that attempting to call directly > MiniDumpWriteDump() causes a linking failure at compilation. Anyway, > in the same fashion, I can get some simplifications done right for > pg_ctl.c, auth.c and restricted_token.c. And I am seeing all these > functions listed in the headers of MinGW, meaning that all these > should work out of the box in this case, no? > > The others are library-dependent, and I not really confident about > ldap_start_tls_sA(). So, at the end, I am finishing with the > attached, what do you think? This cuts some code, which is nice: > 3 files changed, 48 insertions(+), 159 deletions(-) +1 It seems silly to do it at runtime if it's possible to do it at link time. -- Justin
On Thu, Sep 08, 2022 at 08:29:20AM -0500, Justin Pryzby wrote: > It seems silly to do it at runtime if it's possible to do it at link time. Thanks. This set of simplifications is too good to let go, and I have a window to look after the buildfarm today and tomorrow, which should be enough to take action if need be. Hence, I have applied the patch. Now, let's see what the buildfarm tells us ;) -- Michael
Attachment
On Fri, Sep 09, 2022 at 10:55:55AM +0900, Michael Paquier wrote: > Thanks. This set of simplifications is too good to let go, and I have > a window to look after the buildfarm today and tomorrow, which should > be enough to take action if need be. Hence, I have applied the > patch. Now, let's see what the buildfarm tells us ;) Based on what I can see, the Windows animals seem to have digested 47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good. -- Michael
Attachment
On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote: > Based on what I can see, the Windows animals seem to have digested > 47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good. The last part that's worth adjusting is ldap_start_tls_sA(), which would lead to the attached simplification. The MinGW headers list this routine, so like the previous change I think that it should be safe for such builds. Looking at the buildfarm animals, bowerbird, jacana, fairywren, lorikeet and drongo disable ldap. hamerkop is the only member that provides coverage for it, still that's a MSVC build. The CI provides coverage for ldap as it is enabled by default and windows_build_config.pl does not tell otherwise, but with the existing animals we don't have ldap coverage under MinGW. Anyway, I'd like to apply the attached, and I don't quite see why it would not work after 47bd0b3 under MinGW. Any thoughts? -- Michael
Attachment
On Sun, Sep 11, 2022 at 09:28:54AM +0900, Michael Paquier wrote: > On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote: > > Based on what I can see, the Windows animals seem to have digested > > 47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good. > > The last part that's worth adjusting is ldap_start_tls_sA(), which > would lead to the attached simplification. The MinGW headers list > this routine, so like the previous change I think that it should be > safe for such builds. > > Looking at the buildfarm animals, bowerbird, jacana, fairywren, > lorikeet and drongo disable ldap. hamerkop is the only member that > provides coverage for it, still that's a MSVC build. > > The CI provides coverage for ldap as it is enabled by default and > windows_build_config.pl does not tell otherwise, but with the existing > animals we don't have ldap coverage under MinGW. > > Anyway, I'd like to apply the attached, and I don't quite see why it > would not work after 47bd0b3 under MinGW. Any thoughts? There's a CF entry to add it, and I launched it with your patch. (This is in a branch which already has that, and also does a few other things differently). https://cirrus-ci.com/task/6302833585684480 [02:07:57.497] checking whether to build with LDAP support... yes It compiles, which is probably all that matters, and eventually skips the test anyway. [02:23:18.209] [02:23:18] c:/cirrus/src/test/ldap/t/001_auth.pl .. skipped: ldap tests not supported on MSWin32 or dependenciesnot installed -- Justin
On Sat, Sep 10, 2022 at 10:39:19PM -0500, Justin Pryzby wrote: > There's a CF entry to add it, and I launched it with your patch. > (This is in a branch which already has that, and also does a few other > things differently). No need for a CF entry if you want to play with the tree. I have Cirrus enabled on my own fork of Postgres on github, and I saw the same result as you: https://github.com/michaelpq/postgres/ -- Michael
Attachment
On Sun, Sep 11, 2022 at 12:29 PM Michael Paquier <michael@paquier.xyz> wrote: > On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote: > > Based on what I can see, the Windows animals seem to have digested > > 47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good. Great, that's a lot of nice cleanup. > The last part that's worth adjusting is ldap_start_tls_sA(), which > would lead to the attached simplification. - if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS) + if ((r = ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS) When looking the function up it made sense to use the name ending in '...A', but when calling directly I think we shouldn't use the A suffix, we should let the <winldap.h> macros do that for us[1]. (I wondered for a moment if that would even make Windows and Unix code the same, but sadly not due to the extra NULL arguments.) [1] https://docs.microsoft.com/en-us/windows/win32/api/winldap/nf-winldap-ldap_start_tls_sa
On Mon, Sep 12, 2022 at 09:42:25AM +1200, Thomas Munro wrote: > When looking the function up it made sense to use the name ending in > '...A', but when calling directly I think we shouldn't use the A > suffix, we should let the <winldap.h> macros do that for us[1]. (I > wondered for a moment if that would even make Windows and Unix code > the same, but sadly not due to the extra NULL arguments.) Good idea, I did not noticed this part. This should work equally, so done this way and applied. I am keeping an eye on the buildfarm. -- Michael
Attachment
After 495ed0e, do these references to Windows SDK 8.1 still make sense? src/sgml/install-windows.sgml: as well as standalone Windows SDK releases 8.1a to 10. src/sgml/install-windows.sgml: <productname>Microsoft Windows SDK</productname> version 8.1a to 10 or
On Mon, May 15, 2023 at 02:08:32PM +1200, Thomas Munro wrote: > After 495ed0e, do these references to Windows SDK 8.1 still make sense? > > src/sgml/install-windows.sgml: as well as standalone Windows SDK > releases 8.1a to 10. > src/sgml/install-windows.sgml: <productname>Microsoft Windows > SDK</productname> version 8.1a to 10 or As listed on https://en.wikipedia.org/wiki/Microsoft_Windows_SDK, likely not. What do you think about the attached? -- Michael
Attachment
On Mon, May 15, 2023 at 2:57 PM Michael Paquier <michael@paquier.xyz> wrote: > On Mon, May 15, 2023 at 02:08:32PM +1200, Thomas Munro wrote: > > After 495ed0e, do these references to Windows SDK 8.1 still make sense? > As listed on https://en.wikipedia.org/wiki/Microsoft_Windows_SDK, > likely not. What do you think about the attached? LGTM.
On Mon, May 15, 2023 at 03:22:29PM +1200, Thomas Munro wrote: > LGTM. Thanks, fixed! -- Michael