On Mon, Jan 15, 2018 at 02:20:01PM +0100, Andrea Bolognani wrote:
On Mon, 2018-01-15 at 13:31 +0100, Michal Privoznik wrote:
So, one way to solve this once and for all would be to:
* try looking up readline through pkg-config. If that works, then we already know we're compiling against a recent readline version and everything will work;
* if readline's pkg-config file is not available, try linking against it the old way. This will succeed on oldish versions like the one shipped with CentOS but fail because of missing functions on macOS.
I could try cooking up something like the above, but I can never seem to get it right the first couple of times when m4 is involved, so in the interest of time - and not having to merge this patch you hate ;) - would you mind looking into it yourself instead?
The reason I hate this patch is not because the patch itself looks ugly. It's because we have to deal with the situation in the first place and invest our time in resolving it. And what you're suggesting might sound right but we'll end up with the same situation after all.
I don't think that's the case. Right now we have to work around issues in macOS all the time because we're linking against the obsolete readline version included in the base system; if we implemented what I propose above, then we could just mandate that readline 6.0 or newer is required. macOS builds would then fail unless you install a recent readline using brew, but that's entirely acceptable, and we would have obtained a reasonable baseline to work against going forward. Plus it would allow us to get rid of some nasty hacks[1] on our CI as well :)
IMHO it is entirely reasonable to require modern readline when building on macOS, and not try to support the one in bsae system. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|