Don't vendor OpenSSL except on Windows

[bossmc]

What did you implement:

This relates to #170 where shiplift switched to using vendored openssl to assist windows builds. Unfortunately I believe that change went too far the other way, using the system-wide OpenSSL is much more common on *nix systems, and the vendoring gets in the way of other tooling (e.g. building an RPM from such a binary leads to problems, as the RPM will try to "depend" on the vendored shared object, which will not be resolvable from the repo).

This change enables the vendored feature only on windows builds, and reverts to the default behaviour (searching for a system-wide version using pkg-config if present) on other systems (mac, linux...)

I considered an alternative option, that of exposing This PR exposes a vendored-ssl feature that enables the openssl/vendored feature, which can be enabled if desired by builds, this makes windows builds a little harder, as they probably have to enable the feature.

How did you verify your change:

(cross) Compiled for linux and windows.

What (if anything) would need to be called out in the CHANGELOG for the next release:

Clarifying the notes from #170 to be clear that they only apply to windows builds if you enable the vendored-ssl feature.

[richardwhiuk]

It seems like this should work, but it actually doesn't - see rust-lang/cargo#1197 (comment)

😞

[bossmc]

Sounds like adding a feature is a better fix then.

[softprops]

Just catching up on the state of this. I don't have a great way to test on windows at the moment. Are there any pending changes?

[bossmc]

@softprops - There's still some more work to do, the change I've made so far is being ignored by Cargo, so I'm putting together a smaller version of the change. Should be with you today.

[bossmc]

Ok, updated PR, new strategy - expose a vendored-ssl feature on shiplift that activates the openssl/vendored feature on openssl.

Windows:

c:\src\shiplift>cargo t --features vendored-ssl --no-default-features
...
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Linux:

$ cargo t
...
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
$ ldd target/debug/deps/shiplift-5d6a9fe8f830c081
...
	libssl.so.1.1 => /usr/lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007fccd9fa1000)
...
$ cargo t --features vendored-ssl
...
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
$ ldd target/debug/deps/shiplift-792b6a711bfb1b6c
	linux-vdso.so.1 (0x00007f7568604000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f7567e7e000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f7567c76000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f7567a57000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f756783f000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f756744e000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f75683de000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f75670b0000)

[bossmc]

This change prevents an "unreachable branch" warning on builds without unix-socket support (e.g. on windows)

[softprops]

thanks for your work on this @bossmc I'm going to try and cut a release this weekend

[thomaseizinger]

For what it is worth, at least the currently released version doesn't work on Windows because hyper-openssl only actually establishes an SSL connection if the scheme of the URI is https.

However, the DOCKER_HOST variable on Windows (at least with Docker Toolbox), is something like: tcp://192.168.1.100:2367. We had to patch around this behaviour to actually make it work.

The new Docker for Windows (using HyperV) is another whole different story because it doesn't set the DOCKER_HOST variable. I haven't yet figured out how to make that work without disabling TLS.

[thomaseizinger]

Don't mind me, I just saw that there was another PR merged today about this: https://github.com/softprops/shiplift/pull/193/files

[bossmc]

Cool, thanks @softprops!

[bossmc]

I've updated the notes for the CHANGELOG in the main PR description.