rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.75k stars 2.42k forks source link

Linking to native library multiple times through build dependencies #5237

Open bsteinb opened 6 years ago

bsteinb commented 6 years ago

Recently the mpi crate has stopped building because it transitively depends on different versions of the clang-sys crate which links to the native libclang. Here is the error message:

error: multiple packages link to native library `clang`, but a native library can be linked only once

package `clang-sys v0.22.0`
    ... which is depended on by `bindgen v0.33.2`
    ... which is depended on by `mpi-sys v0.1.1 (file:///Users/bsteinb/Documents/Programming/rust/rsmpi/mpi-sys)`
    ... which is depended on by `mpi v0.5.2 (file:///Users/bsteinb/Documents/Programming/rust/rsmpi)`
links to native library `clang`

package `clang-sys v0.21.2`
    ... which is depended on by `bindgen v0.31.3`
    ... which is depended on by `libffi-sys v0.6.0`
    ... which is depended on by `libffi v0.6.3`
    ... which is depended on by `mpi v0.5.2 (file:///Users/bsteinb/Documents/Programming/rust/rsmpi)`
also links to native library `clang`

I understand that this would be a problem if both versions of the clang-sys crate were to end up being linked to by the mpi crate, but they do not. bindgen (and transitively clang-sys) in both cases is a build dependency of mpi-sys and libffi-sys and is only linked to their respective build scripts and does not end up in the mpi crate. Should this really be an error, or is cargo being too cautious here?

lukaslueg commented 6 years ago

For the moment you can make america mpi great again by having mpi-sys depend on bindgen = "0.31.3", which is the exact same version that libffi-sys uses. As the dependencies converge, the conflict no longer occurs.

bsteinb commented 6 years ago

I know. I am doing that at the moment.

My question is whether the constellation where different dependencies of mpi have build dependencies on different versions of clang-sys should actually be considered to be a problem by cargo at all. The way I see it, mpi-sys builds an executable from its build.rs that contains clang-sys v0.33.2 and libffi-sys builds an executable from its build.rs that contains clang-sys v0.22.0. These are completely distinct executables, why is it a problem that they depend on different versions of clang-sys?

alexcrichton commented 6 years ago

cc @Eh2406

Thanks for the report! Unfortunately this may be an exceedingly tricky one to solve. The resolver doesn't understand host/target distinctions as well as linkage, so it can't necessarily conclude the that the linkage here may actually work out..

bsteinb commented 6 years ago

You are quite welcome. If we are honest, I am not being completely selfless here.

I know nothing about cargo internals, but I am not sure this is a host vs. target issue. Both build executables should be host executables, no? The way I see it, this check for linking the same native library should not be applied across build dependencies of different crates (such as mpi-sys and libffi-sys in my case).

Eh2406 commented 6 years ago

Ya this is going to be tricky to solve. It is related to https://github.com/rust-lang/cargo/issues/2589 but in this case it is a conflict between two different packages build dependencies. If we had a solution to https://github.com/rust-lang/rust/issues/44663 then we would just list all build deps as private dependencies. But that is going to be hard to implement as we don't have any support for path of visibility.

alexcrichton commented 6 years ago

Oh sorry @bsteinb I misunderstood! It's true that these are both host dependencies which means that I'm not sure Cargo has ever supported this (I thought this may have been an accidenal recent regression)

bsteinb commented 6 years ago

Well this is something that recently showed up for the mpi crate, but probably not due to a change in cargo. mpi-sys depends on bindgen 0.33 which as of 11 days ago resolves to bindgen 0.33.2 which depends on clang-sys 0.22. Meanwhile libffi-sys has its bindgen dependency pinned to bindgen 0.31.3 which still depends on clang-sys 0.21. As @lukaslueg suggested, I can work around this issue by pinning my dependencies to specific versions that work together.

Even if this was not and is not supported by cargo, I am trying to raise the question whether this behaviour might be too strict/pessimistic.

Eh2406 commented 6 years ago

I can confirm that this is not the new error we just added. So I don't think it is a regression. An ideal cargo would allow this, but I don't think it is going to be easy to fix. I think an implementation of RFC 1977: public & private dependencies would include infrastructure to make this an easy fix, but that is something of a tautology. That RFC is not implemented because no one has designed the infrastructure.

bsteinb commented 6 years ago

All right, for now I can live with the workaround of pinning my dependencies. When cargo is changed to loosen these restrictions, I can loosen the version specifications again.

alexcrichton commented 6 years ago

