cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Resolver is too strict in its multiple-crates-`links`-ing check

Open kamalmarhubi opened this issue 7 years ago • 13 comments

I'm attempting to use a feature to allow controlling which version of a -sys crate is used. It looks like I'm running afoul of the restriction on multiple crates having a links specified for the same library, even though in any particular invocation only one will be used.

Does the resolver ignore features when deciding if a library is being linked multiple times? It seems that it is, and I think that it should only check the final resolved dependency tree for the target being built, and in particular after the features are applied.

kamalmarhubi avatar Sep 03 '18 20:09 kamalmarhubi

cc @Eh2406, would you be able to help out with this one?

alexcrichton avatar Sep 04 '18 16:09 alexcrichton

@kamalmarhubi that is an important issue, but I am not quite seeing how you were proposing to use features to solve it. What prevents someone from specifying that they need both features? Can you point me at what you have tried so far?

Eh2406 avatar Sep 04 '18 17:09 Eh2406

Note that https://github.com/rust-lang/cargo/issues/6179#issuecomment-431544643 wuld provide a workaround by using --minimal-cargo-lock to test only one of the features at a time. (you would need to run the tests twice once with each feature)

Eh2406 avatar Oct 22 '18 18:10 Eh2406

I know that features are additive, and I would like to use it as advantage: emit an error if and only if the final union of all enabled features ends up pulling two crates with conflicting links attributes. If feature settings actually happen to pull only one crate, then don't err.

Regarding lockfile, maybe build the lockfile while completely ignoring existence of links (allowing the lockfile to contain links-conflicting crates), and check links conflicts later as a pass only on the subset of crates that have been selected by the features. In other words, it should be build-time check, not update-time check.

kornelski avatar Oct 29 '18 18:10 kornelski

Features being additive means that if it work with feature A and it works with feature B then it must work with A and B. I know that is not how features are used in practise, but that is the axiom on which they are designed.

Checking links conflicts later unfortunately, fails with an even worse ux problem. If the resolver is unaware of the links attribute, then it easily can decide that 2 of your dependencies 10 or 100 layers deep should use different versions of some sys crate you have never heard of. What ux is there to fix it? You can do the NP-Hard problem of resolving dependencies in your head, figure out which decision (of the thousands) the resolver made incorrectly. If you are lucky pin to the correct version in your Cargo.toml with a blabla = "=1.2.24" # only here to get the correct version of foo-sys. If you are not lucky, then the only thing you can do is write the cargo.lock by hand. What is worse is that this complete breakdown of ux, has everyone involved using everything correctly. The only party at fault is the cargo for making a lockfile that it was then going to refuse to bild.

Eh2406 avatar Oct 29 '18 18:10 Eh2406

The example you've linked to contains two semver-major-incompatible sqlite dependencies (0.x acts as major), so I think it's working as intended, and there is simply no solution for that dependency graph. I don't see how using links in resolution could change anything about it.

The UX of dealing with conflicts is indeed poor. However, I think not looking at unused crates would make conflicts rarer, so it should actually improve the situation.

kornelski avatar Oct 29 '18 21:10 kornelski

">=0.8.0, <0.10.0" means that it will take one of several different semver-major-incompatible versions of sqlite. The resolution where both take v0.9.x is the only one that will build. If the resolver doesn't know about links then it will take the opportunity to give desle the newest v0.10.x. (the test that demonstrates the difference in resolution)

We do ignore the unused optional crates in your dependencies. see my earlier reply. I believe this provides a way to work around this limitation in the functionality of features. I suspect that the number of times there are links related conflicts in optional dependencies in the crate being developed is small.

Eh2406 avatar Oct 29 '18 22:10 Eh2406

In previous reply you've mentioned something about tests, but I don't see how that's related. The problem is reproducible with an empty project and just two dependencies (they are not dev deps, they're supposed to be usable in the final product).

kornelski avatar Oct 29 '18 23:10 kornelski

I am sorry for not explaining myself clearly. I am also sorry for coming across as rude and dismissive. You deserve a clear, explained, and reproducible response. Give me a little while to make sure I have my facts straight; I'd hate to spend the time to have a high quality right up that is completely wrong.

Eh2406 avatar Oct 29 '18 23:10 Eh2406

Attached is a zip file, containing the folder structure I used to check that I was correct. issues6231.zip

The setup For reference here is the tree of that folder:

+---hard
|   |   Cargo.toml
|   \---src
|           lib.rs
+---tester1
|   |   Cargo.toml
|   \---src
|           main.rs
\---tester2
    |   Cargo.toml
    \---src
            main.rs

The library hard is very similar to your excellent example here

[package]
name = "hard"
version = "0.1.0"

[features]
default = ["libz-sys"]

[dependencies]
libz-sys = { version = "1.0.25", optional = true }
cloudflare-zlib-sys = { version = "0.1", optional = true }

Just for testing, I added some code that depended on which features are on:

#[cfg(feature = "libz-sys")]
pub fn thing() -> &'static str {
    "libz"
}

#[cfg(feature = "cloudflare-zlib-sys")]
pub fn thing() -> &'static str {
    "cloudflare-zlib"
}

The problem There is no way to build "hard". A cargo build in the hard folder will give an error.

The ~~solution~~ workaround Add an new project that depends on hard and thereby test if hard can be used.

// tester1/Cargo.toml
[package]
name = "tester1"
version = "0.1.0"

[dependencies]
hard = {path="../hard", version="*", default-features = false, features = ["cloudflare-zlib-sys"]}

It can be built just fine and when it calls hard::thing() it gets "cloudflare-zlib". To test the other configuration,

// tester2/Cargo.toml
[package]
name = "tester1"
version = "0.1.0"

[dependencies]
hard = {path="../hard", version="*"}

It can be built just fine and when it calls hard::thing() it gets "libz".

So there is a way to make a shim library that can switch between two crates that have the same links attribute. It is painful to develop in, but it works.

Eh2406 avatar Oct 30 '18 00:10 Eh2406

Thank you very much for your explanation and the example files.

The result is very surprising to me, since I can build tester1 with hard, tester2 with hard, but not hard itself.

kornelski avatar Oct 30 '18 02:10 kornelski

It is not intuitive. I would go so far as to say if we had to remake features from scratch this would not be the behaviour we would have designed. There are any number of "maybe someday we could" idys that may make this better without regressing the usability. Few of them are close to being designed let alone implemented. At least there is a workaround.

Eh2406 avatar Oct 30 '18 02:10 Eh2406

Attached is a zip file, containing the folder structure I used to check that I was correct. issues6231.zip

I just experiemented with this example. If you are using workspaces and both library configurations are "reachable" within that workspace (choosing once libz-sys and once the cloudflare one), then this does not work. I think the reason for this is that two Cargo.lock files are created in if you are not usage workspaces. In the workspace a single Cargo.lock is used.

maxammann avatar Aug 19 '22 11:08 maxammann

It still conflicts even in the [target.'cfg(foo)'.dependencies] vs [target.'cfg(not(foo))'.dependencies], which is pretty frustrating.

In my case, I have a conflict in switching between web-view and wry with a feature on windows (where the former doesn't have a dependency, but the latter can be debugged), but there is a links conflict for the library both use on linux :|

simonbuchan avatar Nov 14 '22 07:11 simonbuchan

I have a proposal for global, mutually exclusive features (https://internals.rust-lang.org/t/pre-rfc-mutually-excusive-global-features/19618) and hadn't considered this case! I wonder if there is a way we could tell cargo with that feature that this is safe.

epage avatar Oct 23 '23 21:10 epage