Including system library paths makes it difficult to overide library versions

[hugoduncan]

At present, Config::find returns system library paths. This can make it difficult to override library versions.

The motiviating example is using rust-openssl with ssh2-rs on OS X. On Mac, openssl requires a non-system version of openssl for TLS support, which can successfuly be provided by homebrew, for example. ssh2-rs depends on libz-rs, which pkg-config finds as -L /usr/lib. This library path gets added before the openssl library path, resulting in the system (older) openssl libraries being used, and the linker failing.

[alexcrichton]

Currently system library paths are required to allow the compiler to find static versions of libraries, but I can see where this would cause problems with multiple libs on the system...

[hugoduncan]

Could the compiler not add the system default paths, independently of pkg-config, at the end of the arguments list (so they have the lowest priority)?

[alexcrichton]

Perhaps, but the compiler has no knowledge of the default paths for a system (like the linker does)

[hugoduncan]

What would the process be, to move towards a solution for this?

[alexcrichton]

I can't really think of a great solution for this, so it would probably involve coming up with one first :)

[hugoduncan]

I'm not familiar with all the tooling rust uses to compile. Is there some place I can read up on the linkers used on different platforms? Could the compiler query the linker for the default paths?

[alexcrichton]

We generally just shell out to ld, although I'm not sure if we can query ld for search paths.

[julienw]

So in our project we ended up on exactly the same issue, and no easy solution. Of course the root cause is the awful setup of libssl on MacOS X but we need to find a way around this...

I still don't properly get why we need the system paths for the static libs to be found. Isn't it something that ld can find out by itself ?

[julienw]

If we really need it, here is a dumb idea: call pkg-config twice:

• once with PKG_CONFIG_ALLOW_SYSTEM_LIBS
• once without PKG_CONFIG_ALLOW_SYSTEM_LIBS

Diff the 2 outputs. Then we'd know which libs are the system libs.

[alexcrichton]

I still don't properly get why we need the system paths for the static libs to be found. Isn't it something that ld can find out by itself ?

The compiler needs to load the actual .a archive file to work with it much of the time, so we can't defer it to the linker.

Diff the 2 outputs. Then we'd know which libs are the system libs.

Perhaps!

[julienw]

I just re-read the manual page for pkg-config. Actually the list of paths that pkg-config considers as "system" seems to be hard-coded:

       PKG_CONFIG_ALLOW_SYSTEM_LIBS
              Don't strip -L/usr/lib or -L/lib out of libs.

So maybe it's good enough to just special-case these paths ?

[alexcrichton]

I'd be game!

[julienw]

So now, how do we special case them ? Do you think rustc could actually automatically always add them (and then we can remove them from pkg-config output)? Or would it be too fragile and we should instead add them as a new special build.rs output that cargo would understand ?

[alexcrichton]

It's pretty rare to pick up a staticlib from there, so if we're linking a dylib we just shouldn't print out those two -L paths probably

[julienw]

In that case the patch would be as simple as not adding the env variable PKG_CONFIG_ALLOW_SYSTEM_LIBS when calling pkg-config? I can't do it now sorry (afk for some time).

[lilith]

See sfackler/rust-openssl#447 (comment)

I think there's a way to make this specific to certain libraries (or, at least, I did for openssl):

cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libssl*
cargo:rustc-link-lib=ssl
cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libcrypto*
cargo:rustc-link-lib=crypto

My fork: https://github.com/imazen/rust-openssl

[alexcrichton]

@nathanaeljones I don't think that syntax is recognized by the compiler. It will attempt to interpret /usr/lib/x86_64-linux-gnu/libssl* as a literal directory, but it ignores errors. That means that -lssl is passed to the linker and it "just works" because the file is located in a system directory and didn't end up needing a -L in the first place.

[lilith]

What about #11 (comment) ? This is a pretty painful bug.

