Re: Cleaning up historical portability baggage

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Cleaning up historical portability baggage
Date: 2022-07-25 15:09:59
Message-ID: 0f472be9-638d-24cf-193e-1d645315df83@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2022-07-25 Mo 10:35, Thomas Munro wrote:
> On Sun, Jul 24, 2022 at 12:23 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> After looking through these briefly, I'm pretty concerned about
>> whether this won't break our Cygwin build in significant ways.
>> For example, lorikeet reports "HAVE_SETSID 1", a condition that
>> you want to replace with !WIN32. The question here is whether
>> or not WIN32 is defined in a Cygwin build. I see some places
>> in our code that believe it is not, but others that believe that
>> it is --- and the former ones are mostly like
>> #if defined(__CYGWIN__) || defined(WIN32)
>> which means they wouldn't actually fail if they are wrong about that.
> I spent a large chunk of today figuring out how to build PostgreSQL
> under Cygwin/GCC on CI. My method for answering this question was to
> put the following on the end of 192 .c files that contain the pattern
> /#if.*WIN32/:
>
> +
> +#if defined(WIN32) && defined(__CYGWIN__)
> +#pragma message "contradiction"
> +#endif
>
> Only one of them printed that message: dirmod.c. The reason is that
> it goes out of its way to include Windows headers:
>
> #if defined(WIN32) || defined(__CYGWIN__)
> #ifndef __CYGWIN__
> #include <winioctl.h>
> #else
> #include <windows.h>
> #include <w32api/winioctl.h>
> #endif
> #endif
>
> The chain <windows.h> -> <windef.h> -> <minwindef.h> leads to WIN32 here:
>
> https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/include/minwindef.h#L15
>
> I'm left wondering if we should de-confuse matters by ripping out all
> the checks and comments that assume that this problem is more
> widespread, and then stick a big notice about it in dirmod.c, to
> contain this Jekyll/Hide situation safely inside about 8 feet of
> concrete.

Clearly it's something we've been aware of before, port.h has:

* Note: Some CYGWIN includes might #define WIN32.
 */
#if defined(WIN32) && !defined(__CYGWIN__)
#include "port/win32_port.h"
#endif

I can test any patches you want on lorikeet.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Junwang Zhao 2022-07-25 15:14:31 Re: [PATCH v1] strengthen backup history filename check
Previous Message Tom Lane 2022-07-25 15:08:55 Re: fairywren hung in pg_basebackup tests