Thread

Commits

  1. meson: Add basic PGXS compatibility

  2. autoconf: Move export_dynamic determination to configure

  3. autoconf: Don't AC_SUBST() LD in configure

  4. autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C

  5. autoconf: Rely on ar supporting index creation

  1. meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-05T20:07:10Z

    Hi,
    
    As the meson support stands right now, one cannot easily build an extension
    against a postgres built with meson. As discussed at the 2022 unconference
    [1], one way to make that easier for extensions is for meson to generate a
    complete enough Makefile.global for pgxs.mk to work.
    
    This is a series of patches towards that goal. The first four simplify some
    aspects of Makefile.global, and then the fifth generates Makefile.global etc.
    
    
    0001: autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
    
      Right now emit the cflags to build the CRC objects into architecture specific
      variables. That doesn't make a whole lot of sense to me - we're never going to
      target x86 and arm at the same time, so they don't need to be separate
      variables.
    
      It might be better to instead continue to have CFLAGS_SSE42 /
      CFLAGS_ARMV8_CRC32C be computed by PGAC_ARMV8_CRC32C_INTRINSICS /
      PGAC_SSE42_CRC32_INTRINSICS and then set CFLAGS_CRC based on those. But it
      seems unlikely that we'd need other sets of flags for those two architectures
      at the same time.
    
      The separate flags could be supported by the meson build instead, it'd just
      add unneccessary complexity.
    
    
    0002: autoconf: Rely on ar supporting index creation
    
      With meson we don't require ranlib. But as it is set in Makefile.global and
      used by several platforms, we'd need to detect it.
    
      FreeBSD, NetBSD, OpenBSD, the only platforms were we didn't use AROPT=crs,
      all have supported the 's' option for a long time.
    
      On macOS we ran ranlib after installing a static library. This was added a
      long time ago, in 58ad65ec2def. I cannot reproduce an issue in more recent
      macOS versions.
    
      I'm on the fence about removing the "touch $@" from the rule building static
      libs. That was added because of macos's ranlib not setting fine-grained
      timestamps. On a modern mac ar and ranlib are the same binary, and maybe
      that means that ar has the same issue? Both do set fine-grained
      timestamps:
    
      cc ../test.c -o test.o -c
      rm -f test.a; ar csr test.a test.o ; ranlib test.a; python3 -c "import os;print(os.stat('test.a').st_mtime_ns)"
      1664999109090448534
    
      But I don't know how far back that goes. We could just reformulate the
      comment to mention ar instead of ranlib.
    
    
      Tom, CCing you due to 58ad65ec2def and 826eff57c4c.
    
    
    0003: aix: Build SUBSYS.o using $(CC) -r instead of $(LD) -r
    
      This is the only direct use of $(LD), and xlc -r and gcc -r end up with the
      same set of symbols and similar performance (noise is high, so hard to say if
      equivalent).
    
      Now that $(LD) isn't needed anymore, remove it from src/Makefile.global
    
      While at it, add a comment why -r is used.
    
    
    0004: solaris: Check for -Wl,-E directly instead of checking for gnu LD
    
      This allows us to get rid of the nontrivial detection of with_gnu_ld,
      simplifying meson PGXS compatibility. It's also nice to delete libtool.m4.
    
      I don't like the SOLARIS_EXPORT_DYNAMIC variable I invented. If somebody has
      a better idea...
    
    
    0005: meson: Add PGXS compatibility
    
      The actual meson PGXS compatibility. Plenty more replacements to do, but
      suffices to build common extensions on a few platforms.
    
      What level of completeness do we want to require here?
    
    
      A few replacements worth thinking about:
    
      - autodepend - I'm inclined to set it to true when using a gcc like
        compiler. I think extension authors won't be happy if suddenly their
        extensions don't rebuild reliably anymore. An --enable-depend like
        setting doesn't make sense for meson, so we don't have anything to source it
        from.
      - {LDAP,UUID,ICU}_{LIBS,CFLAGS} - might some extension need them?
    
    
      For some others I think it's ok to not have replacement. Would be good for
      somebody to check my thinking though:
    
      - LIBOBJS, PG_CRC32C_OBJS, TAS: Not needed because we don't build
        the server / PLs with the generated makefile
      - ZIC: only needed to build tzdata as part of server build
      - MSGFMT et al: translation doesn't appear to be supported by pgxs, correct?
      - XMLLINT et al: docs don't seem to be supported by pgxs
      - GENHTML et al: supporting coverage for pgxs-in-meson build doesn't seem worth it
      - WINDRES: I don't think extensions are bothering to generate rc files on windows
    
    
    My colleague Bilal has set up testing and verified that a few extensions build
    with the pgxs compatibility layer, on linux at last. Currently pg_qualstats,
    pg_cron, hypopg, orafce, postgis, pg_partman work. He also tested pgbouncer,
    but for him that failed both with autoconf and meson generated pgxs.
    
    I wonder if and where we could have something like this tested continually?
    
    Greetings,
    
    Andres Freund
    
    [1] https://wiki.postgresql.org/wiki/PgCon_2022_Developer_Unconference#Meson_new_build_system_proposal
    
  2. Re: meson PGXS compatibility

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-10-05T20:20:22Z

    Andres Freund <andres@anarazel.de> writes:
    >   On macOS we ran ranlib after installing a static library. This was added a
    >   long time ago, in 58ad65ec2def. I cannot reproduce an issue in more recent
    >   macOS versions.
    
    I agree that shouldn't be necessary anymore (and if it is, we'll find
    out soon enough).
    
    >   I'm on the fence about removing the "touch $@" from the rule building static
    >   libs. That was added because of macos's ranlib not setting fine-grained
    >   timestamps. On a modern mac ar and ranlib are the same binary, and maybe
    >   that means that ar has the same issue? Both do set fine-grained
    >   timestamps:
    
    Please see the commit message for 826eff57c4c: the issue seems to arise
    only with specific combinations of software, in particular with non-Apple
    versions of "make" (although maybe later Apple builds have fixed make's
    failure to read sub-second timestamps?).  That's a relatively recent hack,
    and I'm very hesitant to conclude that we don't need it anymore just
    because you failed to reproduce an issue locally.  It very possibly isn't
    a problem in a meson build, though, depending on how much meson depends on
    file timestamps.
    
    			regards, tom lane
    
    
    
    
  3. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-05T20:49:44Z

    Hi,
    
    On 2022-10-05 16:20:22 -0400, Tom Lane wrote:
    > Andres Freund <andres@anarazel.de> writes:
    > >   On macOS we ran ranlib after installing a static library. This was added a
    > >   long time ago, in 58ad65ec2def. I cannot reproduce an issue in more recent
    > >   macOS versions.
    > 
    > I agree that shouldn't be necessary anymore (and if it is, we'll find
    > out soon enough).
    
    Cool.
    
    
    > >   I'm on the fence about removing the "touch $@" from the rule building static
    > >   libs. That was added because of macos's ranlib not setting fine-grained
    > >   timestamps. On a modern mac ar and ranlib are the same binary, and maybe
    > >   that means that ar has the same issue? Both do set fine-grained
    > >   timestamps:
    > 
    > Please see the commit message for 826eff57c4c: the issue seems to arise
    > only with specific combinations of software, in particular with non-Apple
    > versions of "make" (although maybe later Apple builds have fixed make's
    > failure to read sub-second timestamps?).
    
    My understanding, from that commit message, was that the issue originates in
    apple's ranlib setting the timestamp to its components but only queries / sets
    it using second granularity. I verified that apple's ranlib and ar these days
    just set the current time, at a high granularity, as the mtime.  Whether or
    not make then hides the problem seems not that relevant if the source of the
    problem is gone, no?
    
    
    > That's a relatively recent hack, and I'm very hesitant to conclude that we
    > don't need it anymore just because you failed to reproduce an issue locally.
    
    Yea, that's why I was hesitant as well. I'll reformulate the comment to
    reference ar instead of ranlib instead.
    
    
    > It very possibly isn't a problem in a meson build, though, depending on how
    > much meson depends on file timestamps.
    
    Most of the timestamp sensitive stuff is dealt with by ninja, rather than
    meson. ninja does take timestamps into account when determining what to
    rebuild - although I suspect this specific problem wouldn't occur even with a
    problematic ar/ranlib version, because the relevant timestamps will be on the
    .c (etc) files, rather than the .a. Ninja has the whole dependency graph, so
    it knows what dependencies it has to rebuild, without needing to check
    timestamps of intermediary objects.
    
    Ninja does support build rules where it checks the timestamps of build outputs
    to see if targets depending on those build outputs have to be rebuilt, or not,
    because the target didn't change. But the relevant option ("restat") isn't set
    for compiler / linker invocations in the build.ninja meson generates.
    
    Restat is however set for the "custom_command"s we use to generate all kinds
    of sources. Sometimes that leads to the set of build steps shrinking
    rapidly. E.g. a touch src/include/catalog/pg_namespace.dat starts leads to
    ninja considering 1135 targets out of date, but as genbki.pl doesn't end up
    changing any files, it's done immediately after that...
    
    [1/1135 1   0%] Generating src/include/catalog/generated_catalog_headers with a custom command
    
    Greetings,
    
    Andres Freund
    
    
    
    
  4. Re: meson PGXS compatibility

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-10-05T20:58:46Z

    Andres Freund <andres@anarazel.de> writes:
    > My understanding, from that commit message, was that the issue originates in
    > apple's ranlib setting the timestamp to its components but only queries / sets
    > it using second granularity. I verified that apple's ranlib and ar these days
    > just set the current time, at a high granularity, as the mtime.  Whether or
    > not make then hides the problem seems not that relevant if the source of the
    > problem is gone, no?
    
    Well, (a) it seemed to happen in only some circumstances even back then,
    so maybe your testing didn't catch it; and (b) even assuming that Apple
    has fixed it in recent releases, there may still be people using older,
    un-fixed versions.  Why's it such a problem to keep the "touch" step?
    
    			regards, tom lane
    
    
    
    
  5. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-05T21:10:22Z

    Hi,
    
    On 2022-10-05 16:58:46 -0400, Tom Lane wrote:
    > Andres Freund <andres@anarazel.de> writes:
    > > My understanding, from that commit message, was that the issue originates in
    > > apple's ranlib setting the timestamp to its components but only queries / sets
    > > it using second granularity. I verified that apple's ranlib and ar these days
    > > just set the current time, at a high granularity, as the mtime.  Whether or
    > > not make then hides the problem seems not that relevant if the source of the
    > > problem is gone, no?
    > 
    > Well, (a) it seemed to happen in only some circumstances even back then,
    > so maybe your testing didn't catch it; and (b) even assuming that Apple
    > has fixed it in recent releases, there may still be people using older,
    > un-fixed versions.  Why's it such a problem to keep the "touch" step?
    
    It isn't! That's why I said that I was on the fence about removing the touch
    in my first email and then that I'd leave the touch there and just
    s/ranlib/ar/ in my reply to you.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  6. Re: meson PGXS compatibility

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-10-06T09:34:26Z

    On 05.10.22 22:07, Andres Freund wrote:
    > My colleague Bilal has set up testing and verified that a few extensions build
    > with the pgxs compatibility layer, on linux at last. Currently pg_qualstats,
    > pg_cron, hypopg, orafce, postgis, pg_partman work. He also tested pgbouncer,
    > but for him that failed both with autoconf and meson generated pgxs.
    
    pgbouncer doesn't use pgxs.
    
    
    
    
  7. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-06T19:11:53Z

    Hi,
    
    On 2022-10-06 11:34:26 +0200, Peter Eisentraut wrote:
    > On 05.10.22 22:07, Andres Freund wrote:
    > > My colleague Bilal has set up testing and verified that a few extensions build
    > > with the pgxs compatibility layer, on linux at last. Currently pg_qualstats,
    > > pg_cron, hypopg, orafce, postgis, pg_partman work. He also tested pgbouncer,
    > > but for him that failed both with autoconf and meson generated pgxs.
    > 
    > pgbouncer doesn't use pgxs.
    
    Ah, right. It'd still be interesting to make sure it works, but looks like the
    only integration point is pg_config --includedir and pg_config --libdir, so
    it should...
    
    Greetings,
    
    Andres Freund
    
    
    
    
  8. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-07T19:16:08Z

    Hi,
    
    On 2022-10-05 13:07:10 -0700, Andres Freund wrote:
    > 0003: aix: Build SUBSYS.o using $(CC) -r instead of $(LD) -r
    >
    >   This is the only direct use of $(LD), and xlc -r and gcc -r end up with the
    >   same set of symbols and similar performance (noise is high, so hard to say if
    >   equivalent).
    >
    >   Now that $(LD) isn't needed anymore, remove it from src/Makefile.global
    >
    >   While at it, add a comment why -r is used.
    
    Unfortunately experimenting further with this it turns out I was wrong: While
    xlc -r results in the same set of symbols, that's not true with gcc -r, at
    least with some versions of gcc. gcc ends up exposing some of the libgcc
    symbols.
    
    That can be rectified by adding -nostartfiles -nodefaultlibs, but that
    basically makes the change as-is pointless.
    
    I think it'd still be good to get rid of setting LD via configure.ac,
    mirroring the detection logic in meson sounds like a bad plan.
    
    Given this is aix specific, and only the aix linker works on aix (binutils'
    doesn't), I think the best plan might be to just hardcode ld in the rule
    generating postgres.imp.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  9. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-10T21:35:14Z

    Hi,
    
    On 2022-10-05 13:07:10 -0700, Andres Freund wrote:
    > 0004: solaris: Check for -Wl,-E directly instead of checking for gnu LD
    > 
    >   This allows us to get rid of the nontrivial detection of with_gnu_ld,
    >   simplifying meson PGXS compatibility. It's also nice to delete libtool.m4.
    > 
    >   I don't like the SOLARIS_EXPORT_DYNAMIC variable I invented. If somebody has
    >   a better idea...
    
    A cleaner approach could be to add a LDFLAGS_BE and emit the -Wl,-E into it
    from both configure and meson, solely based on whether -Wl,-E is supported.  I
    haven't verified cygwin, but on our other platforms that seems to do the right
    thing.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  10. Re: meson PGXS compatibility

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-10-12T05:50:07Z

    On 05.10.22 22:07, Andres Freund wrote:
    > 0001: autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
    
    I wonder, there has been some work lately to use SIMD and such in other 
    places.  Would that affect what kinds of extra CPU-specific compiler 
    flags and combinations we might need?
    
    Seems fine otherwise.
    > 0005: meson: Add PGXS compatibility
    > 
    >    The actual meson PGXS compatibility. Plenty more replacements to do, but
    >    suffices to build common extensions on a few platforms.
    > 
    >    What level of completeness do we want to require here?
    
    I have tried this with a few extensions.  Seems to work alright.  I 
    don't think we need to overthink this.  The way it's set up, if someone 
    needs additional variables set, they can easily be added.
    
    
    
    
    
  11. Re: meson PGXS compatibility

    John Naylor <john.naylor@enterprisedb.com> — 2022-10-12T06:53:05Z

    On Wed, Oct 12, 2022 at 12:50 PM Peter Eisentraut <
    peter.eisentraut@enterprisedb.com> wrote:
    >
    > On 05.10.22 22:07, Andres Freund wrote:
    > > 0001: autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
    >
    > I wonder, there has been some work lately to use SIMD and such in other
    > places.  Would that affect what kinds of extra CPU-specific compiler
    > flags and combinations we might need?
    
    In short, no. The functionality added during this cycle only uses
    instructions available by default on the platform. CRC on Arm depends on
    armv8-a+crc, and CRC on x86-64 depends on SSE4.2. We can't assume those
    currently.
    
    --
    John Naylor
    EDB: http://www.enterprisedb.com
    
  12. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-13T05:16:48Z

    Hi,
    
    On 2022-10-10 14:35:14 -0700, Andres Freund wrote:
    > On 2022-10-05 13:07:10 -0700, Andres Freund wrote:
    > > 0004: solaris: Check for -Wl,-E directly instead of checking for gnu LD
    > > 
    > >   This allows us to get rid of the nontrivial detection of with_gnu_ld,
    > >   simplifying meson PGXS compatibility. It's also nice to delete libtool.m4.
    > > 
    > >   I don't like the SOLARIS_EXPORT_DYNAMIC variable I invented. If somebody has
    > >   a better idea...
    > 
    > A cleaner approach could be to add a LDFLAGS_BE and emit the -Wl,-E into it
    > from both configure and meson, solely based on whether -Wl,-E is supported.  I
    > haven't verified cygwin, but on our other platforms that seems to do the right
    > thing.
    
    I think that does look better. See the attached 0003.
    
    
    The attached v3 of this patch has an unchanged 0001 (CRC cflags).
    
    For 0002, I still removed LD from Makefile.global, but instead just hardcoded
    ld in the export file generation portion of the backend build - there's no
    working alternative linkers, and we already hardcode a bunch of other paths in
    mkldexport.
    
    0003 is changed significantly - as proposed in the message quoted above, I
    introduced LDFLAGS_EX_BE and moved the detection -Wl,-E (I used
    -Wl,--export-dynamic, which we previously only used on FreeBSD) into
    configure, getting rid of export_dynamic.
    
    0004, the patch introducing PGXS compat, saw a few changes too:
    - I implemented one of the FIXMEs, the correct determination of strip flags
    - I moved the bulk of the pgxs compat code to src/makefiles/meson.build - imo
      src/meson.build got bulked up too much with pgxs-emulation code
    - some minor cleanups
    
    Greetings,
    
    Andres Freund
    
  13. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-10-13T05:23:40Z

    Hi,
    
    On 2022-10-12 07:50:07 +0200, Peter Eisentraut wrote:
    > On 05.10.22 22:07, Andres Freund wrote:
    > > 0001: autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
    > 
    > I wonder, there has been some work lately to use SIMD and such in other
    > places.  Would that affect what kinds of extra CPU-specific compiler flags
    > and combinations we might need?
    
    The current infrastructure is very CRC specific, so I don't think this change
    would stand in the way of using sse specific flags in other places.
    
    
    > > 0005: meson: Add PGXS compatibility
    > > 
    > >    The actual meson PGXS compatibility. Plenty more replacements to do, but
    > >    suffices to build common extensions on a few platforms.
    > > 
    > >    What level of completeness do we want to require here?
    > 
    > I have tried this with a few extensions.  Seems to work alright.  I don't
    > think we need to overthink this.  The way it's set up, if someone needs
    > additional variables set, they can easily be added.
    
    Yea, I am happy enough with it now that the bulk is out of src/meson.build and
    strip isn't set to an outright wrong value.
    
    Thanks!
    
    Andres
    
    
    
    
  14. Re: meson PGXS compatibility

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-12-01T07:43:19Z

    On 13.10.22 07:23, Andres Freund wrote:
    >>> 0005: meson: Add PGXS compatibility
    >>>
    >>>     The actual meson PGXS compatibility. Plenty more replacements to do, but
    >>>     suffices to build common extensions on a few platforms.
    >>>
    >>>     What level of completeness do we want to require here?
    >>
    >> I have tried this with a few extensions.  Seems to work alright.  I don't
    >> think we need to overthink this.  The way it's set up, if someone needs
    >> additional variables set, they can easily be added.
    > 
    > Yea, I am happy enough with it now that the bulk is out of src/meson.build and
    > strip isn't set to an outright wrong value.
    
    How are you planning to proceed with this?  I thought it was ready, but 
    it hasn't moved in a while.
    
    
    
    
    
  15. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-12-01T20:17:51Z

    Hi,
    
    On 2022-12-01 08:43:19 +0100, Peter Eisentraut wrote:
    > On 13.10.22 07:23, Andres Freund wrote:
    > > > > 0005: meson: Add PGXS compatibility
    > > > > 
    > > > >     The actual meson PGXS compatibility. Plenty more replacements to do, but
    > > > >     suffices to build common extensions on a few platforms.
    > > > > 
    > > > >     What level of completeness do we want to require here?
    > > > 
    > > > I have tried this with a few extensions.  Seems to work alright.  I don't
    > > > think we need to overthink this.  The way it's set up, if someone needs
    > > > additional variables set, they can easily be added.
    > > 
    > > Yea, I am happy enough with it now that the bulk is out of src/meson.build and
    > > strip isn't set to an outright wrong value.
    > 
    > How are you planning to proceed with this?  I thought it was ready, but it
    > hasn't moved in a while.
    
    I basically was hoping for a review of the prerequisite patches I posted at
    [1], particularly 0003 (different approach than before, comparatively large).
    
    Here's an updated version of the patches. There was a stupid copy-paste bug in
    the prior version of the 0003 / export_dynamic patch.
    
    I'll push 0001, 0002 shortly. I don't think 0002 is the most elegant approach,
    but it's not awful. I'd still like some review for 0003, but will try to push
    it in a few days if that's not forthcoming.
    
    Greetings,
    
    Andres Freund
    
    [1] https://www.postgresql.org/message-id/20221013051648.ufz7ud2b5tioctyt%40awork3.anarazel.de
    
  16. Re: meson PGXS compatibility

    Andres Freund <andres@anarazel.de> — 2022-12-07T03:05:53Z

    Hi,
    
    On 2022-12-01 12:17:51 -0800, Andres Freund wrote:
    > I'll push 0001, 0002 shortly. I don't think 0002 is the most elegant approach,
    > but it's not awful. I'd still like some review for 0003, but will try to push
    > it in a few days if that's not forthcoming.
    
    Pushed 0003 (move export_dynamic determination to configure.ac) and 0004 (PGXS
    compatibility). Hope there's no fallout from 0003.
    
    Greetings,
    
    Andres Freund