Specify full curl dependencies

[rainbreak]

Let curl know what its dependencies are. I'm not sure ssh2 is strictly necessary - might vary depending on curl install.

[rainbreak]

The ubuntu build fails with

-- ssh2 headers: SSH2_INCLUDE_DIR-NOTFOUND
-- ssh2 lib   : SSH2_LIBRARY-NOTFOUND
CMake Error at /var/jenkins_home/sharedspace/webthree-helpers/label/master/webthree-helpers/cmake/UseSSH2.cmake:9 (message):
  ssh2 library not found

Which suggests to me that libssh2 maybe isn't actually a dependency (i.e. we don't necessarily need curl compiled with it).

[bobsummerwill]

Yeah - please chop those three lines out entirely, and then we can commit that safely.

Then you could make a new PR with just those three lines, and we can iterate that to check-in-ability.

[bobsummerwill]

See https://curl.haxx.se/docs/libs.html.

So for your static-build scenario, were you getting link errors without ssh2, openssl and zlib? Or were they just header dependencies?

[rainbreak]

I already split them - see #160 for just the added files. Just have both commits here because the second is dependent on the first.

[rainbreak]

R.e. dependencies. I was getting issues at link time - e.g.

[100%] Linking CXX executable eth
/usr/lib/gcc/x86_64-alpine-linux-musl/5.3.0/../../../../lib/libcurl.a(libcurl_la-easy.o): In function `global_init':
easy.c:(.text+0x71): undefined reference to `libssh2_init'
/usr/lib/gcc/x86_64-alpine-linux-musl/5.3.0/../../../../lib/libcurl.a(libcurl_la-easy.o): In function `curl_global_cleanup':
easy.c:(.text+0x216): undefined reference to `libssh2_exit'
/usr/lib/gcc/x86_64-alpine-linux-musl/5.3.0/../../../../lib/libcurl.a(libcurl_la-content_encoding.o): In function `inflate_stream':
content_encoding.c:(.text+0x13f): undefined reference to `inflateEnd'
content_encoding.c:(.text+0x18d): undefined reference to `inflate'

https://travis-ci.org/rainbeam/eth-static/builds/125800060#L1283

I'm still not sure about the problem there though, just that static linking ssh2 fixed it. Maybe something to do with -DCURL_STATICLIB.

[bobsummerwill]

Right - my bad. Merged the other one now. Could you re-pull and merge into here now, so we're left with ONLY those final three lines? Or just make a new PR for those? They definitely need this guard around them, at least:

if (NOT MSVC)
    eth_use(${TARGET} ${REQUIRED} SSH2)
    eth_use(${TARGET} ${REQUIRED} OpenSSL)
    eth_use(${TARGET} ${REQUIRED} ZLIB)
endif()

Any comment on whether they are header-only or linkage too, based on your use-case? EDIT - Just saw above, ignore me!

Also, now we have moved forward to this point, we'll need eth_add_executable too, and/or the other conditional bits of that. That could be its own PR, I think, right?

[bobsummerwill]

So, with regard to these symbols, I am assuming that you were building against a prebuilt binary of curl, right? In which case we probably can't avoid these, and just have to suck up having the additional indirect dependencies. Even if curl can be built in some "mode" which omits these dependencies (if we don't need to use that part of curl), that doesn't even matter if we don't control the building of those curl binaries. We get what we get and have to suck it up.

easy.c:(.text+0x71): undefined reference to `libssh2_init'
easy.c:(.text+0x216): undefined reference to `libssh2_exit'
content_encoding.c:(.text+0x13f): undefined reference to `inflateEnd'
content_encoding.c:(.text+0x18d): undefined reference to `inflate'

So maybe it's worth a few minutes of your team digging into how curl deals with optional dependencies, and if there is some #define CURL_NO_SSH or whatever which we should have within our pre-processor settings? But probably not worth more than a few minutes.

What was the deal with OpenSSL? Link errors for that one too?

[bobsummerwill]

Some hints here: https://github.com/Homebrew/legacy-homebrew/blob/master/Library/Formula/curl.rb

[bobsummerwill]

https://curl.haxx.se/docs/install.html

The configure script always tries to find a working SSL library unless
explicitly told not to. If you have OpenSSL installed in the default search
path for your compiler/linker, you don't need to do anything special. If
you have OpenSSL installed in /usr/local/ssl, you can run configure like:

    ./configure --with-ssl

If you have OpenSSL installed somewhere else (for example, /opt/OpenSSL)
and you have pkg-config installed, set the pkg-config path first, like this:

    env PKG_CONFIG_PATH=/opt/OpenSSL/lib/pkgconfig ./configure --with-ssl

Without pkg-config installed, use this:

    ./configure --with-ssl=/opt/OpenSSL

If you insist on forcing a build without SSL support, even though you may
have OpenSSL installed in your system, you can run configure like this:

    ./configure --without-ssl

To get support for SCP and SFTP, build with --with-libssh2 and have libssh2 0.16 or later installed.

[bobsummerwill]

What we certainly COULD do as a starting point here is to make these three dependencies optional.

Check out the CMake patterns for the optional dependencies for CpuId, libminiupnp.

If we did that then nothing would break on our existing platforms, but your use-case here would start working. That might be the best which we can do anyway, if the indirect dependencies for curl vary a lot between DLL, static and cross-build scenarios anyway.

[rainbreak]

Ok, I'll put that guard in.

Will do eth_add_exe elsewhere (EDIT: #163)

w.r.t. curl - yeah that's right, using the system library. I don't think there is a define that will let us ignore ssh2 if it is already configured with it there. Not documented anyway.

[bobsummerwill]

Given that our Jenkins builds for Windows, OS X and Linux are all failing, I think we should just go with them being optional on all platforms. Because they obviously aren't necessary at least in those desktop environments.

That lets us screw off looking at curl build modes too, eh?

[rainbreak]

Yep, good plan. I've had enough curl internals for one week. Will make optional.

[bobsummerwill]

"I've had enough curl internals for one week"

^ LOL.

[rainbreak]

woo it works. I think setting the deps as optional is the neatest solution anyway. Main thing for static linking purposes was making sure that cmake knew about the libraries so it constructed the correct link arguments.

[bobsummerwill]

It looks like we're within a couple of commits of getting all this functionality in, eh? Thanks for your work on this. Will be a great feature to have.

When we've done that I can try to build our existing Linux, OS X and Windows binaries in that mode too, which will tell us if further edits are needed within those OpenSSL, ssh2 and zlib CMake files to do something cleverer than just making them optional - or perhaps it is just the case that static build require more explicit package installs? We'll see.

[rainbreak]

I've got a feeling we might need to specify all possible curl dependencies here, rather than the limited subset that Alpine uses. For example, Debian curl is linked to all of the optional libraries.

I'm trying to get Debian going now (see problems in #163) so should get an idea.

What would be good would be if cmake could magically work out all the curl dependencies. It has a pkg-config wrapper, but that isn't cross platform.

[bobsummerwill]

"I've got a feeling we might need to specify all possible curl dependencies here, rather than the limited subset that Alpine uses. For example, Debian curl is linked to all of the optional libraries."

Yes, we would need to do that with the current setup, given that we are just relying on some external and completely ill-defined external curl binaries. We don't control the version. We don't control the modes, or the compiler settings or anything.

This is why I want us to add a number of our external dependencies as sub-modules at some stage soon, even though that means they will then be built from source. But it means that we totally side-step all that platform-specific packaging stuff, which is a pain on OS X with Homebrew, utterly undefined on Windows and nightmarishly fragmented in Linux.

[bobsummerwill]

Thanks - merged this!

[bobsummerwill]

FYI ... getting a knock-on issue from this change in a normal cmake .. for webthree-umbrella on OS X.

-- Could NOT find CURL (missing:  CURL_LIBRARY) 
-- curl headers: /usr/include
 -- curl lib   : CURL_LIBRARY-NOTFOUND
CMake Error at webthree-helpers/cmake/UseCURL.cmake:13 (message):
  Curl library not found

Seemingly because the UseJsonRpc is REQUIRED, but the (now chained) dependency onto curl still needs to be optional, but I guess the change from find_package() has screwed with the semantics a bit.

Switching the eth_use from ${REQUIRED} to OPTIONAL doesn't fix it either. Needs some more love.

I will try, but if you know the magic fix, please do feel free to try to fix too :-)

[bobsummerwill]

So in this case, we don't NEED curl, but the new CMake is requiring that (where the chained curl indirect dependencies are optional, but curl itself seemingly is not).