Thread: [PATCH] Print error when libpq-refs-stamp fails

> 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.

> 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; \
+    fi

Could 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/