[PATCH] Print error when libpq-refs-stamp fails
Hello,
While developing I got this error and it was difficult to figure out what
was going on.
Thanks to Jacob, I was able to learn the context of the failure, so we
created this small patch.
The text of the error message, of course, is up for debate, but hopefully
this will make it more clear to others.
Thank you,
Rachel Heaton
Attachments:
0001-Print-error-when-libpq-refs-stamp-fails.patchapplication/octet-stream; name=0001-Print-error-when-libpq-refs-stamp-fails.patchDownload
From c63c102e2609c4ce86e4e3b63ecc5736550ca79b Mon Sep 17 00:00:00 2001
From: Rachel Heaton <rheaton@vmware.com>
Date: Mon, 27 Sep 2021 15:38:06 -0700
Subject: [PATCH] Print error when libpq-refs-stamp fails
The current failure is very difficult to parse without prior context.
Prior context: https://www.postgresql.org/message-id/flat/3128896.1624742969%40sss.pgh.pa.us
Co-authored-by: Jacob Champion <pchampion@vmware.com>
---
src/interfaces/libpq/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 7cbdeb589b..34e972a08b 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -115,7 +115,9 @@ backend_src = $(top_srcdir)/src/backend
libpq-refs-stamp: $(shlib)
ifneq ($(enable_coverage), yes)
ifeq (,$(filter aix solaris,$(PORTNAME)))
- ! nm -A -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit
+ @if nm -a -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit; then \
+ echo 'libpq must not call exit'; exit 1; \
+ fi
endif
endif
touch $@
--
2.28.0
On 28 Sep 2021, at 00:55, Rachel Heaton <rachelmheaton@gmail.com> wrote:
While developing I got this error and it was difficult to figure out what was going on.
Thanks to Jacob, I was able to learn the context of the failure, so we created this small patch.
I can see that, and I think this patch makes sense even though we don't have
much of a precedent for outputting informational messages from Makefiles.
The text of the error message, of course, is up for debate, but hopefully this will make it more clear to others.
+ echo 'libpq must not call exit'; exit 1; \
Since it's not actually libpq which calls exit (no such patch would ever be
committed), I think it would be clearer to indicate that a library linked to is
the culprit. How about something like "libpq must not be linked against any
library calling exit"?
--
Daniel Gustafsson https://vmware.com/
On Tue, Sep 28, 2021 at 6:14 AM Daniel Gustafsson <daniel@yesql.se> wrote:
Since it's not actually libpq which calls exit (no such patch would ever be
committed), I think it would be clearer to indicate that a library linked to is
the culprit. How about something like "libpq must not be linked against any
library calling exit"?
Excellent update to the error message. Patch attached.
Attachments:
0002-Print-error-when-libpq-refs-stamp-fails.patchapplication/octet-stream; name=0002-Print-error-when-libpq-refs-stamp-fails.patchDownload
From 546f66c0ed5bdda5c9fafc9e303bc0cdd9269871 Mon Sep 17 00:00:00 2001
From: Rachel Heaton <rheaton@vmware.com>
Date: Mon, 27 Sep 2021 15:38:06 -0700
Subject: [PATCH] Print error when libpq-refs-stamp fails
The current failure is very difficult to parse without prior context.
Prior context: https://www.postgresql.org/message-id/flat/3128896.1624742969%40sss.pgh.pa.us
Co-authored-by: Jacob Champion <pchampion@vmware.com>
---
src/interfaces/libpq/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 7cbdeb589b..caa013e0f7 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -115,7 +115,9 @@ backend_src = $(top_srcdir)/src/backend
libpq-refs-stamp: $(shlib)
ifneq ($(enable_coverage), yes)
ifeq (,$(filter aix solaris,$(PORTNAME)))
- ! nm -A -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit
+ @if nm -a -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit; then \
+ echo 'libpq must not be linked against any library calling exit'; exit 1; \
+ fi
endif
endif
touch $@
--
2.28.0
On 28 Sep 2021, at 17:52, Rachel Heaton <rachelmheaton@gmail.com> wrote:
Patch attached.
I tweaked the error message a little bit and pushed to master. Thanks!
--
Daniel Gustafsson https://vmware.com/
Hello,
On 28/09/2021 05:55, Rachel Heaton wrote:
Hello,
While developing I got this error and it was difficult to figure out
what was going on.Thanks to Jacob, I was able to learn the context of the failure, so we
created this small patch.
- ! nm -A -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit
+ @if nm -a -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit; then \
+ echo 'libpq must not be linked against any library calling exit';
exit 1; \
+ fiCould you please confirm that the change from -A to -a in nm arguments
in this patch is intentional?
--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru
On Mon, 2021-10-04 at 23:40 +0700, Anton Voloshin wrote:
Could you please confirm that the change from -A to -a in nm arguments
in this patch is intentional?
That was not intended by us, thank you for the catch! A stray
lowercasing in vim, perhaps.
--Jacob
On 4 Oct 2021, at 19:02, Jacob Champion <pchampion@vmware.com> wrote:
On Mon, 2021-10-04 at 23:40 +0700, Anton Voloshin wrote:
Could you please confirm that the change from -A to -a in nm arguments
in this patch is intentional?That was not intended by us, thank you for the catch! A stray
lowercasing in vim, perhaps.
Hmm, I will take care of this shortly.
/ Daniel
On 4 Oct 2021, at 19:21, Daniel Gustafsson <daniel@yesql.se> wrote:
On 4 Oct 2021, at 19:02, Jacob Champion <pchampion@vmware.com> wrote:
On Mon, 2021-10-04 at 23:40 +0700, Anton Voloshin wrote:
Could you please confirm that the change from -A to -a in nm arguments
in this patch is intentional?That was not intended by us, thank you for the catch! A stray
lowercasing in vim, perhaps.Hmm, I will take care of this shortly.
Right, so I missed this in reviewing and testing, and I know why the latter
didn't catch it. nm -A and -a outputs the same thing *for this input* on my
Debian and macOS boxes, with the small difference that -A prefixes the line
with the name of the input file. -a also include debugger symbols, but for
this usage it didn't alter the results. I will go ahead and fix this, thanks
for catching it!
--
Daniel Gustafsson https://vmware.com/
Thanks to you both!
From: Daniel Gustafsson <daniel@yesql.se>
Date: Monday, October 4, 2021 at 11:36 AM
To: Jacob Champion <pchampion@vmware.com>
Cc: Rachel Heaton <rachelmheaton@gmail.com>, pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>, a.voloshin@postgrespro.ru <a.voloshin@postgrespro.ru>
Subject: Re: [PATCH] Print error when libpq-refs-stamp fails
On 4 Oct 2021, at 19:21, Daniel Gustafsson <daniel@yesql.se> wrote:
On 4 Oct 2021, at 19:02, Jacob Champion <pchampion@vmware.com> wrote:
On Mon, 2021-10-04 at 23:40 +0700, Anton Voloshin wrote:
Could you please confirm that the change from -A to -a in nm arguments
in this patch is intentional?That was not intended by us, thank you for the catch! A stray
lowercasing in vim, perhaps.Hmm, I will take care of this shortly.
Right, so I missed this in reviewing and testing, and I know why the latter
didn't catch it. nm -A and -a outputs the same thing *for this input* on my
Debian and macOS boxes, with the small difference that -A prefixes the line
with the name of the input file. -a also include debugger symbols, but for
this usage it didn't alter the results. I will go ahead and fix this, thanks
for catching it!
--
Daniel Gustafsson https://vmware.com/