PostgreSQL does not compile on macOS SDK 15.0

It appears in macOS SDK 14.5, there were include guards in
$SDK_ROOT/usr/include/xlocale/_regex.h:

#ifndef _XLOCALE__REGEX_H_
#define _XLOCALE__REGEX_H_

#ifndef _REGEX_H_
#include <_regex.h>
#endif // _REGEX_H_
#include <_xlocale.h>

In macOS SDK 15.5, these include guards are gone:

#ifndef _XLOCALE__REGEX_H_
#define _XLOCALE__REGEX_H_

#include <_regex.h>
#include <__xlocale.h>

Since _REGEX_H_ was defined locally in PostgreSQL's version of
src/include/regex/regex.h, these include guards prevented duplicate
definitions from /usr/include/_regex.h (not to be confused with
/usr/include/xlocale/_regex.h).

If I hack the PostgreSQL src/include/regex/regex.h to include the double
underscore include guard of __REGEX_H_, the build succeeds:

```
diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index d08113724f..734172167a 100644
--- a/src/include/regex/regex.h
+++ b/src/include/regex/regex.h
@@ -1,3 +1,6 @@
+#ifndef __REGEX_H_
+#define __REGEX_H_ /* never again */
+
 #ifndef _REGEX_H_
 #define _REGEX_H_ /* never again */
 /*
@@ -187,3 +190,5 @@ extern bool RE_compile_and_execute(text *text_re, char
*dat, int dat_len,
    int nmatch, regmatch_t *pmatch);

 #endif /* _REGEX_H_ */
+
+#endif /* __REGEX_H_ */
```

Any better ideas here?

Hi,

I've reproduced this issue by:

1. Download the XCode 16 beta 2 ZIP file:
https://developer.apple.com/services-account/download?path=/Developer_Tools/Xcode_16_beta/Xcode_16_beta.xip
2. Extract this to `/tmp`.
3. Then I ran:

export PATH=/tmp/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin:$PATH
export SDKROOT=/tmp/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.0.sdk
export XCODE_DIR=/tmp/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain
export CC="$XCODE_DIR/usr/bin/clang" export CXX="$XCODE_DIR/usr/bin/clang++"

./configure CC="$CC" CXX="$CXX"
make

Does it work if you do the same with XCode 15?

Perhaps I'm missing something but to me it doesn't look like the
right/supported way of compiling PostgreSQL on this platform [1]https://www.postgresql.org/docs/current/installation-platform-notes.html#INSTALLATION-NOTES-MACOS. I
tried to figure out what version of Xcode I'm using right now, but it
seems to be none:

$ /usr/bin/xcodebuild -version
xcode-select: error: tool 'xcodebuild' requires Xcode, but active
developer directory '/Library/Developer/CommandLineTools' is a command
line tools instance

Clang I'm using doesn't seem to be part of XCode distribution either:

$ clang --version
Homebrew clang version 18.1.6
Target: x86_64-apple-darwin23.5.0
Thread model: posix
InstalledDir: /usr/local/opt/llvm/bin

It's been a while since I installed all the dependencies on my laptop,
but I'm pretty confident I followed the documentation back then.

IMO the right way to test PostgreSQL against the recent beta version
of MacOS SDK would be replacing (via a symlink perhaps) the SDK
provided by the "Command Line Tools for Xcode" package
(/Library/Developer/CommandLineTools/SDKs/). Or alternatively finding
the official way of installing the beta version of this package.

[1]: https://www.postgresql.org/docs/current/installation-platform-notes.html#INSTALLATION-NOTES-MACOS

--
Best regards,
Aleksander Alekseev

Hi,

IMO the right way to test PostgreSQL against the recent beta version
of MacOS SDK would be replacing (via a symlink perhaps) the SDK
provided by the "Command Line Tools for Xcode" package
(/Library/Developer/CommandLineTools/SDKs/). Or alternatively finding
the official way of installing the beta version of this package.

As it turned out there is Command_Line_Tools_for_Xcode_16_beta.dmg
package available. It can be downloaded from
https://developer.apple.com/ after logging it. I installed it and also
did:

```
cd /Library/Developer/CommandLineTools/SDKs
sudo mkdir __ignore__
sudo mv MacOSX14.* __ignore__
```

... to make sure Postgres will not find the older version of SDK (it
did until I made this step).

Now I get the following output from `meson --setup ...`:

```
The Meson build system
Version: 0.61.2
Source dir: /Users/eax/projects/c/postgresql
Build dir: /Users/eax/projects/c/postgresql/build
Build type: native build
Project name: postgresql
Project version: 17beta2
C compiler for the host machine: cc (clang 16.0.0 "Apple clang version
16.0.0 (clang-1600.0.20.10)")
C linker for the host machine: cc ld64 1115.5.3
Host machine cpu family: x86_64
Host machine cpu: x86_64
Run-time dependency threads found: YES
Message: darwin sysroot: /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk
...
```

... and get the error reported by Stan. Also I can confirm that the
proposed workaround fixes it. Attached is the result of `git
format-patch` for convenience.

Personally I'm not extremely happy with this workaround though. An
alternative solution would be adding the "pg_" prefix to our type
declarations.

