-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Windows: Can't place DLLs in load-time link search locations. #326
Comments
For reference, here's some information about building native commands. In general this is not clearly documented, but that is being worked on, and I apologize for the lack of documentation so far! What's listed here is today's current state of affairs.
Given that information, can you perhaps rephrase what it is exactly you're trying to solve and why the current system is falling short? |
It's worth noting that OUT_DIR is not sufficient. The OUT_DIR is not the path of the final resulting binary; on windows dynamically linked binaries (specifically for test runs) will require that some DLLs be placed in the same folder as the final binary (target/test/) not in the native dependency directory passed in OUT_DIR (target/test/native/foo). Just to be absolutely clear here: This currently breaks windows builds. It's not possible to write a windows build file that is valid for dynamically linked librarys. Here's an example of a work around that currently works... ...but this is an issue that should be addressed at a top level in cargo. Specifically in https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_rustc/mod.rs#L167 we need either a TARGET_PATH, or something similar so that dynamically built libraries can be placed in the right folder during test runs. The current behavior only works for linking, not running. |
@shadowmint, can you be sure you've looked over http://crates.io/native-build.html? From what you're saying it sounds like you're misunderstanding the current purpose of OUT_DIR and friends.
I also want to be clear in that I do not understand why this is windows specific. On unix platforms you need an environment variable to tell you how to get to a dynamic library. Can you explain why you think this is a windows only issue and not a unix issue?
If you read over the documentation I believe you'll find out that this is not what OUT_DIR is.
I think you may be conflating a few issues here. When linking, cargo provides all necessary The gist of this is that I don't quite believe your claim that this breaks windows builds, which is why I'm looking for some concrete examples here which are built with an understanding of the documentation and still fall short of their needs. What you've explained does not appear to me today as a bug as we should have all the necessary infrastructure in place for building running binaries of all forms inside of a cargo project. Note, however, that cargo does not yet support installing a package or project onto a system. If you would like to run an artifact without the assistance of cargo, that is not currently under the purview of what cargo does. We would like to develop a story around this use case soon, but I want to be clear that this would not be solved by "just shuffle some things around". |
The issue is that (as far as I'm aware) windows linked binaries do not link to the path of the linked library (see http://stackoverflow.com/questions/107888/is-there-a-windows-msvc-equivalent-to-the-rpath-linker-flag) so if the DLL is built by the rust package, it won't be visible at run time when if it is visible at link time. This is not the case for unix systems where the rpath allows binaries to resolve the location of the shared library at runtime. So sure, we can say that for the (not yet implemented) install command (however it will work) cargo will have to know what build artifacts to ship into a release folder with the final binary on windows. However, during 'cargo test' this still has to work; that's what is specifically broken; the builder attempts to invoke the test binary (target/test/blah.exe) and the linked DLLs (/target/test/native/foo/foo.dll) are not in the DLL search path for the binary, so it fails to run. The original issue in this ticket is: "For DLLs built as part of a custom build step, there is no way of reliably placing it in the executables output directory. You can hard code it to target/ , but this breaks for tests." That's the issue here. |
Sorry, just to add: re-breaking windows builds; technically you're right. This won't stop a working 'cargo build' from running on windows; it just means if you want to run 'cargo test', it won't work. Sorry if my comment was too far into the realm of hyperbole. I guess it's more accurate to say: This means that 'cargo test' doesn't work on windows machines. |
Remember that rustc has rpath disabled by default, so everyone's in the same bucket, windows shouldn't be tougher to work with in this regard.
Indeed!
Oh dear, that's definitely a serious bug! I thought I had fixed all these problems by now. Do you have a test case that I could try out? |
Mhm. Cargo must already do doing some magic to make this work on other platforms.
but if I try to run that myself...
? Perhaps it's exporting some kind of environment variable to make this work. Also worth noting that on mac at least an rpath is assigned to the test binary:
Ironically enough I don't have a windows machine here at work, but I'm pretty confident that https://github.com/shadowmint/rust-dl-example/tree/dynamic should fail to run tests on windows (this is the repo from the examples above). |
This is what I meant when I said "You'll also find code in cargo for setting up the appropriate path for running binaries and tests." You're running the tests manually, not through cargo. You have also discovered that this is not isolated to windows, it is also a problem for unix (your DYLD_LIBRARY_PATH isn't set up). Currently it is only supported to run tests/executables through
Yes, this is how unix and windows work. Cargo is making sure you can run the executables and tests when you run them through cargo.
That's actually a bit of a red herring. I believe that's just there saying that there's a dependence on
I just checked it out and it succeeded on windows. It sounds like you're expecting to be able to run the test binaries by hand, and this is not currently a true assumption (this is why |
mm... I'll have to test it out later and report back. I'm pretty sure I was getting the same sort of dll missing error using 'cargo test' on my win7 box at home when I was messing around with this before. |
cargo 0.0.1-pre-nightly (0569c62 2014-08-12 06:54:26 +0000) rustc 0.12.0-pre-nightly (28b5e4588 2014-08-13 23:11:28 +0000) Still seems to do that on my windows 7 pro 64-bit machine? |
Can you update to the very tip of cargo? This may have been fixed quite recently. |
woo, that totally works in my virtualbox. I'm not sure why that nightly I was using would be a different version (?) since there haven't been any changes on the repo in a few days, but self-compiled version works! I'll try again at home to be sure later, but that looks like it's exporting the path stuff right. |
Works on my other machine too~ |
Ok, I'm going to close this in favor of #387 as it sounds like that's the real issue at hand. |
We'll probably have to add RPATH eventually when we build a binary for a release. For now, though, `cargo run` sets everything up correctly and it's how the develompent/testing is supposed to be done. See Alex Crichton's comments here: rust-lang/cargo#326
For DLLs built as part of a custom build step, there is no way of reliably placing it in the executables output directory. You can hard code it to target/ , but this breaks for tests.
Windows searches the exe's location, ./ , then the standard library locations, and finally PATH.
OUT_DIR and DEPS_DIR both point to target/deps//, so are not suitable.
Suggestion:
An environment variable that points to the output path would be enable you to place dynamic libraries there on Windows. It would be useful for other artefacts too, like art assets, config, etc.
See:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682586(v=vs.85).aspx#search_order_for_desktop_applications
The text was updated successfully, but these errors were encountered: