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

[Question] fail to build serde_json dylib #3

Closed
huahouye opened this issue Sep 13, 2022 · 7 comments
Closed

[Question] fail to build serde_json dylib #3

huahouye opened this issue Sep 13, 2022 · 7 comments

Comments

@huahouye
Copy link

Thanks for this cool tool!

I tried to build serde_json dylib with serde dylib, but failed. If I change crate-type to rlib for the serde_json-dynamic sub-package, then it works. simple project is here.

Not sure whether rlib works like dylib here or not as expected?

@rksm
Copy link
Owner

rksm commented Sep 14, 2022

The problem seems essentially to be the diamond dependency issue described here: rust-lang/rust#34909 (comment)

  graph TD;
      bin["rust-dylib-serde-issue"]-.->serde_dylib["serde (dylib)"];
      bin-.->serde_json_dylib["serde_json (dylib)"];
      serde_json_dylib-->serde_json;
      serde_json-->serde1["serde"];
      serde_dylib-->serde2["serde"];
Loading

The problem is that the serde (dylib) dependency provides a copy of serde to the binary and serde_json pulls in another copy of serde as well. Even though the versions are the same, Rust cannot deal with this because of global state mentioned in the linked issue above. This then results in the error error: cannot satisfy dependencies so serde only shows up once.

The solution would be to make serde_json depend on serde dynamically as well so that the same dylib can be linked to serde-dynamic and serde_json.

  graph TD;
      bin["rust-dylib-serde-issue"]-.->serde_dylib["serde (dylib)"];
      bin-.->serde_json_dylib["serde_json (dylib)"];
      serde_json_dylib-->serde_json;
      serde_json-.->serde_dylib;
Loading

The only way I was able to make this happen was by cloning https://github.com/serde-rs/json and modifying it's Cargo.toml to use the provided serde-dylib crate. And then have serde_json_dylib use that instead of the upstream serde_json (or change it 's crate type to use dylib and use it directly). Now this is certainly undesirable.

Unfortunately, using a Cargo.toml override such as

[patch.crates-io]
serde = { version = "0.1.0", path = "../serde-dynamic", package = "serde-dynamic" }

in serde_json-dynamic did not fix the issue. Using RUSTFLAGS="-C prefer-dynamic" didn't either.

So generally this would work but if a dependency (such as serde) specifies to be static (the default) and it appears multiple times in the dependency graph the resolution mechanism of Rust cannot deal with the situation.

This seems a shortcoming of how dylib builds can be specified: The crate that is to be consumed needs to do it via crate-type. Dependent crates cannot override this. IMHO this is something that could be added to Rust/Cargo as it seems to be a backwards compatible addition. Though maybe there are dragons somewhere.

@rksm
Copy link
Owner

rksm commented Sep 14, 2022

I gonna update the blog post with this as this is likely something that will come up repeatedly. Thanks for bringing this up!

@rksm
Copy link
Owner

rksm commented Sep 14, 2022

Described the problem here: https://robert.kra.hn/posts/2022-09-09-speeding-up-incremental-rust-compilation-with-dylibs/#limitation-the-diamond-dependency-problem.

Maybe a PR against serde to have it include dylib in its crate-type would be an option. This way consumers could opt-in to dylibs with -C prefer-dynamic... though I do not know if this would have any side effects.

@huahouye
Copy link
Author

Thank you for your response and detail explanation. It is much clearer to me now. l'll keep an eye on this issue and this one serde-rs/serde#2278

@rksm
Copy link
Owner

rksm commented Sep 17, 2022

I don't think this will really go anywhere with serde but all is not lost. This technique works when serde and serde_json (or whatever other serde implementation you choose) are dynamic. The changes for that are straightforward and mechanical. I'll look into setup a script that subscribes to serde releases and re-publishes serde and a few of often used impls with the necessary changes as serde-dynamic etc. It's just a hack but this would then be a drop-in-replacement and maybe over time there is more acceptance for dynamic libraries (or even better cargo support in specifying dylib builds from the consuming crate).

@huahouye
Copy link
Author

I don't think this will really go anywhere with serde but all is not lost.

Yeah, I've realized that what you said. Feel free to close it when it should be. It is not just serde_json has this kind of issue, tokio, chrono crates are the same. My applications are building on them. It seems like that I have to dependency on them directly, grin and bear it.

I'll look into setup a script that subscribes to serde releases and re-publishes serde and a few of often used impls with the necessary changes as serde-dynamic etc.

The approach you mentioned it should work for those crates when their dependencies are not too complex to analyze, I am not expert on Rustlang and its ecosystem/toolchain, but that is what i can see so far.

Thanks again.

@rksm
Copy link
Owner

rksm commented Oct 1, 2022

Closing this for now, I might find time later to continue working on this.

@rksm rksm closed this as completed Oct 1, 2022
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

2 participants