Another question is whether we should fix this while the SDK is in
beta or only after it is released.

Thoughts?

I added the patch to the nearest commitfest so that it wouldn't be lost [1]https://commitfest.postgresql.org/48/5073/.

[1]: https://commitfest.postgresql.org/48/5073/

--
Best regards,
Aleksander Alekseev

Hi,

On 2024-06-25 16:49:32 +0300, Aleksander Alekseev wrote:

... to make sure Postgres will not find the older version of SDK (it
did until I made this step).

You should be able to influence that by specifying -Ddarwin_sysroot=...

... and get the error reported by Stan. Also I can confirm that the
proposed workaround fixes it. Attached is the result of `git
format-patch` for convenience.

Personally I'm not extremely happy with this workaround though.

Yea, it seems decidedly not great.

An alternative solution would be adding the "pg_" prefix to our type
declarations.

A third approach would be to make sure we don't include xlocale.h from
pg_locale.h. IMO pg_locale currently exposes too many implementation details,
neither xlocale.h nor ucol.h should be included in it, that should be in a C
file.

Another question is whether we should fix this while the SDK is in
beta or only after it is released.

Yea.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2024-06-25 16:49:32 +0300, Aleksander Alekseev wrote:

Another question is whether we should fix this while the SDK is in
beta or only after it is released.

Yea.

Stan has started multiple threads about this, which is not doing
anyone any favors, but that issue was already brought up in

/messages/by-id/4edd2d3c30429c4445cc805ae9a788c489856eb7.1719265762.git.stanhu@gmail.com

I think the immediate action item should be to push back on the
change and see if we can get Apple to undo it. If we have to
fix it on our side, it is likely to involve API-breaking changes
that will cause trouble for extensions. The more so because
we'll have to change stable branches too.

I tend to agree with the idea that not including <xlocale.h>
so widely might be the least-bad fix; but that still risks
breaking code that was dependent on that inclusion.

regards, tom lane

Thanks, everyone. Sorry to create multiple threads on this.

As I mentioned in the other thread, I've already submitted a bug
report to Apple (FB14047412). My colleagues know a key macOS engineer,
and they have reached out to him to review that thread and bug report.
I'll update if we hear anything.

On Wed, Jun 26, 2024 at 2:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think the immediate action item should be to push back on the
change and see if we can get Apple to undo it. If we have to
fix it on our side, it is likely to involve API-breaking changes
that will cause trouble for extensions. The more so because
we'll have to change stable branches too.

I am struggling to understand why they would consider such a request.
POSIX reserves *_t, and then regex_t et al explicitly, and I don't see
why Apple isn't allowed to have arbitrary transitive inclusions across
system headers... AFIACT nothing in POSIX or C restricts that (?).
Any system could have the same problem. It's only a coincidence that
we got away with it before because apparently other OSes don't pull in
the system regex-related definitions from anything that we include,
except macOS which previously happened to use the same include guards
scheme. I guess you could write PostgreSQL extensions (or TCL
programs) that could crash due to using bad struct definitions on
earlier SDK versions depending on whether you included <regex.h> or
PostgreSQL headers first?

Amusingly, those matching include guards probably came from the same
keyboard (Apple's regex code is an earlier strain of Henry Spencer's
regex code, from 4.4BSD). FreeBSD et al have it too. FreeBSD also
has an xlocale.h "extended locale" header (which came back from
Apple), though on FreeBSD it is automatically included by <locale.h>,
because that "extended" locale stuff became standard issue basic
locale support in POSIX 2008, it's just that Apple hasn't got around
to tidying that up yet so they still force us to include <xlocale.h>
explicitly (now *that* is material for a bug report)...

If you look at the header[1]https://github.com/apple-oss-distributions/Libc/blob/main/include/xlocale.h, you can see the mechanism for pulling in
a ton of other stuff: <xlocale.h> wants to activate all the _l
functions, so it runs around including xlocale/_EVERYTHING.h. For
example xlocale/_string.h adds strcoll_l(..., locale_t), and
xlocale/_regex.h adds regcomp_l(..., locale_t), etc etc. Which all
seems rather backwards from our vantage point where locale_t is
standard and those should ideally have been declared in the "primary"
header when people actually wanted them and explicitly said so by
including eg <string.h>. So why doesn't FreeBSD have the same
problem? Just because it doesn't actually have reg*_l() functions...
yet. But it will, there is talk of adding the complete set of every
imaginable _l function to POSIX. So FreeBSD might eventually add
xlocale/_regex.h to that header explosion (unless someone does the
completely merge/tidy-up I imagined above, which I might suggest).
Perhaps in the fullness of time Apple will also do a similar clean-up,
so that xlocale.h goes away, but I wouldn't hold my breath.

I don't have any great ideas about what to do about this.
Cybersquatting system facilities is a messy business, so maybe the
proposed grotty solution is actually appropriate! We did bring this
duelling Henry Spencers problem upon ourselves. Longer term,
pg_regex_t seems to make a lot of sense, except IIUC we want to keep
this code in sync with TCL so perhaps a configurable prefix could be
done with macrology?

