savonet / ocaml-ssl

OCaml SSL bindings.
http://liquidsoap.info/ocaml-ssl/
Other
58 stars 48 forks source link

Configure script should not silently fall back to arbitrary defaults when pkg-config fails #55

Open vbgl opened 5 years ago

vbgl commented 5 years ago

The configuration script uses arbitrary defaults when calling pkg-config fails:

https://github.com/savonet/ocaml-ssl/blob/0.5.9/src/config/discover.ml#L6

These defaults are wrong on some platforms.

What seems worse is that this fall-back is silent: one can successfully build this library and then get arcane linking failure when using it:

Undefined symbols for architecture x86_64:
  "_BIO_ctrl", referenced from:
      _ocaml_ssl_flush in libssl_stubs.a(ssl_stubs.o)
  "_BIO_free", referenced from:
      _ocaml_ssl_ctx_init_dh_from_file in libssl_stubs.a(ssl_stubs.o)
  "_BIO_new_file", referenced from:
      _ocaml_ssl_ctx_init_dh_from_file in libssl_stubs.a(ssl_stubs.o)
  "_DH_free", referenced from:
      _ocaml_ssl_ctx_init_dh_from_file in libssl_stubs.a(ssl_stubs.o)
  "_EC_KEY_free", referenced from:
      _ocaml_ssl_ctx_init_ec_from_named_curve in libssl_stubs.a(ssl_stubs.o)
  "_EC_KEY_new_by_curve_name", referenced from:
      _ocaml_ssl_ctx_init_ec_from_named_curve in libssl_stubs.a(ssl_stubs.o)
  "_ERR_clear_error", referenced from:
      _ocaml_ssl_write in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_write_bigarray in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_write_bigarray_blocking in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_read in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_read_into_bigarray in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_read_into_bigarray_blocking in libssl_stubs.a(ssl_stubs.o)
      _ocaml_ssl_accept in libssl_stubs.a(ssl_stubs.o)

Possible fix: require pkg-config.

anmonteiro commented 1 year ago

I think it's a fair trade-off to require pkg-config. It's well-supported in the platforms I'm aware (OPAM, Nix) and other similar packages that bind to native require it.

Any historical reason why pkg-config is just optional, @smimram / @toots ?

toots commented 1 year ago

I can't think of any. This package is old and perhaps back in the days, pkg-config wasn't as readily available as it is now? Also, on macos or cross-compiled conditions, it's better to fail when pkg-config isn't present I believe b/c finding the right library to use is tricky.