Thanks for the clarifications @bsteinb and @Eh2406!

klausi commented 6 years ago

Running into the same problem when building crates with their minimal version:

$ cargo build -Z minimal-versions
error: multiple packages link to native library `z`, but a native library can be linked only once

package `libz-sys v0.0.1`
    ... which is depended on by `libssh2-sys v0.2.4`
    ... which is depended on by `libgit2-sys v0.7.1 (file:///home/klausi/workspace/git2-rs/libgit2-sys)`
    ... which is depended on by `git2 v0.7.1 (file:///home/klausi/workspace/git2-rs)`
links to native library `z`

package `libz-sys v0.1.2`
    ... which is depended on by `libgit2-sys v0.7.1 (file:///home/klausi/workspace/git2-rs/libgit2-sys)`
    ... which is depended on by `git2 v0.7.1 (file:///home/klausi/workspace/git2-rs)`
also links to native library `z`

cargo should be able to solve this: it should select the higher version only of libz-sys in this case because both libgit2-sys and libssh2-sys allow it. Then libz-sys can be linked only once and all is well.

Can we modify cargo's resolver to take into account links = "z" in Cargo.toml files? If 2+ versions of the same package are selected and that package links to a native library then try to select only one version that does not violate the version range specified by all dependents.

alexcrichton commented 6 years ago

@klausi oh that's actually already implemented, but it requires changes to the index which are happening with newer Cargos but old entries in the index aren't retroactively changed with this information

klausi commented 6 years ago

Hm, I ran rm -rf ~/.cargo/registry and cargo downloaded the index again, but still having the same problem when building https://github.com/alexcrichton/git2-rs with cargo build -Z minimal-versions.

Maybe it does not work with the -Z minimal-versions switch?

sfackler commented 6 years ago

Deleting and redownloading the index isn't going to change what's inside of it.

Eh2406 commented 6 years ago

I made an issue to link all the discussions about this to. So here gose https://github.com/rust-lang/crates.io/issues/7310

bsteinb commented 6 years ago

The issue I initially described here is not related to what @klausi describes and I do not think it can be solved by rust-lang/cargo#4978.

In the case I described, mpi-sys and libffi-sys build-depend on different versions of bindgen (in @klausi's scenario, libgit2-sys and libssh2-sys are regular dependencies). There is no single version of clang-sys that works with these two different versions of bindgen that the resolver can converge on. However, I am still quite certain that this should not actually be an error, because these different versions of clang-sys will not end up in the same artefact, but rather in two different executables – the compiled build scripts of mpi-sys and libffi-sys respectively.

(Just to be clear, I am not saying that the issue @klausi reported is not valid, just that I think it is a different issue from this one.)

klausi commented 6 years ago

Sorry, yep I think I posted too early in this issue without understanding how the registry index works :) I think I have enough pointers now how to fix the index to resolve my problem, please carry on with the original problem + discussion.

ExpHP commented 6 years ago

Hmm, this is an unfortunate issue. I just ran into it with more recent bindgens that require libclang v0.23, while the latest release of mpi-sys uses bindgen v0.31 which in turn depends on libclang v0.21.

I've long thought that a crate at the top of the dependency tree could link in any arbitrary number of versions of bindgen (and this would be true for most other crates!); but in reality, the FFI ecosystem is split into groups of crates based on their libclang version. Crates using bindgen v0.37-39 are incompatible with crates using bindgen v0.33-36, and etc..

russel commented 5 years ago

So if a project P depends on two crates X-sys and Y-sys from crates.io where X-sys and Y-sys are both bindgen generated but use different versions of bindgen that depend on different clang-sys crates, then P is unbuildable using Cargo?

ExpHP commented 5 years ago

Tragically, yes, unless you can find older versions of X-sys or Y-sys that have compatible versions of bindgen.

One must almost wonder how this isn't an issue that pops up more frequently. I doubt the average person would ever think that updating their bindgen dependency could be a semver-breaking change... but it very well can be!

jsgf commented 5 years ago

Just ran into this again - I have a 3-way bindgen fight between libnfs-sys, libffi-sys and zstd-sys.

handicraftsman commented 5 years ago

Same problem, but: imgui-sdl2 -> sdl2-sys, ggez -> sdl2-sys

boringcactus commented 5 years ago

I am running into this issue with bindgen and I thought I could work around it with

[patch.crates-io]
bindgen = { git = "https://github.com/rust-lang/rust-bindgen", tag = "v0.50.0" }

