On 2022-07-25 Mo 10:35, Thomas Munro wrote: > On Sun, Jul 24, 2022 at 12:23 PM Tom Lane <t...@sss.pgh.pa.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
