Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.2.16 causes build to fail on Debian/sid #174

Closed
sdroege opened this issue Mar 24, 2020 · 12 comments
Closed

0.2.16 causes build to fail on Debian/sid #174

sdroege opened this issue Mar 24, 2020 · 12 comments

Comments

@sdroege
Copy link

sdroege commented Mar 24, 2020

Forcing 0.2.15 as dependency makes it work again.

What happens is that at link-time, the following error (example: cargo-outdated) happens:

error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/slomo/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/tmp/cargo-installybkdsp/release/deps/cargo_outdated-1ea77f6eb6917f05.cargo_outdated.6ksrfsse-cgu.15.rcgu.o" "-o" "/tmp/cargo-installybkdsp/release/deps/cargo_outdated-1ea77f6eb6917f05" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-Wl,-O1" "-nodefaultlibs" "-L" "/tmp/cargo-installybkdsp/release/deps" "-L" "/usr/lib/x86_64-linux-gnu" "-L" "/tmp/cargo-installybkdsp/release/build/libnghttp2-sys-1df74163b8635c4a/out/i/lib" "-L" "/usr/lib/x86_64-linux-gnu" "-L" "/tmp/cargo-installybkdsp/release/build/backtrace-sys-21cc0c968ec9fc50/out" "-L" "/tmp/cargo-installybkdsp/release/build/libgit2-sys-9304318852370821/out/build" "-L" "/usr/lib/x86_64-linux-gnu" "-L" "/tmp/cargo-installybkdsp/release/build/libssh2-sys-f852cf8ccaa1fceb/out/build" "-L" "/home/slomo/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/tmp/rustcZ7rwZW/liblibgit2_sys-0fe20175db1a82f4.rlib" "/tmp/rustcZ7rwZW/liblibssh2_sys-cf317a9056f6cde6.rlib" "/tmp/rustcZ7rwZW/libbacktrace_sys-9be9c43e32499755.rlib" "-Wl,--start-group" "/tmp/rustcZ7rwZW/libbacktrace_sys-32c2dc6fbc292c9c.rlib" "-Wl,--end-group" "/home/slomo/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-0b278345638bce90.rlib" "-Wl,-Bdynamic" "-lz" "-lssl" "-lcrypto" "-lz" "-lcurl" "-lutil" "-ldl" "-lutil" "-ldl" "-lrt" "-lpthread" "-lgcc_s" "-lc" "-lm" "-lrt" "-lpthread" "-lutil" "-lutil"
  = note: /usr/bin/ld: /tmp/rustcZ7rwZW/liblibssh2_sys-cf317a9056f6cde6.rlib(libgcrypt.o): undefined reference to symbol 'gcry_sexp_nth_data@@GCRYPT_1.6'
          /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libgcrypt.so.20: error adding symbols: DSO missing from command line
          collect2: error: ld returned 1 exit status

Depending on the crate it's other symbols, but always something from libgcrypt.so.20. It seems like for some reason it got dropped from the linker commandline between 0.2.15 and 0.2.16.

@sdroege
Copy link
Author

sdroege commented Mar 24, 2020

The problem seems to be that libssh is built against libgcrypt instead of openssl, and the build.rs assumes openssl.

@chrysn
Copy link
Contributor

chrysn commented Mar 30, 2020

A RUSTFLAGS="-C link-arg=-lgcrypt" in the environment fixes this (at least for installing cargo-crev, which depends on ssh2-rs ahd shows the same symptom without).

@sdroege
Copy link
Author

sdroege commented Apr 2, 2020

Setting LIBSSH2_SYS_USE_PKG_CONFIG=1 also fixes it. Not sure why this isn't the default if found.

@juliangilbey
Copy link

I'm hitting the same problem. The compilation of the ssh2 library depends upon what crypto libraries are found on the system. If you do not have libgcrypt20-dev installed when you build the ssh2 rust package, all is fine, but it breaks if you do, as it builds with the gcrypt functionality enabled. So the current setup is dependent upon the libraries present in the build environment, which is obviously really bad! (And it makes things break.)

The configure script (which must be run somehow during compilation) has various options which might be useful here:

  --with-openssl          Use OpenSSL for crypto
  --with-libgcrypt        Use libgcrypt for crypto
  --with-wincng           Use Windows CNG for crypto
  --with-mbedtls          Use mbedTLS for crypto

and all of these can be given in the form --without-openssl and so on to explicitly not use them.

@juliangilbey
Copy link

A RUSTFLAGS="-C link-arg=-lgcrypt" in the environment fixes this (at least for installing cargo-crev, which depends on ssh2-rs ahd shows the same symptom without).

No, please don't do this! This assumes that the system has libgcrypt present, and it will break if it doesn't. Instead, this crate should explicitly state which crypto library to use and which ones not to use. (In my last comment, I realise that I don't yet understand how build.rs works. I'll get there!)

@infinity0
Copy link

Setting LIBSSH2_SYS_USE_PKG_CONFIG=1 also fixes it. Not sure why this isn't the default if found.

A lot of rust crates have these special-snowflake envvars that you have to switch on for things to work.

