pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedBogus HAVE_DECL_FOO entries in msvc/Solution.pm
7+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2021-07-15 e529b2dc37 91a1651057 081e86bd9e Ensure HAVE_DECL_XXX macros in MSVC builds match those in Unix.
* Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-12 23:46 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 7+ messages in thread
From: Tom Lane @ 2021-07-12 23:46 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Autoconf's AC_CHECK_DECLS always defines HAVE_DECL_whatever
as 1 or 0, but some of the entries in msvc/Solution.pm show
such symbols as "undef" instead. Shouldn't we fix it as
per attached? This is probably only cosmetic at the moment,
but it could bite us someday if someone wrote a complex
conditional using one of these symbols.
These apparently-bogus values date to Peter's 8f4fb4c64,
which created that table; but AFAICS it was just faithfully
emulating the previous confused state of affairs.
regards, tom lane
Attachments:
fix-bogus-HAVE_DECL-values.patch (text/x-diff, 1.4K) download
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 294b968dcd..cfda5ac185 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -239,18 +239,18 @@ sub GenerateFiles
HAVE_CRYPTO_LOCK => undef,
HAVE_DECL_FDATASYNC => 0,
HAVE_DECL_F_FULLFSYNC => 0,
- HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER => undef,
- HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER => undef,
+ HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER => 0,
+ HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER => 0,
HAVE_DECL_LLVMGETHOSTCPUNAME => 0,
HAVE_DECL_LLVMGETHOSTCPUFEATURES => 0,
HAVE_DECL_LLVMORCGETSYMBOLADDRESSIN => 0,
- HAVE_DECL_POSIX_FADVISE => undef,
+ HAVE_DECL_POSIX_FADVISE => 0,
HAVE_DECL_PREADV => 0,
HAVE_DECL_PWRITEV => 0,
HAVE_DECL_RTLD_GLOBAL => 0,
HAVE_DECL_RTLD_NOW => 0,
- HAVE_DECL_STRLCAT => undef,
- HAVE_DECL_STRLCPY => undef,
+ HAVE_DECL_STRLCAT => 0,
+ HAVE_DECL_STRLCPY => 0,
HAVE_DECL_STRNLEN => 1,
HAVE_DECL_STRTOLL => 1,
HAVE_DECL_STRTOULL => 1,
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 01:56 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 7+ messages in thread
From: Michael Paquier @ 2021-07-13 01:56 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
On Mon, Jul 12, 2021 at 07:46:32PM -0400, Tom Lane wrote:
> Autoconf's AC_CHECK_DECLS always defines HAVE_DECL_whatever
> as 1 or 0, but some of the entries in msvc/Solution.pm show
> such symbols as "undef" instead. Shouldn't we fix it as
> per attached? This is probably only cosmetic at the moment,
> but it could bite us someday if someone wrote a complex
> conditional using one of these symbols.
Hmm. I have not tested, but agreed that this is inconsistent. I
would tend to vote for a backpatch to keep some consistency across the
branches as changes in this area could easily lead to rather conflicts
harder to parse.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 04:17 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 7+ messages in thread
From: Peter Eisentraut @ 2021-07-13 04:17 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org
On 13.07.21 01:46, Tom Lane wrote:
> Autoconf's AC_CHECK_DECLS always defines HAVE_DECL_whatever
> as 1 or 0, but some of the entries in msvc/Solution.pm show
> such symbols as "undef" instead. Shouldn't we fix it as
> per attached? This is probably only cosmetic at the moment,
> but it could bite us someday if someone wrote a complex
> conditional using one of these symbols.
Yes, I think that is correct.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 04:25 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 7+ messages in thread
From: Tom Lane @ 2021-07-13 04:25 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Jul 12, 2021 at 07:46:32PM -0400, Tom Lane wrote:
>> Autoconf's AC_CHECK_DECLS always defines HAVE_DECL_whatever
>> as 1 or 0, but some of the entries in msvc/Solution.pm show
>> such symbols as "undef" instead. Shouldn't we fix it as
>> per attached? This is probably only cosmetic at the moment,
>> but it could bite us someday if someone wrote a complex
>> conditional using one of these symbols.
> Hmm. I have not tested, but agreed that this is inconsistent. I
> would tend to vote for a backpatch to keep some consistency across the
> branches as changes in this area could easily lead to rather conflicts
> harder to parse.
That's easy enough in v13 and up, which have 8f4fb4c64 so that
Solution.pm looks like this. We could make it consistent in older
branches by manually hacking pg_config.h.win32 ... but I'm wondering
if the smarter plan wouldn't be to back-patch 8f4fb4c64. Without
that, we're at risk of messing up anytime we back-patch something
that involves a change in the set of configure-defined symbols, which
we do with some regularity.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 07:53 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 7+ messages in thread
From: Michael Paquier @ 2021-07-13 07:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
On Tue, Jul 13, 2021 at 12:25:06AM -0400, Tom Lane wrote:
> That's easy enough in v13 and up, which have 8f4fb4c64 so that
> Solution.pm looks like this. We could make it consistent in older
> branches by manually hacking pg_config.h.win32 ... but I'm wondering
> if the smarter plan wouldn't be to back-patch 8f4fb4c64. Without
> that, we're at risk of messing up anytime we back-patch something
> that involves a change in the set of configure-defined symbols, which
> we do with some regularity.
I was thinking to just do the easiest move and fix this issue down to
13, not bothering about older branches :p
Looking at the commit, a backpatch is not that complicated and it is
possible to check the generation of pg_config.h on non-MSVC
environments if some objects are missing. Still, I think that it
would be better to be careful and test this code properly on Windows
with a real build. It means that.. Err... Andrew or I should look
at that. I am not sure that the potential maintenance gain is worth
poking at the stable branches, to be honest.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 13:58 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 7+ messages in thread
From: Tom Lane @ 2021-07-13 13:58 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jul 13, 2021 at 12:25:06AM -0400, Tom Lane wrote:
>> That's easy enough in v13 and up, which have 8f4fb4c64 so that
>> Solution.pm looks like this. We could make it consistent in older
>> branches by manually hacking pg_config.h.win32 ... but I'm wondering
>> if the smarter plan wouldn't be to back-patch 8f4fb4c64.
> ... I am not sure that the potential maintenance gain is worth
> poking at the stable branches, to be honest.
Fair enough. I wasn't very eager to do the legwork on that, either,
given that the issue is (so far) only cosmetic.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Bogus HAVE_DECL_FOO entries in msvc/Solution.pm
@ 2021-07-13 15:01 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 7+ messages in thread
From: Peter Eisentraut @ 2021-07-13 15:01 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org
On 13.07.21 09:53, Michael Paquier wrote:
> I was thinking to just do the easiest move and fix this issue down to
> 13, not bothering about older branches :p
>
> Looking at the commit, a backpatch is not that complicated and it is
> possible to check the generation of pg_config.h on non-MSVC
> environments if some objects are missing. Still, I think that it
> would be better to be careful and test this code properly on Windows
> with a real build. It means that.. Err... Andrew or I should look
> at that. I am not sure that the potential maintenance gain is worth
> poking at the stable branches, to be honest.
We have lived with the previous system for a decade, so I think
backpatching this would be a bit excessive.
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2021-07-13 15:01 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 23:46 Bogus HAVE_DECL_FOO entries in msvc/Solution.pm Tom Lane <tgl@sss.pgh.pa.us>
2021-07-13 01:56 ` Michael Paquier <michael@paquier.xyz>
2021-07-13 04:25 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-07-13 07:53 ` Michael Paquier <michael@paquier.xyz>
2021-07-13 13:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-07-13 15:01 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-07-13 04:17 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox