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

Linking to native library multiple times through build dependencies #5237

Open
bsteinb opened this issue Mar 24, 2018 · 31 comments
Open

Linking to native library multiple times through build dependencies #5237

bsteinb opened this issue Mar 24, 2018 · 31 comments
Labels
A-build-dependencies Area: [build-dependencies] A-dependency-resolution Area: dependency resolution and the resolver A-links Area: `links` native library links setting S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@bsteinb
Copy link

bsteinb commented Mar 24, 2018

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
Copy link
Contributor

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
Copy link
Author

bsteinb commented Mar 24, 2018

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
Copy link
Member

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
Copy link
Author

bsteinb commented Mar 24, 2018

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
Copy link
Contributor

Eh2406 commented Mar 24, 2018

Ya this is going to be tricky to solve. It is related to #2589 but in this case it is a conflict between two different packages build dependencies. If we had a solution to rust-lang/rust#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
Copy link
Member

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
Copy link
Author

bsteinb commented Mar 24, 2018

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
Copy link
Contributor

Eh2406 commented Mar 24, 2018

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
Copy link
Author

bsteinb commented Mar 24, 2018

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
Copy link
Member

Thanks for the clarifications @bsteinb and @Eh2406!

@klausi

This comment has been minimized.

@alexcrichton

This comment has been minimized.

@klausi

This comment has been minimized.

@sfackler

This comment has been minimized.

@Eh2406

This comment has been minimized.

@bsteinb
Copy link
Author

bsteinb commented Apr 4, 2018

The issue I initially described here is not related to what @klausi describes and I do not think it can be solved by #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
Copy link
Contributor

klausi commented Apr 6, 2018

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
Copy link
Contributor

ExpHP commented Sep 4, 2018

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..

jsgf added a commit to jsgf/libffi-sys-rs that referenced this issue Oct 12, 2018
This is to avoid a clash over clang-sys. Workaround for cargo bug rust-lang/cargo#5237.
jsgf added a commit to jsgf/libffi-sys-rs that referenced this issue Oct 12, 2018
This avoids a cargo issue rust-lang/cargo#5237 which prevents different build scripts using different versions of libclang-sys.
@russel
Copy link

russel commented Dec 13, 2018

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
Copy link
Contributor

ExpHP commented Dec 14, 2018

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
Copy link
Contributor

jsgf commented Jan 17, 2019

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

@handicraftsman
Copy link

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

@boringcactus
Copy link

boringcactus commented Aug 1, 2019

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
Copy link

dylanede commented Sep 2, 2019

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).

@ehuss ehuss added A-build-dependencies Area: [build-dependencies] A-links Area: `links` native library links setting labels Jan 25, 2020
BusyJay added a commit to BusyJay/grpc-rs that referenced this issue Mar 31, 2020
Due to rust-lang/cargo#5237, upgrading bindgen is not backward
compatible.

Signed-off-by: Jay Lee <[email protected]>
BusyJay added a commit to BusyJay/grpc-rs that referenced this issue Mar 31, 2020
Due to rust-lang/cargo#5237, upgrading bindgen is not backward
compatible.

Signed-off-by: Jay Lee <[email protected]>
BusyJay added a commit to tikv/grpc-rs that referenced this issue Mar 31, 2020
Due to rust-lang/cargo#5237, upgrading bindgen is not backward
compatible.

Signed-off-by: Jay Lee <[email protected]>
@LukasBombach
Copy link

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
Copy link

ydirson commented Oct 3, 2023

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
Copy link
Contributor

Eh2406 commented Oct 3, 2023

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

@ydirson
Copy link

ydirson commented Oct 6, 2023

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
Copy link
Contributor

Eh2406 commented Oct 9, 2023

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 [email protected] to the selected set, what are all of the selected packages whose build script would end up linking in [email protected]". 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.

@epage epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Oct 18, 2023
370417 added a commit to 370417/rust-wooting-sdk that referenced this issue Jan 3, 2024
Bindgen versions need to be compatible because of a cargo limitation:
rust-lang/cargo#5237
@Xaeroxe
Copy link
Contributor

Xaeroxe commented May 14, 2024

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
Copy link
Contributor

Eh2406 commented May 15, 2024

"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".

@weihanglo weihanglo added the A-dependency-resolution Area: dependency resolution and the resolver label Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-dependencies Area: [build-dependencies] A-dependency-resolution Area: dependency resolution and the resolver A-links Area: `links` native library links setting S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests