-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-91962: Fix hstrerror
detection issues on Solaris
#91963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3964610
to
29fb29f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Let's merge after alpha 4 is cut, so if something's wrong we don't block the release process.
I'm not sure if this is the correct fix; please hold on merging it. |
I tested it on Solaris and it seems to work as expected, but I won't pretend that I am an autotools expert and it's perfectly possible that I overlooked something. |
There is an existing |
62d527a
to
f2a9799
Compare
I presume you mean the That said, there was indeed a slight issue with the detection - it wouldn't work correctly on a system where |
@erlend-aasland, does this look good? |
040652c
to
3cfc710
Compare
I just finished working on an updated version of the check - the old one stopped working at some point, because extensions are no longer using This one keeps |
I plan to merge this tomorrow if there are no objections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to merge this tomorrow if there are no objections.
With this PR, HAVE_LIBRESOLV
will never be defined in pyconfig.h
anymore. I don't think that will be a problem; just a heads-up. We don't use HAVE_LIBRESOLV
in our code base.
My gut feeling is that the general idea is OK: Alas, if you link |
You can just modify the action-if-true block to explicitly define |
I can do so, but is it really necessary considering that |
It is also exported in |
Thank you both very much! |
Configure only checks whether
inet_aton
needs-lresolv
but it also needs to checkhstrerror
(because on Solaris,inet_aton
no longer needs-lresolv
, buthstrerror
does).Issue #91962 has more detailed explanation.