sgrif / pq-sys

Auto-generated rust bindings for libpq
Apache License 2.0
41 stars 32 forks source link

Add feature for including openssl-sys #29

Closed Raniz85 closed 10 months ago

Raniz85 commented 4 years ago

Added a new feature (openssl-static) that includes the openssl-sys crate to fix static builds.

Renamed src/lib.rs to src/bindgen.rs and added a new lib.rs that re-exports everything from bindgen.rs to prevent future bindgen runs from reverting this change.

This fixes #25

golddranks commented 4 years ago

I think #25 was missing a reliable reproduction; here is one: https://github.com/golddranks/pq_link_test

I built it against your fork of pq-sys, and indeed, adding the feature openssl-static fixes the build.

Raniz85 commented 3 years ago

Any chance of merging this?

weiznich commented 3 years ago

@Raniz85 I got recently the rights to merge PR's in here, but I want to do a check of the current status of the crate first before merging anything. Unfortunately I haven't found the necessary time yet.

weiznich commented 3 years ago

While thinking a bit more about this PR: What's the expected use case for this? You mention fixing static builds, but this change on your own won't help you there much as you need to enable the corresponding feature anyway. Most people will likely use pq-sys through diesel that means that with this change you need to add pq-sys to your dependencies + enable the corresponding. feature. Without this feature you would need to add openssl-sys to your dependencies and be done. I must say that I don't really get what's then the advantage of having this feature here.

Raniz85 commented 3 years ago

Without this feature you would need to add openssl-sys to your dependencies and be done.

It's been more than a year since I worked on this so my memory is hazy, but if I remember correctly, just adding a dependency on openssl-sys isn't enough, the dependency (or rather the extern crate openssl_sys) must come from pq-sys so that we can guarantee that openssl-sys is linked before pq-sys.

If the order is reversed, compilation will fail.

Read the linked issue and the issues referenced there, they contain quite a lot of information and debugging.

golddranks commented 3 years ago

I can confirm that there were certainly problems with the linking ordrer. What made them hard to deal with was that they were somewhat non-deterministic.

Raniz85 commented 2 years ago

@weiznich Any progress on this? Either reject it or merge it please

weiznich commented 2 years ago

@Raniz85 I had no time to look into this yet, so there is no progress here. To be clear even to decide if this should be rejected or not I need time to understand the underlying problem. Unfortunately I cannot say when I will find the required time to look into this.

weiznich commented 10 months ago

Closed in favor of #46