Re: PG vs macOS Mojave

Well, macOS 10.14 (Mojave) is out, so I installed it on a spare machine,
and naturally the first thing I tried was to build PG with it. Our
core code seems fine, but:

* --with-perl fails in configure, complaining that it can't find perl.h.

* --with-tcl fails in configure, complaining that it can't find
tclConfig.sh. Furthermore, the historical workaround for that
(--with-tclconfig=/System/Library/Frameworks/Tcl.framework) doesn't fix it.

After some investigation, it seems that Apple has been busy moving
header files (not libraries) under SDK-specific "sysroot" directories,
with the expectation that you'd compile using "-isysroot $SYSROOT".
There's some mention of that here, for example:

https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/cross_development/Configuring/configuring.html#//apple_ref/doc/uid/10000163i-CH1-SW7

The sysroot seems to contain only headers; stuff you need at runtime,
such as shared libraries, is still where it used to be.

The recommended way to get the appropriate sysroot path seems to be

SYSROOT=`xcodebuild -version -sdk macosx Path`

Attached is a draft patch to fix things up. The core ideas are

(1) Stop assuming that the Perl headers and library are necessarily
in the same place; create a perl_includedir variable to represent the
path to the former.

(2) Tweak src/template/darwin to inject the appropriate -isysroot
option into CPPFLAGS.

(3) Remove the need to manually specify the path to tclConfig.sh,
which has gotten even more painful than before because now it's
somewhere under the sysroot. You can still specify --with-tclconfig
if you really want to, but it's not necessary anymore to build pltcl
under recent macOS.

Note that (3) alone is not sufficient to fix pltcl; we must do (2)
as well because tclConfig.sh now reports the Tcl include flags as
TCL_INCLUDE_SPEC = -iwithsysroot /System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers
so unless we also set -isysroot this doesn't work.

It's a bit scary to be adding -isysroot globally. I thought
briefly about using it only while building pltcl, but that seems
even more dangerous: if there were any discrepancies between the
headers in the sysroot and those in the normal include directories,
building pltcl with different headers from the rest of the system
would surely be disastrous. In any case, I suspect that the handwriting
is on the wall, and before very much longer it's going to be impossible
to build meaningful code on macOS without -isysroot anyway.

I've tested this on all the macOS versions I have at hand, and it
doesn't seem to break anything. Only part (1) could possibly
affect other platforms, and that seems safe enough.

I'd like to commit and backpatch this, because otherwise longfin
is going to start falling over when I upgrade its host to Mojave.

Thoughts?

regards, tom lane

On Tue, Sep 25, 2018 at 4:49 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I've tested this on all the macOS versions I have at hand, and it
> doesn't seem to break anything. Only part (1) could possibly
> affect other platforms, and that seems safe enough.
>
> I'd like to commit and backpatch this, because otherwise longfin
> is going to start falling over when I upgrade its host to Mojave.

Looks good on this 10.13.4 system. About to upgrade to 10.14...

--
Thomas Munro
http://www.enterprisedb.com

Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> On Tue, Sep 25, 2018 at 4:49 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I've tested this on all the macOS versions I have at hand, and it
>> doesn't seem to break anything. Only part (1) could possibly
>> affect other platforms, and that seems safe enough.
>>
>> I'd like to commit and backpatch this, because otherwise longfin
>> is going to start falling over when I upgrade its host to Mojave.

> Looks good on this 10.13.4 system. About to upgrade to 10.14...

Thanks for testing! I'll set to work on back-patching that.

regards, tom lane

On Wed, Sep 26, 2018 at 3:14 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> > On Tue, Sep 25, 2018 at 4:49 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I've tested this on all the macOS versions I have at hand, and it
> >> doesn't seem to break anything. Only part (1) could possibly
> >> affect other platforms, and that seems safe enough.
> >>
> >> I'd like to commit and backpatch this, because otherwise longfin
> >> is going to start falling over when I upgrade its host to Mojave.
>
> > Looks good on this 10.13.4 system. About to upgrade to 10.14...
>
> Thanks for testing! I'll set to work on back-patching that.

... and now I'm on macOS 10.14. I removed all traces of MacPorts from
my PATH and configure line to test this. --with-tcl worked, but
--with-perl couldn't find "perl.h". Then I realised that it was
because I was still on Xcode 9, so I was in for another ~5GB of
upgrading to get to Xcode 10. After that, it all worked fine.