Since cargo [replace] is non-deterministically buggy (and after 3 hours, I still can't isolate any variables as to why), this is very challenging to work around. ideas?

[lilith]

For reference, this is how I'm using [replace]. Cargo pretends it doesn't exist.

https://github.com/imazen/imageflow/blob/24b3519c62241135a16c8a2414485a3bb224f72c/imageflow_server/Cargo.toml#L8-L9

[replace]
"pkg-config:0.3.8" = { git = "https://github.com/imazen/pkg-config-rs", branch="master"}
"pkg_config:0.3.8" = { git = "https://github.com/imazen/pkg-config-rs", branch="master"}

[lilith]

[replace] must only be in the root/"empty" crate of a workspace: rust-lang/cargo#3099

[alexcrichton]

@nathanaeljones hm the pkg-config and pkg_config shouldn't be necessary, but were both required?

[lilith]

No, they both were not. But due to the non-deterministic order of link paths in cargo, it is very hard to construct tests. Failures can range from 5% of builds to 95% of builds, so isolating variables is near-impossible.

(I'm not blaming pkg-config - there are larger questions)

Friction

This is by far the worst whack-a-mole situation I've experienced in the Rust ecosystem. Since September, I've averaged 16 hours per month in fighting recurrences on Travis and AppVeyor (along with various dev environments). I forked pkg-config, but any new crates I use could pull in a standard version, causing the issue to ... eventually, even weeks later ... reoccur. It's not easy to diagnose, even the (n)th time, because sometimes it is just OpenSSL breaking its ABI again, or an actual build environment issue, and it's just infrequent enough to be extremely painful.

I should be shipping a production beta of Imageflow this month, but for now I've had to disable TLS support across all crates and purge usage of HTTPS. For the server component, I'm planning an nginx proxy to substitute, but I don't yet have an answer for making HTTPS requests, and that's kind a key feature for an image proxy. Right now I'm holding my breath and hoping that this PR goes somewhere fast. If it doesn't, I'll need to fork Hyper and most of the Iron ecosystem rather quickly.

Unfortunately, this isn't limited to OpenSSL - I've run into it with libpng, libjpeg, and various other packages.

Larger question

My understanding is that there are several issues at play:

1. Non-determinism in link path order. In isolation, non-determinism is a serious problem. If this is agreed, perhaps we should open a bug on cargo or rustc? I realize that caching and parallelism complicate this, and it's probably hard to solve. Nonetheless, .... non-determinism. This makes debugging improbable when linking to a large number of libs.

2. Conflicting link paths from different crates. It would be nice if duplicate archives in user link paths caused a build failure (I think). If an archive exists in two user-specified locations, I can't easily reason about which one will be selected. I could see this behavior as modal - perhaps --allow-link-candidates=any, --allow-link-candidates=one, --allow-link-candidates=one-unique-hash? Or this could be per-crate, which would let us safe-list candidate sources more cleanly. The links metadata in cargo tries to prevent this, but can't help if a directory contains more than the specified library. But there's a more generic way to do this - see end.

3. Reliance on system-wide libraries. I realize this might still be a topic of contention, but devs have put a lot of effort into gaining control over their dependencies and lowering the costs of this control. There's a servicing trade-off here, and likely some dependencies should be linked dynamically and serviced by the system update mechanism. Docker makes this less of a problem on Linux. However, *I think it might be worthwhile to consider supporting alternate artifact sources for -sys crates

Hear me out. You've got a great solution to the OpenSSL nightmare in the works, but the problem with C dependencies (in general) persists. Cross-plat is much harder when wrangling compatibility with a range of versions for each dependency. With diamond dependencies (like openssl) this is sometimes impossible without forking the world.

What if we made it easier for developers to use specific versions of C/C++ dependencies?

I realize this feature (discovered as I write this) tries to make that easier, and I suspect I could .gitignore .cargo/config and generate it from Conan.

However, if I miss a target, or a library, I risk silently linking against the wrong binaries and risking future heap or stack corruption.

What if - rather than preventing various build.rs scripts from executing - I was able to specify a 'filter' - an executable receiving the concatenated build script output via STDIN (along with all env vars like the target triple), and producing modified results to STDOUT?

This method would permit:

• Linting and reporting of link paths
• Testing of link-order determinism (for potential improvements).
• Detection of duplicate link candidates
• On-demand production of artifacts (and their link arguments) for only the required target triples.
• Code reuse - just a build-dependency. We could then share providers for various artifact sources. I'd probably provide one for Conan.io and all major *-sys crates.

Anticipated challenges:

• Standardizing conventions on how *-sys crates communicate acceptable ABI versions to filters.
• Optimizing away redundant builds. For *-sys packages that actually build from source, we don't want to do this twice. Perhaps we also invoke the filter with a command-line argument for each crate - before running build.rs, and let the filter choose whether to run or skip build.rs? I'd probably write out specific use cases before adding complexity, though.

[julienw]

@nathanaeljones if you already forked to remove https://github.com/alexcrichton/pkg-config-rs/blob/e9297b2e16e40026ec546324b657b780d2c305b3/src/lib.rs#L363 in your fork, maybe you can propose a PR ?

[alexcrichton]

@nathanaeljones sorry you've been having so much trouble lately! It sounds like there's definitely some improvement we could make somewhere.

That being said though this perhaps isn't the place to discuss these issues? I'm more than willing to help out with any particular issue, but general changes to Cargo should probably be discussed on internals.rust-lang.org, rather than the pkg-config-rs issue tracker.

[lilith]

@julienw #35 Would have done so earlier, but I didn't get confirmation that the change was acceptable.

@alexcrichton I wasn't sure, but figured you could suggest the best forum. Thanks! I'll post there.

[lilith]

I posted here: https://internals.rust-lang.org/t/cargo-support-for-build-script-filters-lints-and-link-path-providers/4563

[lilith]

In light of the refactoring of SSL in Hyper, I upgraded all crates and gave this another try. It would appear that this is still broken by default - at least, compiling openssl-sys on Ubuntu 14.04 brings in the world (cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu), and overrides my other dependencies.

[lilith]

What if we could somehow pass paths to the compiler only? Is there anything we can do to make this correct?

[Fuuzetsu]

We have lost bunch of developer time due to this surprising default 7 years since the ticket was opened...

At least we were able to locate the library leaking in the system directory by grepping for it in target and subsequently ensure it's not invoking pkg-config (at all in this case).

If there's nothing that's going to happen here, close as WONTFIX?