search

mmottl / postgresql-ocaml

OCaml-bindings for the PostgreSQL database
Other
141 stars 23 forks source link

Question about discover.ml #25

Closed rgrinberg closed 6 years ago

rgrinberg commented 6 years ago

I had a look over your discover.ml script and glancing over these lines:

https://github.com/mmottl/postgresql-ocaml/blob/916bb7bfa3b5f1bea9767858b44d22ac1bed5c7f/src/config/discover.ml#L54-L64

I find this code a bit weird. We're constructing this conf record simply to access its fields. Which doesn't seem too useful.Are you forgetting to actually call C.Pkg_config.query to respect the user's pkg-config settings whenever they're available?

mmottl commented 6 years ago

AFAIR pg_config should always be available with a proper PostgreSQL installation and give you everything you need. Obviously, there is no need to construct the conf record. I don't remember, but maybe I was looking for a function to write out the Pkg_config information and then decided to just write out the sexp files myself. It doesn't really hurt. Let me know in case you run into any configuration issues with the current discover script.

rgrinberg commented 6 years ago

I see. I thought your intent was to use the pg_config settings as the default whenever pkg-config is absent or isn't aware of libpq being installed. I did look around and couldn't find any instances where this is possible. I.e. libpq would be installed without pg_config. Hence what you're doing is sensible.

But I still wonder if it's the best. In particular, cross compilation with pkg-config is easier because you can control the host's & target's lib db with a single environment variable. While relying on pg_config will require some special handling. Anyway, as I said, there's no real bug. Not unless I find a distro that ships libpq without pg_config :) Thanks for answering the question.

mmottl commented 6 years ago

For most use cases it seemed a little more reliable to me to use pg_config. Though it may be rare, some machines do not have pkg-config. Feel free to add more elaborate configuration options if needed.