Cygwin cleanup

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Cygwin cleanup
Date: 2022-07-26 04:24:25
Message-ID: CA+hUKG+e13wK0PBX5Z63CCwWm7MfRQuwBRabM_3aKWSko2AUww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Continuing a discussion started over at [1]. Moving this to a new
thread so that other thread can focus on Unix cleanup, and both
threads can get CI coverage...

1. In a few places, it is alleged that both __CYGWIN__ and WIN32
might be defined at the same time. Do you think we should try to get
rid of that possibility? I understand that we have to have a few
tests for __CYGWIN__ here and there, because eg file permissions don't
work quite right and there's not much we can do about that. But it
seems a bit unhelpful if we also have to worry about a more-or-less
POSIX-ish build taking WIN32 paths at uncertain times if we forget to
defend against that, or wonder why some places are not consistent.

A quick recap of the three flavours of Windows platform we have to
handle, as I understand it:

* MSVC: Windowsy toolchain, Windowsy C
* custom perl scripts instead of configure
* msbuild instead of make
* MSVC compiler
* Windows C APIs
* we provide our own emulation of some POSIX C APIs on top
* MSYS: Unixy toolchain, Windowsy C
* configure (portname = "win32")
* make
* GCC compiler
* Windows C APIs
* we provide our own emulation of some POSIX C APIs on top
* Cygwin: Unixy toolchain, Unixy C
* configure (portname = "cygwin")
* make
* GCC compiler
* POSIX C APIs (emulations provided by the Cygwin runtime libraries)

(The configure/make part will be harmonised by the Meson project.)

The macro WIN32 is visibly defined by something in/near msbuild in
MSVC builds: /D WIN32 is right here in the build transcripts (whereas
the compiler defines _WIN32; good compiler). I am not sure how
exactly it is first defined in MSYS builds; I suspect that MSYS gcc
might define it itself, but I don't have access to MSYS to check. As
for Cygwin, the only translation unit where I could find both
__CYGWIN__ and WIN32 defined is dirmod.c, and that results from
including <windows.h> and ultimately <minwindef.h> (even though WIN32
isn't defined yet at that time). I couldn't understand why we do
that, but I probably didn't read enough commit history. The purpose
of dirmod.c on Cygwin today is only to wrap otherwise pure POSIX code
in retry loops to handle those spurious EACCES errors due to NT
sharing violations, so there is no need for that.

Proposal: let's make it a programming rule that we don't allow
definitions from Windows headers to leak into Cygwin translation
units, preferably by never including them, or if we really must, let's
grant specific exemptions in an isolated and well documented way. We
don't seem to need any such exemptions currently. Places where we
currently worry about the contradictory macros could become
conditional #error directives instead.

2. To make it possible to test any of that, you either need a working
Windows+Cygwin setup, or working CI. I'm a salty old Unix hacker so I
opted for the latter, and I also hope this will eventually be useful
to others. Unfortunately I haven't figured out how to get everything
working yet, so some of the check-world tests are failing. Clues most
welcome!

The version I'm posting here is set to run always, so that cfbot will
show it alongside others. But I would imagine that if we got a
committable-quality version of this, it'd probably be opt-in, so you'd
have to say "ci-os-only: cygwin", or "ci-os-only: cygwin, windows" etc
in a commit to your private github account to ask for it (or maybe
we'll come up with a way to tell cfbot we want the full works of CI
checks; the same decision will come up for MSYS, OpenBSD and NetBSD CI
support that my colleague is working on). There are other things to
fix too, including abysmal performance; see commit message.

3. You can't really run PostgreSQL on Cygwin for real, because its
implementation of signals does not have reliable signal masking, so
unsubtle and probably also subtle breakage occurs. That was reported
upstream by Noah years ago, but they aren't working on a fix.
lorikeet shows random failures, and presumably any CI system will do
the same... I even wondered about putting our own magic entry/exit
macros into signal handlers, that would use atomics to implement a
second level of signal masking (?!) but that's an uncommonly large
bandaid for a defective platform... and trying to fix Cygwin itself
is a rabbithole too far for me.

4. When building with Cygwin GCC 11.3 you get a bunch of warnings
that don't show up on other platforms, seemingly indicating that it
interprets -Wimplicit-fallthrough=3 differently. Huh?

[1] https://www.postgresql.org/message-id/CA%2BhUKGKZ_FjkBnjGADk%2Bpa2g4oKDcG8%3DSE5V23sPTP0EELfyzQ%40mail.gmail.com

Attachment Content-Type Size
0001-WIP-CI-support-for-Cygwin.patch text/x-patch 2.9 KB
0002-WIP-Do-not-pollute-Cygwin-namespace-with-Windows-hea.patch text/x-patch 6.3 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-07-26 04:34:18 Re: Cygwin cleanup
Previous Message Amit Kapila 2022-07-26 04:09:01 Re: Handle infinite recursion in logical replication setup