--
Thomas Munro
http://www.enterprisedb.com

Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> ... and now I'm on macOS 10.14. I removed all traces of MacPorts from
> my PATH and configure line to test this. --with-tcl worked, but
> --with-perl couldn't find "perl.h". Then I realised that it was
> because I was still on Xcode 9, so I was in for another ~5GB of
> upgrading to get to Xcode 10. After that, it all worked fine.

Interesting. IME, updating Xcode first usually gives you a working
system, for some value of "work". Right now I've updated to Xcode
10 but not yet Mojave on my main laptop, and PG works except that
the linker warns about out-of-sync library files during some link
steps. I don't see that on the other machine where I've done both
updates. I don't recall ever having tried the other order, but
evidently it (sometimes?) has some issues.

Maybe we should make the test in configure for whether to prepend
PG_SYSROOT be more specific, ie

- if test -d "$PG_SYSROOT$perl_archlibexp" ; then
+ if test -f "$PG_SYSROOT$perl_archlibexp/CORE/perl.h" ; then
perl_includedir="$PG_SYSROOT$perl_archlibexp"

I think what must've happened to you is that in Xcode 9, the
$PG_SYSROOT$perl_archlibexp directory exists but doesn't contain
the header files. Now, once you'd updated the main system,
neither would $perl_archlibexp, so you're going to fail either
way in that state :-(. But this feels a bit safer somehow.

regards, tom lane

Am 25. Sep. 2018 um 06:49 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> It's a bit scary to be adding -isysroot globally.
> (...)
> I've tested this on all the macOS versions I have at hand, and it
> doesn't seem to break anything.
> (...)
> Thoughts?
>
> regards, tom lane

This breaks building extensions with a different SDK than the server itself.

Previously, it was possible for users to download a binary PostgreSQL distribution and build an extension just by typing 'make install'.

Now pgxs/src/makefile.global contains -isysroot of the machine where PostgreSQL was built. If they use a different version of Xcode, or if they renamed Xcode (I often use multiple versions side by side), compilation will fail.

As a workaround for Postgres.app, I've tried setting PG_SYSROOT to /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk (which is a symlink to the current SDK). This allows building extensions on a newer OS than what I use for building the server, but if the user renames Xcode that still doesn't help.

Jakob

Jakob Egger <jakob(at)eggerapps(dot)at> writes:
> Am 25. Sep. 2018 um 06:49 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> It's a bit scary to be adding -isysroot globally.

> This breaks building extensions with a different SDK than the server itself.

Hm, under what circumstances would that be safe or a good idea?

The concerns that strike my mind are mainly around the hazard that
configure's choices could be inappropriate for a build with a
different SDK. Given that, though, wiring -isysroot into the
configure output doesn't seem all that insane.

It's possible that we could arrange things so that the generated
Makefile.global looks like

PG_SYSROOT = /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
CPPFLAGS = -isysroot $(PG_SYSROOT)

instead of

CPPFLAGS = -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk

thus making it a bit simpler to override that path at build time.
But I'm not sure that that really moves the goalposts very far for you
in terms of ease-of-use, and anyway it might just be greasing the skids
to a non-working extension.

regards, tom lane

I wrote:
> Jakob Egger <jakob(at)eggerapps(dot)at> writes:
>> Am 25. Sep. 2018 um 06:49 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> It's a bit scary to be adding -isysroot globally.

>> This breaks building extensions with a different SDK than the server itself.

> Hm, under what circumstances would that be safe or a good idea?

Oh, well *this* is interesting. I stated at the top of this thread that
pltcl doesn't build on Mojave without -isysroot because TCL_INCLUDE_SPEC
gets set to
-iwithsysroot /System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers

I wondered how messy it would be to translate that to a plain -I spec,
and along the way I found out that we apparently don't have to. If I do
nothing except remove -isysroot from CPPFLAGS, Apple's clang seems to
still find tcl.h at
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers/tcl.h

Now, I would swear that I tested that before, and it didn't work;
but I think I might have been testing the combination of Xcode 10 on
High Sierra. It definitely works with Xcode 10 on Mojave. I no longer
have a High Sierra installation to recheck the original observation on.

However, it also works for me if I change "-isysroot /..." to
"-I $(PG_SYSROOT)/...", and that seems like on the whole it'd be the
safest solution across different compilers and version combinations.

If we go this route, then the configure-time sysroot path will get
embedded into $perl_includedir and $TCL_INCLUDE_SPEC, but noplace
else. I don't know how far that would improve your concern about
extensions, but it should help a little.

I'm still a bit dubious about the safety of building extensions
with different SDKs than the server, but if it worked for you
before then there's no reason for us to break it further than
we have to.

regards, tom lane

I wrote:
> If we go this route, then the configure-time sysroot path will get
> embedded into $perl_includedir and $TCL_INCLUDE_SPEC, but noplace
> else. I don't know how far that would improve your concern about
> extensions, but it should help a little.
> I'm still a bit dubious about the safety of building extensions
> with different SDKs than the server, but if it worked for you
> before then there's no reason for us to break it further than
> we have to.

Concretely, I'm planning to push the attached as soon as I finish
back-patching it.

regards, tom lane

Am 16. Okt. 2018 um 17:13 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Oh, well *this* is interesting. I stated at the top of this thread that
> pltcl doesn't build on Mojave without -isysroot because TCL_INCLUDE_SPEC
> gets set to
> -iwithsysroot /System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers

> Now, I would swear that I tested that before, and it didn't work;
> but I think I might have been testing the combination of Xcode 10 on
> High Sierra. It definitely works with Xcode 10 on Mojave. I no longer
> have a High Sierra installation to recheck the original observation on.

I realised that I can disable PG_SYSROOT by providing a non-existing dir name,
so I've done some testing to try to reproduce your original issue.

I've tested building the PG 11.0 tarball on the following systems:

macOS 10.13 with Xcode 9.4.1
macOS 10.13 with Xcode 10.0
macOS 10.14 with Xcode 10.0

On macOS 10.13 the following worked fine both for Xcode 9 and 10:
PG_SYSROOT=no-sysroot ./configure --with-tcl

On macOS 10.14 the above failed because it didn't find tclConfig.sh.
The following did work:
PG_SYSROOT=no-sysroot ./configure --with-tcl --with-tclconfig=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Tcl.framework

So it seems that providing -isysroot is not actually necessary for building --with-tcl.
clang finds the headers just fine.

Is it possible that you were using a different compiler that made the -isysroot setting necessary?

In any case, I'm pretty sure that your patch should fix my problem with building extensions.

For Postgres.app, since I'm building on macOS 10.12, I think I will just use the PG_SYSROOT=no-sysroot trick for now.

Best regards,
Jakob

Jakob Egger <jakob(at)eggerapps(dot)at> writes:
> Am 16. Okt. 2018 um 17:13 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> Oh, well *this* is interesting. I stated at the top of this thread that
>> pltcl doesn't build on Mojave without -isysroot because TCL_INCLUDE_SPEC
>> gets set to
>> -iwithsysroot /System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers
>> Now, I would swear that I tested that before, and it didn't work;
>> but I think I might have been testing the combination of Xcode 10 on
>> High Sierra. It definitely works with Xcode 10 on Mojave. I no longer
>> have a High Sierra installation to recheck the original observation on.

> I realised that I can disable PG_SYSROOT by providing a non-existing dir name,
> so I've done some testing to try to reproduce your original issue.

Thanks for looking into it!

> On macOS 10.13 the following worked fine both for Xcode 9 and 10:
> PG_SYSROOT=no-sysroot ./configure --with-tcl
> On macOS 10.14 the above failed because it didn't find tclConfig.sh.

Yeah, the problem of not finding tclConfig.sh is definitely the first
stumbling block for a non-sysroot-aware build. What's not clear to me
now is whether the second stumbling block, finding tcl.h, is an issue.

> The following did work:
> PG_SYSROOT=no-sysroot ./configure --with-tcl --with-tclconfig=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Tcl.framework

Yeah, that agrees with the results I'm getting now. Evidently Apple's
compiler has some obscure search path rules that let it find tcl.h despite
not being explicitly told what sysroot to use. But I have a moderately
clear recollection of getting "could not find tcl.h" configure failures
once I'd worked past the tclConfig.sh part. Weird. It's also very odd,
if the compiler will search the sysroot automatically, why that seemingly
works for tcl.h but not perl.h. plperl definitely still fails if you
lobotomize the PG_SYSROOT logic.

> Is it possible that you were using a different compiler that made the -isysroot setting necessary?

Nope, definitely using Apple's clang --- don't have anything else
installed.

> In any case, I'm pretty sure that your patch should fix my problem with building extensions.

OK. I've pushed that now, it will be in November's point releases.

regards, tom lane

Am 17.10.2018 um 16:28 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> It's also very odd,
> if the compiler will search the sysroot automatically, why that seemingly
> works for tcl.h but not perl.h. plperl definitely still fails if you
> lobotomize the PG_SYSROOT logic.

Because pl/tcl ends up using -iwithsysroot to specify the header search path and pl/perl uses -I

See also: https://www.postgresql.org/message-id/153558865647.1483.573481613491501077%40wrigleys.postgresql.org

I would assume that clang sets -isysroot automatically, but I have no idea why that didn't work for you previously.

Jakob

Jakob Egger <jakob(at)eggerapps(dot)at> writes:
> Am 17.10.2018 um 16:28 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> It's also very odd,
>> if the compiler will search the sysroot automatically, why that seemingly
>> works for tcl.h but not perl.h. plperl definitely still fails if you
>> lobotomize the PG_SYSROOT logic.

> Because pl/tcl ends up using -iwithsysroot to specify the header search path and pl/perl uses -I

No, that's not it. It occurred to me to investigate this by using
"gcc ... -E pltcl.c" to see where the compiler is finding the tcl.h
header file. The answer is that if you use

-I /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/Tcl.framework/Versions/8.5/Headers

either explicitly or with a combination of -isysroot and -iwithsysroot,
then that directory is where it finds tcl.h. But if you *don't* do that,
it still manages to find tcl.h here:

# 1 "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/tcl.h" 1 3 4

So the true answer seems to be that the compiler's built-in search path
includes $SYSROOT/usr/include, and that pltcl works despite an "incorrect"
-I value on the command line because there is a copy of tcl.h there (it's
a symlink to the other one, actually). On the other hand, Perl's headers
are *not* under that directory, so plperl doesn't work without an accurate
-I switch on the command line.

> See also: https://www.postgresql.org/message-id/153558865647.1483.573481613491501077%40wrigleys.postgresql.org

Yeah, it was my recollection of that discussion that led me to jump
to the conclusion that we had to use -isysroot, which we now see to
be not such a great idea.

> I would assume that clang sets -isysroot automatically, but I have no idea why that didn't work for you previously.

[ experiments further ... ] It looks like clang does default to assuming
-isysroot with the correct sysroot for its Xcode version. The missing bit
of info is that -I /System/... is taken as an absolute path that's not
affected by the sysroot. You have to write -iwithsysroot /System/...
to have something that is taken relative to the sysroot.

I'm still a little confused by this, because my previous experiments
seemed to imply that we had to write -isysroot explicitly to get the
compiler to assume a sysroot path, ie there did not seem to be any
built-in default. That never made much sense, and the results
I'm getting now contradict it.

So as far as pltcl is concerned, we don't really need the configure
hack to insert PG_SYSROOT into TCL_INCLUDE_SPEC; we could leave Apple's
value alone and it should work. (Not that that gets pltcl out of
dependency on the sysroot entirely, because AFAICS there's still no way
to find tclConfig.sh without explicitly accounting for sysroot. And
I'm not exactly convinced that using explicit sysroot to do that and
then leaving it out later is a hot idea.)

However, on the Perl side, we'd have to change -I to -iwithsysroot if
we wanted to avoid explicitly mentioning the sysroot path, and that
seems way more invasive than it's worth.

I'm inclined to leave things as they stand now; it makes for reasonably
consistent build behavior between pltcl and plperl.

regards, tom lane

I wrote:
> Jakob Egger <jakob(at)eggerapps(dot)at> writes:
>> I would assume that clang sets -isysroot automatically, but I have no idea why that didn't work for you previously.

> [ experiments further ... ] It looks like clang does default to assuming
> -isysroot with the correct sysroot for its Xcode version. The missing bit
> of info is that -I /System/... is taken as an absolute path that's not
> affected by the sysroot. You have to write -iwithsysroot /System/...
> to have something that is taken relative to the sysroot.

> So as far as pltcl is concerned, we don't really need the configure
> hack to insert PG_SYSROOT into TCL_INCLUDE_SPEC; we could leave Apple's
> value alone and it should work. (Not that that gets pltcl out of
> dependency on the sysroot entirely, because AFAICS there's still no way
> to find tclConfig.sh without explicitly accounting for sysroot. And
> I'm not exactly convinced that using explicit sysroot to do that and
> then leaving it out later is a hot idea.)

> However, on the Perl side, we'd have to change -I to -iwithsysroot if
> we wanted to avoid explicitly mentioning the sysroot path, and that
> seems way more invasive than it's worth.

> I'm inclined to leave things as they stand now; it makes for reasonably
> consistent build behavior between pltcl and plperl.

After sleeping on it, I'm not really happy with that answer. I think
we'd be better advised to remove the hack on TCL_INCLUDE_SPEC, and
change what we've done for Perl to look more like what Tcl does.
That is, instead of writing "-I$(perl_includedir)/CORE" in Makefiles,
we should write "$(perl_includespec)" so that the switch spelling
is included in the variable. Then we'd make configure set that
variable to either "-I$perl_archlibexp/CORE" or
"-iwithsysroot $perl_archlibexp/CORE" depending on what it finds
to be needed. In this way, while we can't avoid knowing the sysroot
path explicitly during configure, we aren't wiring it into anything
that configure emits.

It would be a good idea to change this now, before the results of commit
5e2217131 spread too far in the wild, so that plperl-related extensions
only have to deal with one Makefile change not two.

(BTW, plpython is already doing it like this, meaning that it's plperl
that's the odd man out, not pltcl. It appears that Apple have not yet
sysroot-ified their Python installation, but it is not hard to see where
that train is going.)

regards, tom lane

[ just when you thought it was safe to go back in the water ]

I wrote:
>> Jakob Egger <jakob(at)eggerapps(dot)at> writes:
>>> I would assume that clang sets -isysroot automatically, but I have no idea why that didn't work for you previously.

>> [ experiments further ... ] It looks like clang does default to assuming
>> -isysroot with the correct sysroot for its Xcode version.

So today I updated longfin's host to macOS 10.14.1 + Xcode 10.1,
and things promptly broke:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=longfin&dt=2018-10-31%2013%3A49%3A50

It can't find Perl's headers anymore. Investigation says that the reason
is that clang no longer has this as default:

-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk

You can verify the thing's built-in settings with something like

$ echo >test.c
$ clang -v -E test.c

and on Xcode 10.0 I get

"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -E -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info -debugger-tuning=lldb -target-linker-version 409.12 -v -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/10.0.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -I/usr/local/include -fdebug-compilation-dir /Users/tgl -ferror-limit 19 -fmessage-length 80 -stack-protector 1 -fblocks -fencode-extended-block-signature -fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -o - -x c test.c

while with Xcode 10.1's compiler:

"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -E -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info -debugger-tuning=lldb -target-linker-version 409.12 -v -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/10.0.0 -fdebug-compilation-dir /Users/tgl -ferror-limit 19 -fmessage-length 80 -stack-protector 1 -fblocks -fencode-extended-block-signature -fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -o - -x c test.c

No default sysroot at all. So the immediate question is whether Apple
broke this in Xcode 10.1, or whether it was broken in Xcode 10.0 and
now they've "fixed" it. Unfortunately, I'm afraid the answer is the
latter. I do not any longer have a pre-10.0 Xcode handy to try, but
if those versions did not have any default sysroot, that would handily
explain the results I was getting before updating to Mojave. Also,
some googling suggests that older Xcode versions had no default sysroot
setting; for instance see here:

https://langui.sh/2015/07/24/osx-clang-include-lib-search-paths/

In short, it looks like our current solution of expecting the compiler
to default to the correct -isysroot setting is just broken. It
accidentally works on Xcode 10.0, but not earlier or later versions.

We don't have a lot of time to fix this, as our next quarterly
releases wrap on Monday. Right now I think the only plausible
fix is to go back to adding "-isysroot $PG_SYSROOT" to CPPFLAGS.
Maybe we can fix things to make it relatively easy to override that
for cases like building against a different Xcode version, but it's
not going to just be automatic for that to work.

regards, tom lane

> On 1 Nov 2018, at 04:17, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> and on Xcode 10.0 I get

Odd. I don’t actually get -isysroot on XCode 10.0 on my 10.13.6 installation,
on 10.12 with XCode 8.3.3 I do however get -isysroot.

> Right now I think the only plausible
> fix is to go back to adding "-isysroot $PG_SYSROOT" to CPPFLAGS.

+1. That does seem like the safe option.

cheers ./daniel

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> Odd. I don’t actually get -isysroot on XCode 10.0 on my 10.13.6 installation,
> on 10.12 with XCode 8.3.3 I do however get -isysroot.

Wow ... could it be that it actually varies depending on the combination
of compiler and OS versions? That would be weird.

regards, tom lane

> On 1 Nov 2018, at 15:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> Odd. I don’t actually get -isysroot on XCode 10.0 on my 10.13.6 installation,
>> on 10.12 with XCode 8.3.3 I do however get -isysroot.
>
> Wow ... could it be that it actually varies depending on the combination
> of compiler and OS versions? That would be weird.

Or the version of XCode and the set of installed SDKs? I only have a single
SDK installed on both of these systems, if you have multiple ones on the 10.0
installation that might explain something. Or not.

cheers ./daniel

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> On 1 Nov 2018, at 15:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Wow ... could it be that it actually varies depending on the combination
>> of compiler and OS versions? That would be weird.

> Or the version of XCode and the set of installed SDKs? I only have a single
> SDK installed on both of these systems, if you have multiple ones on the 10.0
> installation that might explain something. Or not.

Nope, I just have the one:

$ ls -l /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/
total 0
drwxr-xr-x 7 root wheel 224 Oct 31 11:22 MacOSX.sdk/
lrwxr-xr-x 1 root wheel 10 Sep 19 19:28 MacOSX10.14.sdk@ -> MacOSX.sdk

regards, tom lane

> On 1 Nov 2018, at 15:53, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> On 1 Nov 2018, at 15:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Wow ... could it be that it actually varies depending on the combination
>>> of compiler and OS versions? That would be weird.
>
>> Or the version of XCode and the set of installed SDKs? I only have a single
>> SDK installed on both of these systems, if you have multiple ones on the 10.0
>> installation that might explain something. Or not.
>
> Nope, I just have the one:

Then I’m inclined to say that it probably depends on the combination of OS
version, XCode version and potentially the SDK version. Either way, passing
the -isysroot explicitly as in your suggestion should still work unless I’m
missing something.

cheers ./daniel

So it seems like there are two ways we could go about this. One is
to go back to the scheme of adding an -isysroot switch to CPPFLAGS,
where it'd have global effects. We could make this slightly less
painful for scenarios like Jakob's if we set things up in Makefile.global
this way:

CPPFLAGS = -isysroot $(PG_SYSROOT)
PG_SYSROOT = /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk

and then, if you need to build on a different SDK version without
reconfiguring, you can do something like "make PG_SYSROOT=/proper/path".
I coded this up, as attached, and it seems to work but it's still not all
that friendly for such cases.

The other idea that's occurred to me is to go back to the scheme of
commit 68fc227dd, where we inject the sysroot path into just the -I
switches used for PL/Perl and PL/Tcl. We could improve on that
commit by injecting it symbolically similar to what I did here, ie
what ends up in the configure output is

PG_SYSROOT = /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk

perl_includespec = -I $(PG_SYSROOT)/System/Library/Perl/5.18/darwin-thread-multi-2level/CORE

Then somebody who wants to build on a different SDK version still needs
to do "make PG_SYSROOT=/proper/path", but only if they're trying to
build PL/Perl or related extensions. So this second way seems uglier
in some sense but less likely to cause problems for most people.

Either way I guess we'd need to document it rather than just hoping
it's invisible.

Thoughts?

regards, tom lane

On 01/11/2018 22:17, Tom Lane wrote:
> The other idea that's occurred to me is to go back to the scheme of
> commit 68fc227dd, where we inject the sysroot path into just the -I
> switches used for PL/Perl and PL/Tcl. We could improve on that
> commit by injecting it symbolically similar to what I did here, ie
> what ends up in the configure output is

How does that work when building against a non-system Perl or Tcl?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> On 01/11/2018 22:17, Tom Lane wrote:
>> The other idea that's occurred to me is to go back to the scheme of
>> commit 68fc227dd, where we inject the sysroot path into just the -I
>> switches used for PL/Perl and PL/Tcl. We could improve on that
>> commit by injecting it symbolically similar to what I did here, ie
>> what ends up in the configure output is

> How does that work when building against a non-system Perl or Tcl?

It does nothing, because configure will not find that it needs to
inject any sysroot reference in order to find such a Perl or Tcl's
headers.

regards, tom lane

I wrote:
> Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
>> On 01/11/2018 22:17, Tom Lane wrote:
>>> The other idea that's occurred to me is to go back to the scheme of
>>> commit 68fc227dd, where we inject the sysroot path into just the -I
>>> switches used for PL/Perl and PL/Tcl. We could improve on that
>>> commit by injecting it symbolically similar to what I did here, ie
>>> what ends up in the configure output is

>> How does that work when building against a non-system Perl or Tcl?

> It does nothing, because configure will not find that it needs to
> inject any sysroot reference in order to find such a Perl or Tcl's
> headers.

Here's a lightly-tested patch for that approach.

regards, tom lane

I wrote:
> Then somebody who wants to build on a different SDK version still needs
> to do "make PG_SYSROOT=/proper/path", but only if they're trying to
> build PL/Perl or related extensions. So this second way seems uglier
> in some sense but less likely to cause problems for most people.

> Either way I guess we'd need to document it rather than just hoping
> it's invisible.

Here's a proposed doc patch for this second approach. It'd still mostly
work for the first approach, if we drop the sentence about it only
mattering for PL/Perl and PL/Tcl.

I failed to resist the temptation to mutter something about SIP while
at it. AFAIK that problem isn't mentioned anywhere else.

regards, tom lane

I wrote:
> Here's a lightly-tested patch for that approach.

Anybody have an opinion about which approach to use? We need to choose
one, and PDQ too, if we want full buildfarm coverage on it before Monday's
wrap.

The main argument in favor of #1 (restore use of -isysroot) is fear that
Apple's going to force us into that sometime soon anyhow, so we might as
well just bite the bullet instead of inserting weird workarounds to avoid
it. But perhaps that isn't going to happen. If it doesn't, then #2
(hack PLPerl and PLTcl include switches only) should result in slightly
less pain for people in Jakob's situation. But either patch does offer
a workaround for that case.

regards, tom lane

> On 2 Nov 2018, at 15:42, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> I wrote:
>> Here's a lightly-tested patch for that approach.
>
> Anybody have an opinion about which approach to use? We need to choose
> one, and PDQ too, if we want full buildfarm coverage on it before Monday's
> wrap.

Sorry for being slow to respond, I was hoping to find time for testing but it’s
a scarce resource right now.

> The main argument in favor of #1 (restore use of -isysroot) is fear that
> Apple's going to force us into that sometime soon anyhow, so we might as
> well just bite the bullet instead of inserting weird workarounds to avoid
> it. But perhaps that isn't going to happen.

#1 is the option that appeals to me the most, mostly because it removes all
possible ambiguity of when/if it’s required compared to #2.

+ Most Postgres developers just turn off SIP, though.

Minor nitpick, shouldn’t this be <productname>Postgres</productname>?

cheers ./daniel

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> On 2 Nov 2018, at 15:42, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Anybody have an opinion about which approach to use? We need to choose
>> one, and PDQ too, if we want full buildfarm coverage on it before Monday's
>> wrap.

> #1 is the option that appeals to me the most, mostly because it removes all
> possible ambiguity of when/if it’s required compared to #2.

I'm sort of leaning in that direction too, after sleeping on it.
It's also closer to what we already shipped in 11.0 than #2 is.

regards, tom lane

> Am 1. Nov. 2018 um 04:17 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>
> You can verify the thing's built-in settings with something like
>
> $ echo >test.c
> $ clang -v -E test.c
>
> and on Xcode 10.0 I get
>
> "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -E -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info -debugger-tuning=lldb -target-linker-version 409.12 -v -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/10.0.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -I/usr/local/include -fdebug-compilation-dir /Users/tgl -ferror-limit 19 -fmessage-length 80 -stack-protector 1 -fblocks -fencode-extended-block-signature -fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -o - -x c test.c
>
> while with Xcode 10.1's compiler:
>
> "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -E -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info -debugger-tuning=lldb -target-linker-version 409.12 -v -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/10.0.0 -fdebug-compilation-dir /Users/tgl -ferror-limit 19 -fmessage-length 80 -stack-protector 1 -fblocks -fencode-extended-block-signature -fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -o - -x c test.c
>
> No default sysroot at all. So the immediate question is whether Apple
> broke this in Xcode 10.1, or whether it was broken in Xcode 10.0 and
> now they've "fixed" it.

I've tried to replicate your experiment, but I initially got the "-isysroot" flag for both Xcode 10 and Xcode 10.1.

Then I upgraded the "Comand Line Tools for Xcode 10.1" using Software Update, and suddenly the "-isysroot" flag disappeared (for both Xcode 10 and Xcode 10.1).

In any case, it looks like we can't rely on "-isysroot" being present.

> Am 2. Nov. 2018 um 16:09 schrieb Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se <mailto:daniel(at)yesql(dot)se>> writes:
>>> On 2 Nov 2018, at 15:42, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>> wrote:
>>> Anybody have an opinion about which approach to use? We need to choose
>>> one, and PDQ too, if we want full buildfarm coverage on it before Monday's
>>> wrap.
>
>> #1 is the option that appeals to me the most, mostly because it removes all
>> possible ambiguity of when/if it’s required compared to #2.
>
> I'm sort of leaning in that direction too, after sleeping on it.
> It's also closer to what we already shipped in 11.0 than #2 is.
>
> regards, tom lane

I think I would prefer #2.

I think it would work better for my use case (end users download a binary of the server but may build their own extensions).

Regarding #1:
Since -isysroot is only necessary for building pl/tcl and pl/perl, why do you add it to CPPFLAGS in the global makefile?
Wouldn't it make more sense to just add it in the TCL and Perl makefiles?

Best regards,
Jakob

Jakob Egger <jakob(at)eggerapps(dot)at> writes:
> Then I upgraded the "Comand Line Tools for Xcode 10.1" using Software Update, and suddenly the "-isysroot" flag disappeared (for both Xcode 10 and Xcode 10.1).

Interesting. I'd not tested the intermediate state without the CLT
update.

> In any case, it looks like we can't rely on "-isysroot" being present.

Yeah, that's becoming clear, even if it's not clear exactly when it's
pre-supplied or not.

> Regarding #1:
> Since -isysroot is only necessary for building pl/tcl and pl/perl, why do you add it to CPPFLAGS in the global makefile?
> Wouldn't it make more sense to just add it in the TCL and Perl makefiles?

Well, that's what #2 is, but the point is that #2 is a bit of a kluge that
might not work for much longer anyway. If Apple has moved Perl's include
files to only appear under the sysroot, what's the reason to think they
will stop there? It's pretty clear from documentation already cited in
this thread that they think specifying a sysroot path is good practice;
so I fear it's likely that they will stop providing much of anything under
/usr/include, except maybe the bare minimum required by POSIX. An example
that's particularly likely to bite us soon is the headers for Secure
Transport, which isn't POSIX. While we can get away with having separate
include switches for Perl and Tcl, I don't think we're going to have an
appetite for trying to apply a separate include switch for ST to just
our SSL-related .c files.

The bigger issue though is that on a machine with multiple sysroots
installed, approach #2 creates a very severe risk that portions of
Postgres will be built against a different sysroot than other portions.
That will inevitably lead to ABI-compatibility issues. Debugging just
one of those would waste far more time than anyone could ever save by
being able to skip specifying the sysroot.

> I think I would prefer #2.
> I think it would work better for my use case (end users download a binary of the server but may build their own extensions).

Well, it's clear that anybody who's doing that and using a different
sysroot version for their extension is taking their chances with ABI
compatibility. I was willing to avoid breaking that case unnecessarily,
but what we're now seeing is that doing so means exposing everybody else
to those same ABI hazards. I think it's probably better to have the
explicit sysroot setting, provide a way to override it at build time
(as patch #1 does), and just make it clear that if you override it
you are taking chances.

regards, tom lane