Build system: fix configure script for HAS_SOCKETS#10156
Conversation
e20dda5 to
bce8373
Compare
|
No problems on Solaris. |
damiendoligez
left a comment
There was a problem hiding this comment.
The macro AC_CHECK_FUNCS seems to be designed to handle this kind of thing without the need for a cascade of tests.
bce8373 to
b0005fc
Compare
|
@damiendoligez Thanks for the advice, I updated the PR. |
dra27
left a comment
There was a problem hiding this comment.
What is here is correct - I would however state it slightly differently, changing sockets=false on L1296 to sockets=true and removing the sockets=true lines from L1301, L1304, L1307 and L1309 - i.e. change the assumption to be that we have sockets and then AC_CHECK_FUNCS refutes that assumption. At the moment, sockets is set to false then always set to true and then possibly set back to false.
|
As it fixes a bug, it's definitely worth a |
69b0cfd to
12621a5
Compare
damiendoligez
left a comment
There was a problem hiding this comment.
Very good, thanks.
Good to merge as soon as CI is happy.
dra27
left a comment
There was a problem hiding this comment.
You appear not to have regenerated configure, so I include the layout fix to Changes for the same time!
|
@Octachron - this is a bug fix, is it OK for 4.12? |
12621a5 to
7b21acc
Compare
|
Updated the branch according to reviews ! For reference, I had to downgrade |
That'll teach you to use Arch 😉 |
|
More seriously, there was an 8.5 year gap between autoconf 2.69 and 2.70, so we'll need to look at changing CI at some point.. however, the fact that 2.71 has been released within 2 months suggests that holding at 2.69 might be sensible for a bit. |
7b21acc to
8bcbb48
Compare
My understanding is "no". The signalling was less clear than in previous releases (to me at least) but I think we morally are in the "release candidate" phase, where only major issues can be integrated, at the cost of delaying the release. |
|
(But it could be backported in the release branch after the 4.12 release. Maybe we should fork the 4.12 branch into a temporary 4.12.0 branch at the start of the release-candidate phase?) |
|
Reading the discussion, I am not sure when this fixed bug can be triggered? |
|
For context, I encountered the issue when building |
|
Indeed, the bug was triggered on any OS which happens not to have sockets... of course, these days, that's not many! |
Until now, the
HAS_SOCKETSdefinition ins.his set to1even if sockets are not available.This PR aims to fix the sockets feature detection in the configure script.