-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 issue with Rust 1.37.0 #64340
Comments
This bug is can also be reproduced with a small project: link (be sure to use a linked revision, since project is under active development, and upstream may change). If I try to compile |
Just finished bisecting and the culprit commit affecting my project is 185dceb which appears to be the same changes affecting #66265 and #65610. @alexcrichton is there any chance we can revert this? or is there a way to move this issue forwards? |
Sorry I don't know much about that change, I didn't author it, I'd recommend bringing it up on the PR and/or with the release team to figure out a revert. |
Thank you, appologies for incorrectly @'ing you, I shall do just that. |
Is there are strong reason for using Rust dylibs instead of rlibs or cdylibs for these projects? Rust dylibs are rather underspecified and the concrete set of symbols exported from them is an implementation detail. |
Yes, if the built library should be usable by both Rust and other programming language. |
@michaelwoerister so I need to use dylibs over rlibs and cdylibs for a couple of reasons:
Very happy to be corrected if any of the above is incorrect. |
@alexkornitzer It sounds like your project should not be affected by the PR in question. Can you share a reproduction? |
@michaelwoerister annoyingly it is private code, let me see if I can reproduce it in a generic project. |
Btw, we can confirm that reverting the PR in question with locally build Rust compiler solves the problem on Rust 1.37 and Rust 1.39. cc @skletsun |
@michaelwoerister here is a relatively streamlined build that will cause linker errors. Commenting out the Consul crate (https://github.com/AlexKornitzer/dylib-errors/blob/1ab7cdbddebec3353a80a40d7b8ae0899f6ac581/shared/src/lib.rs#L2) will cause the majority of the errors to go away (no idea why!), I believe the NNG errors are due to related C issues, but am unsure. I will make a branch without NNG and see if the ones caused by Consul go away. The below builds on 1.36.0 but not above: Build output: |
Thank you, @alexkornitzer! That looks like a reasonably small reproduction case. If you can reduce it further, as you mentioned, that would be even better. Let's nominate this issue for priority assignment. Looks like it might be a proper regression. |
Right @michaelwoerister, that has now been done so from looking over the issues, I have a C based linking error and a Rust based error, which I have split into two branches:
Hopefully these are reduced enough, the consul one is fairly large in regards to deps but as I am not sure what is causing it I have no idea how to isolate that further. Thanks again for looking into this. |
@michaelwoerister has identified the issue for branch 2 (error/consul) and has suggested opening a new issue that is more targeted to its root cause, hence #67276 |
So I have tried @Zoxc solution as mentioned here (#65610 (comment)) for branch 1 (error/nng) and the appears to work. In https://gitlab.com/neachdainn/nng-sys/blob/master/src/bindings.rs#L665 I added |
triage: this has been prioritized as P-high, it sounds like progress is being made (or at least sub-issues like #67276 are being filed), so I'm going to remove the nomination label |
I have refreshed @popzxc bug example: This demonstration works fine in release mode but fails in debug with error.
Command to run is: P.S. Sorry for huge size of example. |
So with the dylib part fixed by @michaelwoerister 🙌 we are now only left with the issue of linking C code through dylibs. Originally I thought I could just decorate the Without insight from the rust team we will not know how we should proceed with this issue. Again the small isolated example is here: https://github.com/AlexKornitzer/dylib-errors/tree/error/nng and should give the following (latest nightly):
Surely with Also the above |
As per #65610 you are required to annotate the extern blocks with |
@nagisa understood, but as these files are the output of |
Yes. What |
Error: Parsing assign command in comment failed: ...'n pnkfelix' | error: user should start with @ at >| ''... Please let |
Visiting for P-high review I was looking at this the other day, trying to decide whether it would be suitable subject for screencast. I still suspect the core issue may be to make an upstream change with bindgen. I'll continue to own making progress here. |
Hello, my code started failing with the following linking issues between
The issue is still present in the latest nightly. Adding |
Visited during the compiler team's P-high review meeting. As far as we can tell, the original issue has been resolved. Therefore, we are going to close this as completed. If there are any issues in this thread which have not been resolved, please file new issues for those so we can track them properly. Thanks! |
See build log.
It looks like name mangling doesn't work correctly when linking with a dynamic library which is also depends on
libstd
.The application structure is the following:
dylib
cratejava_bindings
, containing 400+ dependencies and is dynamically linked tolibstd.dylib
.exonum-java
binary application, depends onjava_bindings
and is dynamically linked tolibstd.dylib
.The compilation works well on Rust 1.36.0, but doesn't work on 1.37.0 and nightly versions (such as
rustc 1.39.0-nightly (c6e9c76c5 2019-09-04)
).To reproduce:
git clone https://github.com/exonum/exonum-java-binding cd exonum-java-binding/exonum-java-binding/core/rust/exonum-java rustup default nightly cargo build
Sorry for such heavy project, couldn't reproduce on an easier example.
The text was updated successfully, but these errors were encountered: