Meson add host_system to PG_VERSION_STR

Started by Juan José Santamaría Flechaabout 3 years ago9 messages
#1Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
1 attachment(s)

Hello all,

As mentioned here [1]/messages/by-id/20221115195318.5v5ynapmkusgyzks@awork3.anarazel.de it might be interesting to complete the returned
information by version() when compiled with meson by including the
host_system.

[1]: /messages/by-id/20221115195318.5v5ynapmkusgyzks@awork3.anarazel.de
/messages/by-id/20221115195318.5v5ynapmkusgyzks@awork3.anarazel.de

Regards,

Juan José Santamaría Flecha

Attachments:

0001-meson-add-host_system-to-PG_VERSION_STR.patchapplication/octet-stream; name=0001-meson-add-host_system-to-PG_VERSION_STR.patchDownload
From af1af961687524b0fb664c41d6be2b09b7494f63 Mon Sep 17 00:00:00 2001
From: Juan Jose Santamaria Flecha <juanjo.santamaria@gmail.com>
Date: Tue, 15 Nov 2022 11:07:23 -0500
Subject: [PATCH] meson add host_system to PG_VERSION_STR

---
 meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 259240f..e569b7a 100644
--- a/meson.build
+++ b/meson.build
@@ -143,8 +143,8 @@ cdata.set_quoted('PACKAGE_TARNAME', 'postgresql')
 
 pg_version += get_option('extra_version')
 cdata.set_quoted('PG_VERSION', pg_version)
-cdata.set_quoted('PG_VERSION_STR', 'PostgreSQL @0@ on @1@, compiled by @2@-@3@'.format(
-  pg_version, build_machine.cpu_family(), cc.get_id(), cc.version()))
+cdata.set_quoted('PG_VERSION_STR', 'PostgreSQL @0@ on @1@-@2@, compiled by @3@-@4@'.format(
+  pg_version, build_machine.cpu_family(), host_system, cc.get_id(), cc.version()))
 cdata.set_quoted('PG_MAJORVERSION', pg_version_major.to_string())
 cdata.set('PG_MAJORVERSION_NUM', pg_version_major)
 cdata.set('PG_MINORVERSION_NUM', pg_version_minor)
-- 
2.11.0

#2Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#1)
Re: Meson add host_system to PG_VERSION_STR

On Wed, Nov 16, 2022 at 12:08:56AM +0100, Juan José Santamaría Flecha wrote:

As mentioned here [1] it might be interesting to complete the returned
information by version() when compiled with meson by including the
host_system.

The meson build provides extra_version, which would be able to do the
same, no? The information would be appended to PG_VERSION_STR through
PG_VERSION.
--
Michael

#3Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Michael Paquier (#2)
Re: Meson add host_system to PG_VERSION_STR

On 16.11.22 01:01, Michael Paquier wrote:

On Wed, Nov 16, 2022 at 12:08:56AM +0100, Juan José Santamaría Flecha wrote:

As mentioned here [1] it might be interesting to complete the returned
information by version() when compiled with meson by including the
host_system.

The meson build provides extra_version, which would be able to do the
same, no? The information would be appended to PG_VERSION_STR through
PG_VERSION.

I think this is meant to achieve parity between the version strings
generated by configure and by meson.

Perhaps some examples before and after on different platforms could be
shown.

#4Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Peter Eisentraut (#3)
Re: Meson add host_system to PG_VERSION_STR

On Wed, Nov 16, 2022 at 10:50 AM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

On 16.11.22 01:01, Michael Paquier wrote:

The meson build provides extra_version, which would be able to do the
same, no? The information would be appended to PG_VERSION_STR through
PG_VERSION.

I think this is meant to achieve parity between the version strings
generated by configure and by meson.

Perhaps some examples before and after on different platforms could be
shown.

Yes, that would make clear what the patch is trying to do. For version() we
get:

Configure:
PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Debian
6.3.0-18+deb9u1) 6.3.0 20170516, 64-bit

Meson:
PostgreSQL 16devel on x86_64, compiled by gcc-6.3.0

Patched:
PostgreSQL 16devel on x86_64-linux, compiled by gcc-6.3.0

Regards,

Juan José Santamaría Flecha

#5Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#2)
1 attachment(s)
Re: Meson add host_system to PG_VERSION_STR

Hi,

On 2022-11-16 09:01:04 +0900, Michael Paquier wrote:

On Wed, Nov 16, 2022 at 12:08:56AM +0100, Juan Jos� Santamar�a Flecha wrote:

As mentioned here [1] it might be interesting to complete the returned
information by version() when compiled with meson by including the
host_system.

The meson build provides extra_version, which would be able to do the
same, no? The information would be appended to PG_VERSION_STR through
PG_VERSION.

I don't really follow: Including the operating system in PG_VERSION_STR,
as we're doing in autoconf, seems orthogonal to extra_version? Adding linux
into extra_version would result in linux showing up in e.g.
SHOW server_version;
which doesn't seem right.