(since v0.50.0 is the more recent of the two versions at play), but evidently that didn't actually work - the 0.50.0 dependency was replaced but the older dependency was not. Is there a way to work around this issue without going to each transitive dependency and asking them to update bindgen?

If build dependencies are only required while building the build script, then this definitely shouldn't be happening at all, since it's not like build scripts can depend on each other. If the resolver doesn't know about build vs. regular dependencies, that seems fixable; if anybody wants to point me towards where that would belong, I can take a stab at fixing it.

I went ahead and took a look at changing it myself, but i'd probably have to change Cargo's "all the dependencies that matter are one big DAG" entirely, and that's a little audacious to take on myself.

dylanede commented 5 years ago

I've also just run into this, with a private crate conflicting with lttng-ust-generate (bindgen versions 0.51.0 and 0.32.3 respectively).

LukasBombach commented 3 years ago

I am getting this when trying to use skia-safe and yoga, both bind to clang, but to different versions:

error: multiple packages link to native library `clang`, but a native library can be linked only once

package `clang-sys v0.22.0`
    ... which is depended on by `bindgen v0.37.0`
    ... which is depended on by `yoga v0.3.1`
    ... which is depended on by `react-dom-native v0.1.0 (/Users/luke/Projects/react-dom-native)`
links to native library `clang`

package `clang-sys v1.0.1`
    ... which is depended on by `bindgen v0.59.1`
    ... which is depended on by `skia-bindings v0.43.0`
    ... which is depended on by `skia-safe v0.43.0`
    ... which is depended on by `react-dom-native v0.1.0 (/Users/luke/Projects/react-dom-native)`
also links to native library `clang`

is there any solution to this, I mean other than trying out different version combinations in hopes of finding a working combination by chance?

ydirson commented 1 year ago

Same problem hit. Is there really any reason to insist on crate unification, when the different crate versions would really be used for building different binaries? @alexcrichton, any hope for getting this issue solved soon? This kind of problem sure is not going to silence the critics of the "build all deps in every workspace" approach anytime soon...

Eh2406 commented 1 year ago

If there were progress it would be listed here. It is still a hard problem.

ydirson commented 1 year ago

I don't see a description of the difficulty here, it would be great to see where this is heading.

From an outsider point of view it looks like each build.rs of each build-dep is linked, separately from other build-deps, against a single bindgen version, where a single clang-sys gets linked. I am surely not seeing the big picture, and likely miss something, as from this what I see looks like an over-eager unification of clang-sys, while cargo could just allow distinct build.rs to each link to a different clang-sys version.

Is it more than "just" finding the right criteria to get cargo not to mistake this as different external libs getting linked to a single binary? What am I missing?

Eh2406 commented 1 year ago

As you point out, once you have a complete dependency graph selected it is fairly easy to check before linking whether this particular call will lead to linking multiple crates with the same "links" field. Unfortunately, by the linking stage is far too late to determine if there is a different dependency graph that would allow the build to succeed. The much earlier stage that determines which complete dependency graph to select is called dependency resolution. We added functionality to ban a dependency graph that has two crates with the same "links" field. Most of the time this means that cargo automatically does the right thing, picking dependencies that will work, so that no error message needs to come to the user at all.

This is one of the cases where that functionality creates a false positive. To make the functionality more accurate dependency resolution would need to keep track of not only the set of packages that have been chosen with the set of requirements not yet fulfilled, but also a fully constructed graph data structure. So that we can answer queries about "if I add foo@1.0.0 to the selected set, what are all of the selected packages whose build script would end up linking in foo@1.0.0". This is expensive, but doable. The problem I have not yet found a solution to is to efficiently determine which prior selections could be changed in order to resolve a conflict that depends on one of these "is connected in the graph" relationships. Specifically an encoding that naturally composes with the other kinds of conflicts already encoded in dependency resolution.

Without a efficient encoding the dependency resolver ends up trying an exponential number of graphs in determining whether there is a solution that avoids the problem. Even if the encoding is efficient if it doesn't compose with other conflicts then situations like "three versions have links conflict and the other two are semver compatible with a prior selection" end up triggering an exponential number of graphs to try.

Xaeroxe commented 5 months ago

Is this resolved by using resolver version 2? If not, why?

https://doc.rust-lang.org/edition-guide/rust-2021/default-cargo-resolver.html

Eh2406 commented 5 months ago

"resolver version 2" is implemented as a "feature resolver" pass that runs after the "dependency resolution" where this error is coming from. My last comment explains why it is hard to allow within the "dependency resolution".