[1]: https://github.com/apple-oss-distributions/Libc/blob/main/include/xlocale.h

Thomas Munro <thomas.munro@gmail.com> writes:

I don't have any great ideas about what to do about this.
Cybersquatting system facilities is a messy business, so maybe the
proposed grotty solution is actually appropriate! We did bring this
duelling Henry Spencers problem upon ourselves. Longer term,
pg_regex_t seems to make a lot of sense, except IIUC we want to keep
this code in sync with TCL so perhaps a configurable prefix could be
done with macrology?

Yeah. I'd do pg_regex_t in a minute except that it'd break existing
extensions using our facilities. However, your mention of macrology
stirred an idea: could we have our regex/regex.h intentionally
#include the system regex.h and then do
#define regex_t pg_regex_t
? If that works, our struct is really pg_regex_t, but we don't have
to change any existing calling code. It might get a bit messy
undef'ing and redef'ing all the other macros in regex/regex.h, but
I think we could make it fly without any changes in other files.

regards, tom lane

On Mon, Jul 1, 2024 at 2:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

I don't have any great ideas about what to do about this.
Cybersquatting system facilities is a messy business, so maybe the
proposed grotty solution is actually appropriate! We did bring this
duelling Henry Spencers problem upon ourselves. Longer term,
pg_regex_t seems to make a lot of sense, except IIUC we want to keep
this code in sync with TCL so perhaps a configurable prefix could be
done with macrology?

Yeah. I'd do pg_regex_t in a minute except that it'd break existing
extensions using our facilities. However, your mention of macrology
stirred an idea: could we have our regex/regex.h intentionally
#include the system regex.h and then do
#define regex_t pg_regex_t
? If that works, our struct is really pg_regex_t, but we don't have
to change any existing calling code. It might get a bit messy
undef'ing and redef'ing all the other macros in regex/regex.h, but
I think we could make it fly without any changes in other files.

Good idea. Here's an attempt at that.

I don't have a Mac with beta SDK 15 yet, but I think this should work?

Hi,

Good idea. Here's an attempt at that.

I don't have a Mac with beta SDK 15 yet, but I think this should work?

I checked against SDK 15 and 14. I also checked that it doesn't break
something on Linux.

The patch seems to work. I don't have a Windows machine unfortunately.

--
Best regards,
Aleksander Alekseev

Thomas Munro <thomas.munro@gmail.com> writes:

On Mon, Jul 1, 2024 at 2:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah. I'd do pg_regex_t in a minute except that it'd break existing
extensions using our facilities. However, your mention of macrology
stirred an idea: could we have our regex/regex.h intentionally
#include the system regex.h and then do
#define regex_t pg_regex_t
?

Good idea. Here's an attempt at that.

I think it might be cleaner to put the new #include and macro hacking
into regcustom.h, to show that it's our own hack and not part of the
"official" Spencer code. OTOH, we do have to touch regex.h anyway
to change the #include guards, and it's not like there are not any
other PG-isms in there. So I'm not 100% sold that that way would
be better --- what do you think?

regards, tom lane

I wrote:

I think it might be cleaner to put the new #include and macro hacking
into regcustom.h, to show that it's our own hack and not part of the
"official" Spencer code.

Oh, scratch that. I was thinking regex.h included regcustom.h,
but it doesn't, so there's no way that can work. Never mind...

regards, tom lane

On Thu, Jul 4, 2024 at 9:12 PM Aleksander Alekseev
<aleksander@timescale.com> wrote:

I checked against SDK 15 and 14. I also checked that it doesn't break
something on Linux.

Thanks for testing!

The patch seems to work. I don't have a Windows machine unfortunately.

Yeah, Windows doesn't have <regex.h> (it has <regex> as part of the
C++ standard library, but nothing for C because that's from POSIX, not
the C standard library). So I just skip the #include on Windows, and
I see that it's passing on all CI.

It seems like there is no reason not to go ahead and push this,
including back-patching, then.

I had been thinking that I should try harder to make the pg_ prefix
compile-time configurable (imagine some kind of string-pasting macros
constructing the names), so that TCL and PG could have fewer diffs.
But we're already not doing that for the function names, so unless Tom
wants me to try to do that...?

It's a funny position to finish up in: we have pg_ functions, pg_
types but still standard REG_XXX macros. In the future someone might
want to rename them all to PG_REG_XXX, so that we completely move out
of the way of the system regex stuff. But not today, and certainly
not in back-branches.

Thomas Munro <thomas.munro@gmail.com> writes:

I had been thinking that I should try harder to make the pg_ prefix
compile-time configurable (imagine some kind of string-pasting macros
constructing the names), so that TCL and PG could have fewer diffs.
But we're already not doing that for the function names, so unless Tom
wants me to try to do that...?

Nah, I don't see much point in that.

It's a funny position to finish up in: we have pg_ functions, pg_
types but still standard REG_XXX macros. In the future someone might
want to rename them all to PG_REG_XXX, so that we completely move out
of the way of the system regex stuff. But not today, and certainly
not in back-branches.

That would be an API break for any extensions using our regex code,
so I'm not especially in favor of it.

regards, tom lane

And pushed.