I think there's a further deficiency in the PG_VERSION_STR the meson build
generates - we use the build system's CPU. Autoconf shows $host, not $build.

For comparison, on my machine autoconf shows:
PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc-12 (Debian 12.2.0-9) 12.2.0, 64-bit
whereas with meson we currently end up with
PostgreSQL 16devel on x86_64, compiled by gcc-13.0.0

I still don't think it makes sense to try to copy (or invoke)
config.guess. Particularly when targetting windows, but even just having to
keep updating config.guess in perpituity seems unnecessary.

Given we're looking at improving this, should we also add 32/64-bit piece?

If so, we probably should move building PG_VERSION_STR to later so we can use
SIZEOF_VOID_P - configure.ac does that too.

With extra_version set to -andres the attached results in:

PostgreSQL 16devel-andres on x86_64-linux, compiled by gcc-13.0.0, 64-bit

Greetings,

Andres Freund

Attachments:

version.difftext/x-diff; charset=us-asciiDownload
diff --git i/meson.build w/meson.build
index 058382046e1..843b9e19c6b 100644
--- i/meson.build
+++ w/meson.build
@@ -143,12 +143,11 @@ cdata.set_quoted('PACKAGE_TARNAME', 'postgresql')
 
 pg_version += get_option('extra_version')
 cdata.set_quoted('PG_VERSION', pg_version)
-cdata.set_quoted('PG_VERSION_STR', 'PostgreSQL @0@ on @1@, compiled by @2@-@3@'.format(
-  pg_version, build_machine.cpu_family(), cc.get_id(), cc.version()))
 cdata.set_quoted('PG_MAJORVERSION', pg_version_major.to_string())
 cdata.set('PG_MAJORVERSION_NUM', pg_version_major)
 cdata.set('PG_MINORVERSION_NUM', pg_version_minor)
 cdata.set('PG_VERSION_NUM', pg_version_num)
+# PG_VERSION_STR is built later, it depends compiler test results
 cdata.set_quoted('CONFIGURE_ARGS', '')
 
 
@@ -2435,6 +2434,15 @@ cdata.set('MEMSET_LOOP_LIMIT', memset_loop_limit)
 cdata.set_quoted('DLSUFFIX', dlsuffix)
 
 
+# built later than the rest of the version metadata, we need SIZEOF_VOID_P
+cdata.set_quoted('PG_VERSION_STR',
+  'PostgreSQL @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
+    pg_version, host_machine.cpu_family(), host_system,
+    cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8,
+  )
+)
+
+
 
 ###############################################################
 # Threading
#6Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Andres Freund (#5)
Re: Meson add host_system to PG_VERSION_STR

On Wed, Nov 16, 2022 at 8:02 PM Andres Freund <andres@anarazel.de> wrote:

Given we're looking at improving this, should we also add 32/64-bit piece?

If so, we probably should move building PG_VERSION_STR to later so we can
use
SIZEOF_VOID_P - configure.ac does that too.

With extra_version set to -andres the attached results in:

PostgreSQL 16devel-andres on x86_64-linux, compiled by gcc-13.0.0, 64-bit

WFM.

Regards,

Juan José Santamaría Flecha

#7Michael Paquier
michael@paquier.xyz
In reply to: Juan José Santamaría Flecha (#4)
Re: Meson add host_system to PG_VERSION_STR

On Wed, Nov 16, 2022 at 02:12:05PM +0100, Juan José Santamaría Flecha wrote:

On Wed, Nov 16, 2022 at 10:50 AM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

Perhaps some examples before and after on different platforms could be
shown.

Yes, that would make clear what the patch is trying to do. For version() we
get:

Configure:
PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Debian
6.3.0-18+deb9u1) 6.3.0 20170516, 64-bit

Meson:
PostgreSQL 16devel on x86_64, compiled by gcc-6.3.0

Patched:
PostgreSQL 16devel on x86_64-linux, compiled by gcc-6.3.0

Ah, thanks. I was not following this point. Adding the host
information for consistency makes sense, indeed.
--
Michael

#8Juan José Santamaría Flecha
juanjo.santamaria@gmail.com
In reply to: Michael Paquier (#7)
Re: Meson add host_system to PG_VERSION_STR

On Thu, Nov 17, 2022 at 3:35 AM Michael Paquier <michael@paquier.xyz> wrote:

Ah, thanks. I was not following this point. Adding the host
information for consistency makes sense, indeed.

I've added an entry [1]https://commitfest.postgresql.org/41/4057/ in the commitfest so we don't miss this subject.

[1]: https://commitfest.postgresql.org/41/4057/

Regards,

Juan José Santamaría Flecha

#9Andres Freund
andres@anarazel.de
In reply to: Juan José Santamaría Flecha (#8)
Re: Meson add host_system to PG_VERSION_STR

Hi,

On 2022-12-09 14:53:10 +0100, Juan Jos� Santamar�a Flecha wrote:

I've added an entry [1] in the commitfest so we don't miss this subject.

I indeed had forgotten. Pushed now.

Greetings,

Andres Freund