Remove win32ver.rc from version_stamp.pl

Started by Peter Eisentrautabout 6 years ago5 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

This removes another relic from the old nmake-based Windows build.
version_stamp.pl put version number information into win32ver.rc. But
win32ver.rc already gets other version number information from the
preprocessor at build time, so it would make more sense if all version
number information would be handled in the same way and we don't have
two places that do it.

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

Attachments:

0001-Remove-win32ver.rc-from-version_stamp.pl.patchtext/plain; charset=UTF-8; name=0001-Remove-win32ver.rc-from-version_stamp.pl.patch; x-mac-creator=0; x-mac-type=0Download+47-49
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Remove win32ver.rc from version_stamp.pl

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

This removes another relic from the old nmake-based Windows build.
version_stamp.pl put version number information into win32ver.rc. But
win32ver.rc already gets other version number information from the
preprocessor at build time, so it would make more sense if all version
number information would be handled in the same way and we don't have
two places that do it.

This has a minor conflict in Solution.pm according to the cfbot.

In general, while I'm on board with the idea, I wonder whether it
wouldn't be smarter to keep on defining PG_MAJORVERSION as a string,
and just add PG_MAJORVERSION_NUM alongside of it. This would
eliminate some hunks from the patch in places where it's more
convenient to have the version as a string, and it would avoid
what could otherwise be a pretty painful cross-version incompatibility
for extensions. We already provide PG_VERSION in both forms, so
I don't see any inconsistency in doing likewise for PG_MAJORVERSION.

regards, tom lane

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)
Re: Remove win32ver.rc from version_stamp.pl

On 2020-03-01 23:51, Tom Lane wrote:

In general, while I'm on board with the idea, I wonder whether it
wouldn't be smarter to keep on defining PG_MAJORVERSION as a string,
and just add PG_MAJORVERSION_NUM alongside of it. This would
eliminate some hunks from the patch in places where it's more
convenient to have the version as a string, and it would avoid
what could otherwise be a pretty painful cross-version incompatibility
for extensions. We already provide PG_VERSION in both forms, so
I don't see any inconsistency in doing likewise for PG_MAJORVERSION.

Agreed. Here is another patch.

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

Attachments:

v2-0001-Remove-win32ver.rc-from-version_stamp.pl.patchtext/plain; charset=UTF-8; name=v2-0001-Remove-win32ver.rc-from-version_stamp.pl.patch; x-mac-creator=0; x-mac-type=0Download+36-27
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#3)
Re: Remove win32ver.rc from version_stamp.pl

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-03-01 23:51, Tom Lane wrote:

In general, while I'm on board with the idea, I wonder whether it
wouldn't be smarter to keep on defining PG_MAJORVERSION as a string,
and just add PG_MAJORVERSION_NUM alongside of it.

Agreed. Here is another patch.

This version LGTM. (I can't actually test the Windows aspects
of this, but I assume you did.)

I'm wondering a little bit whether it'd be worth back-patching the
additions of the new #defines. That would cut about five years off
the time till they could be relied on by extensions. However,
I'm not sure anyone is eager to rely on them, so it may not be
worth the effort.

regards, tom lane

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: Remove win32ver.rc from version_stamp.pl

On 2020-03-09 15:55, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-03-01 23:51, Tom Lane wrote:

In general, while I'm on board with the idea, I wonder whether it
wouldn't be smarter to keep on defining PG_MAJORVERSION as a string,
and just add PG_MAJORVERSION_NUM alongside of it.

Agreed. Here is another patch.

This version LGTM. (I can't actually test the Windows aspects
of this, but I assume you did.)

committed

I'm wondering a little bit whether it'd be worth back-patching the
additions of the new #defines. That would cut about five years off
the time till they could be relied on by extensions. However,
I'm not sure anyone is eager to rely on them, so it may not be
worth the effort.

I doubt external code really needs these symbols. You can always use
PG_VERSION_NUM.

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