As a policy, for Debian rust crates we patch these envvars away, switching the behaviour to using pkg-config by default.

@juliangilbey the reason your build was failing was because you were presumably not setting LIBSSH2_SYS_USE_PKG_CONFIG=1 and you were using cargo build which uses your local unpatched crates in ~/.cargo. If you build the debcargo Debian package (e.g. with dpkg-buildpackage), this would use the patched Debian crates in /usr/share/cargo/registry and this should work just fine. So I'll close the Debian bug, but please feel free to reopen if my guess was wrong.

I do consider it a bug that these special-snowflake envvars exist in multiple crates with unpredictable names, but it's a question for the wider rust community to decide on, I don't have any control over that.

@juliangilbey
Copy link

Thanks @infinity0 - that's really helpful. It's not a great state of affairs, as you say.

@wez
Copy link
Collaborator

wez commented Apr 25, 2020

I finally got some time to sit and look at this issue and I'm a little confused!

The build.rs will ask pkg-config about libssh2 if LIBSSH2_SYS_USE_PKG_CONFIG is present in the environment and that seems to be working as a workaround for the folks that have run into this.

Otherwise it will set up a manual build; it doesn't use configure or cmake; it manually lists out the files and flags that are used to build libssh2. None of those include gcrypt, so there is nothing in this crate that should be directly pulling in anything to do with gcrypt.

The most likely change in 0.2.16 that might have changed the build behavior is this PR:
#170
which uses pkg-config to add linkage to zlib and openssl that was previously implicit. Note that the pkg-config crate implicitly emits linkage options to cargo when it locates a library but that it requires explicit emission for include paths.

So, the question on my mind is: where is the gcrypt dependency coming from? These compilation errors make it appear to be coming from libssh2 itself, which is surprising given the above.

My best guess here is that #170 causes the build to pick up the system libssh2 (if it is installed) due to the implicit library search path changes and that even though we think we're building the library we're linking in both the thing we built and the system one.

I don't have a system where this fails so I'd appreciate a bit of additional info from those that have it reproducing:

Do they all have the system libssh2 package installed? (dev as well as the shared object? or just the shared object?). And importantly: do any of them NOT have it installed? (that would help disprove my theory!). Does the system libssh2 link against gcrypt?

@juliangilbey
Copy link

Hi @wez, you may be on to something! In answer to your final question, on a Debian system:

  • If libssh2-1-dev is installed, then all of the following are present: the libssh2 shared library, the libssh2 header files, the libssh2.a archive, the libgcrypt shared library, the libgcrypt header files and the libgcrypt.a archive. In this case, the libssh2-sys Rust build fails (it ends up with libgcrypt functions in the compiled object), unless LIBSSH2_SYS_USE_PKG_CONFIG is set. Note that it is building the local libssh2 library and there is no gcrypt symbol in the object files in target/debug/build/libssh2-sys-ebd37513ad34322b/out/build/libssh2/src/. Yet the error message says that there is a gcrypt symbol in target/debug/deps/liblibssh2_sys-7a4d55b535cc8702.rlib(libgcrypt.o). Unfortunately this file seems to be deleted during the build process, so it is hard to examine it: it seems, though, to have perhaps been built using the system version. (This build was as part of a build which depends on libssh2.)

  • If libssh2-1-dev is not installed but libssh2-1 is, then the following are present: the libssh2 and libgcrypt shared libraries, but the libssh2 and libgcrypt header files and .a archives are not present. In this case, the libssh2-sys Rust build succeeds.

I hope this is helpful!

@wez
Copy link
Collaborator

wez commented Apr 26, 2020

OK, I set up a system where I could reproduce this:

  • Install libssh2-1-dev on an ubuntu system
  • cargo install cargo-update

Patching cargo-update to point at a local copy of ssh2-rs so that could play around with build.rs I found that the problem is definitely this bit of code:
https://github.com/alexcrichton/ssh2-rs/blob/master/libssh2-sys/build.rs#L127

which is triggering essentially this same problem as described here:
https://github.com/rust-lang/libz-sys/blob/master/build.rs#L20-L23

The fundamental problem here is that the libz-sys crate owns managing and exporting the requisite dependencies to use libz but there are some system configurations where it doesn't export all that it should. Attempting to use pkg-config here in a downstream crate is bad because we have no knowledge of what really should be done for libz and similarly for openssl.

For now, I'm going to revert #170 and publish a new package. I believe that rust-lang/rust#69552 likely requires some changes in the crates upstream from this one.

wez added a commit that referenced this issue Apr 26, 2020
This caused fairly widespread problems and it seems that the original
issue that led to this change (rust-lang/rust#69552)
should really be fixed by better defining the data exported from eg:
libz-sys rather than having downstream crates replicating the same logic
from inside that crate.

refs: #174
refs: #170
refs: #169
refs: rust-lang/rust#69552
@wez
Copy link
Collaborator

wez commented Apr 26, 2020

I published 0.2.17 and confirmed that I can now build cargo-update on my test system.

@juliangilbey
Copy link

Thanks @wez!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants