Re: [PATCH v3 1/1] Fix detection of preadv/pwritev support for OSX.

From: James Hilliard <james(dot)hilliard1(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: David Steele <david(at)pgmasters(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Sergey Shinderuk <s(dot)shinderuk(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: [PATCH v3 1/1] Fix detection of preadv/pwritev support for OSX.
Date: 2021-03-29 23:31:51
Message-ID: CADvTj4oSfH8qN-v_W1ydbA5uJkChxCj8RntwsH-EStytygtALg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 29, 2021 at 4:10 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On Tue, Mar 30, 2021 at 6:37 AM James Hilliard
> <james(dot)hilliard1(at)gmail(dot)com> wrote:
> > Should it work if I just attach it to the thread like this?
>
> Yes. It automatically tries patches that are attached to threads that
> are registered on commitfest.postgresql.org on 4 OSes, and we can see
> that it succeeded, and we can inspect the configure output and see
> that only the two clang-based systems detected and used the new
> unguarded-availability-new flags and used them.
That sounds right, the flag is clang specific from my understanding.
>
> This should be alphabetised better:
>
> HAVE_PREAD => undef,
> - HAVE_PREADV => undef,
> + HAVE_DECL_PREADV => 0,
> HAVE_PSTAT => undef,
Should I resend with that changed or can it just be fixed when applied?
>
> So the question here is really: do we want to support Apple cross-SDK
> builds, in our configure scripts? It costs very little to switch from
> traditional "does-this-symbol-exist?" tests to testing declarations,
> so no objections here.
Well this adds support for the target availability setting, which applies
to effectively all Apple SDK's, so it's really more of a cross target issue
rather than a cross SDK issue than anything from what I can tell.

Effectively without this change setting MACOSX_DEPLOYMENT_TARGET
would not work properly on any Apple SDK's. Currently postgres essentially
is relying on the command line tools not supporting newer targets than
the host system, which is not something that appears to be guaranteed
at all by Apple from my understanding. This is because the current
detection technique is unable to detect if a symbol is restricted by
MACOSX_DEPLOYMENT_TARGET, so it will essentially always
use the newest SDK symbols even if they are only available to a
MACOSX_DEPLOYMENT_TARGET newer than the configured
deployment target.

Say for example if I want to build for a 10.14 target from a 10.15 host
with the standard 10.15 command line tools, with this change that is
possible simply by setting MACOSX_DEPLOYMENT_TARGET,
otherwise it will only build for a 10.14 target from a SDK that does not
have 10.15 only symbols present at all, even if that SDK has full
support for 10.14 targets.
>
> I doubt people will remember to do this for other new syscall probes
> though, so it might be a matter of discussing it case-by-case when a
> problem shows up. For example, I recently added another new test,
> specifically targeting macOS: pthread_barrier_wait. One day they
> might add it to libSystem and we might need to tweak that one
> similarly.
Yeah, this technique should allow for trivially supporting new symbol
target availability in the OSX SDK fairly easily, as long as you have
the unguarded-availability-new flag set the compile tests respect the
compilers target availability settings if the appropriate header is included.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2021-03-29 23:53:03 Re: Proposal: Save user's original authenticated identity for logging
Previous Message Mark Dilger 2021-03-29 23:16:44 Re: pg_amcheck contrib application