Add the rust lib dir (containing std-<hash>.dll) to the path on windows #1093
Conversation
Adds sysroot/bin to the path on Windows which fixes finding libstd in dynamic builds. This fixes a load of dylib tests in the cargo testsuite on Windows. This will result in rustc.exe being directly in the path although it is after CARGO_HOME/bin so the shim should be found first.
|
Thanks for tackling this @mcgoo. This path munging code ... omg, so frustrating. It's so hard to imagine what is going on at runtime with all the layers of indirection and environment frobbing. A worry I have with this patch is that it might accidentally be reverting what the original PR that caused this was fixing - that all I guess I'm willing to try this and see what happens. The three failures on appveyor seem legit. Probably some patch-munging corner case that requires things to be perfectly right in a way that is contradictory to other goals. |
fix `cargo test` of dylib projects for end user runs too Fixes running `cargo test` and `cargo test --target <target>` for dylib projects. Moves the logic just landed in #3996 into cargo itself, so cargo sets the dylib path for anyone running `cargo test` or `cargo run`. Current master sets the dylib path only for `cargo test` on cargo itself. This PR pins to rustup 1.2.0 for the purposes of testing. If rust-lang/rustup#1093 ends up working out, then this PR would only be important for non-rustup users and people doing cross testing, `cargo test --target <target>`. Arguably https://github.com/mcgoo/cargo/blob/ed273851f8bc76f726eda4a2e2a7bb470c3718bc/src/cargo/ops/cargo_rustc/context.rs#L249-L253 should point to lib/rustlib/\<host triple\>/lib instead of sysroot/lib, because I think if the libs are different, you will never be able to compile a working plugin anyway, and for the host=target case you get the lib/rustlib/\<host triple\>/lib anyhow. Is there ever a case where the lib/rustlib/\<host triple\>/lib and sysroot/lib versions of the libs would be expected to differ? This is not a huge deal for me one way or the other - it doesn't impact my workflow at all. I nearly dropped it when I saw @alexcrichton had made it all work in 3996, but I think it's worth doing because it removes a surprise. It certainly would have saved me a couple of days of confusion. Either way, thanks for looking it over.
|
If this regresses something, it means we need more regression tests. The test failures on appveyor are a known flaky test, but it's surprising that three failed, let's give bors a try... |
|
📌 Commit 0555dbe has been approved by |
|
I actually ended up with this change in cargo that fixes the path for both cross and native builds, which rustup would not be able to do easily, so anything running under cargo is fixed already. I still think this change is correct from the standpoint of consistency with other platforms, but I don't know what it actually fixes as such anymore. |
Add the rust lib dir (containing std-<hash>.dll) to the path on windows I'm not sure if this is the right thing or not, but this fixes some of rust-lang/cargo#3394. My apologies if it undoes earlier path related fixes. The dylib tests in the cargo testsuite work. `cargo test` of a dylib crate works. It does nothing for the `cargo install` case. It adds sysroot/bin to the path on Windows which fixes finding libstd. A side effect of this is that rustc.exe is directly in the path although it is after CARGO_HOME/bin so the shim should be found first. The behavior of prepend_path in case of a corrupt path is changed - it used to clobber the existing path with the new path, and now it will leave leave the existing path unchanged. It leaves LD_LIBRARY_PATH set on Windows also as I believe it is used by MinGW.
|
☀️ Test successful - status-appveyor, status-travis |
I'm not sure if this is the right thing or not, but this fixes some of rust-lang/cargo#3394. My apologies if it undoes earlier path related fixes.
The dylib tests in the cargo testsuite work.
cargo testof a dylib crate works.It does nothing for the
cargo installcase.It adds sysroot/bin to the path on Windows which fixes finding libstd. A side effect of this is that rustc.exe is directly in the path although it is after CARGO_HOME/bin so the shim should be found first.
The behavior of prepend_path in case of a corrupt path is changed - it used to clobber the existing path with the new path, and now it will leave leave the existing path unchanged.
It leaves LD_LIBRARY_PATH set on Windows also as I believe it is used by MinGW.