Use the openssl-sys package to find or compile openssl

[svanharmelen]

I have a use case where I need to cross-compile an app that also depends on the native-tls crate. This one depends (eventually) on the openssl-sys crate which I use with the feature vendored. This feature compiles and builds OpenSSL libs which can be statically linked.

Now I tried to make the paho-mqtt-sys crate use those same libs, but as the path to them is generated dynamically during the build, that is currently not possible. So in this case the only option is to first cross-compile OpenSSL myself and then point to those libs. But that makes it a bit cumbersome if other people checkout the project and want to work on it (they would all have to do the same thing).

A possible solution for that could be to make the paho-mqtt-sys crate depend on the openssl-sys crate, as then we can use the links metadata to get the location and reuse the same libs.

This also improves (IMHO) the usability when not cross-compiling or when you don't want to statically link the OpenSSL libs. In those cases it tries to discover the libs on your system and use those instead. Additionally it offers a lot of options to tweak things to your liking making easy for people to build the crate in a way that suits their project.

For the implementation in this PR I chose to add a new feature called vendored to the paho.mqtt.rust crate that re-exports the paho-mqqt-sys/vendored feature which in turn re-exports the openssl-sys/vendored feature. While this works very nicely (tested quite a bit both with and without the vendored feature and with and without cross-compiling), I think it would maybe be nicer to not make a new feature, but instead let it be added to the existing bundled feature.

In cases were people do want to use the bundled feature, but do not want to use the openssl-sys/vendored feature for some reason, this can still be managed and configured using env vars supported by the openssl-sys crate (by setting OPENSSL_NO_VENDOR). So just let me know if you want me to update the PR to use the bundled feature instead...

[fpagliughi]

Yes! This is a great idea. I never even thought of it.

I need a day or two to find time to test, but on a quick look, I'm thinking:

• The terms "bundled" and "vendored" tend to mean the same things on different projects, and I had been wondering if I should switch terms in the project because "vendored" seems to be coming more popular/standard (I think). Anyway, it's a bit generic to refer to just the SSL library. Perhaps we should use something like "vendored-ssl"?

• I think the case of using "bundled" without vendored-ssl might be more common that you think - as in the case of embedded Linux where a single SSL lib is shared across all apps. So I'd prefer to see if we can control this through features rather than an external environment variable.

• Could you update the README.md file to describe all this?

[svanharmelen]

Thanks for the feedback, I'll have another go at it tomorrow.

The terms "bundled" and "vendored" tend to mean the same things on different projects, and I had been wondering if I should switch terms in the project because "vendored" seems to be coming more popular/standard (I think).

Agree, I think that would make sense.

Anyway, it's a bit generic to refer to just the SSL library. Perhaps we should use something like "vendored-ssl"?

Sounds good to me.

I think the case of using "bundled" without vendored-ssl might be more common that you think - as in the case of embedded Linux where a single SSL lib is shared across all apps. So I'd prefer to see if we can control this through features rather than an external environment variable.

Check. So then we just keep it like it's done in the current PR, as a dedicated feature 👍

Could you update the README.md file to describe all this?

Yep, will do...

[svanharmelen]

@fpagliughi I just updated the PR with the feedback you gave yesterday. I also updated (renamed) the bundled feature to vendored. If you do not want that at this point, then I can of course reverse that part again.

Additionally I also made the vendored-ssl feature enable the ssl and vendored features, as otherwise it would not give the expected result (right?). But again, I can revert that if you want to...

[fpagliughi]

I think, for now, I would prefer to keep the bundled feature name. I'll do some research to see how other projects are using the terms lately.

[svanharmelen]

Check. I’ll revert that part 👍 Are you good with the rest of the PR or do you have any other feedback I need to take into account?

[svanharmelen]

@fpagliughi reverted the bundled -> vendored change. Let me know if you have any additional feedback...

[svanharmelen]

@fpagliughi any other things that you want to have updated/changed? Or is this one good to go?

[fpagliughi]

Sorry, just been slammed with work. I'll have a look within the next few days.

[svanharmelen]

Ping 😏

[svanharmelen]

Hi @fpagliughi, how are things going? Any change to get this one merged?

[KoWu]

reqwest uses the native-tls-vendored feature for this purpose btw. And I am excited to see this feature in the 0.8 release, will simplify or build process by a lot :)

[fpagliughi]

Merged manually (to fix conflicts) into the develop branch.

[svanharmelen]

Nice 👍🏻