Install LLVM DLL in the right place on Windows#152609
Install LLVM DLL in the right place on Windows#152609mati865 wants to merge 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
A-CI
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| /// compiler's sysroot. | ||
| /// | ||
| /// For example this returns `lib` on Unix and `bin` on Windows. | ||
| pub fn sysroot_runtime_libdir_relative(&self, compiler: Compiler) -> PathBuf { |
There was a problem hiding this comment.
What is the difference with relative_libdir? And if there is difference when should you choose this one over relative_libdir and vice versa?
There was a problem hiding this comment.
To be honest, I have no idea what is the deal with stage == 0, but I thought leaving it there would be safer.
Also, libdir_relative doesn't "Windowsize" the path, so uncommenting install.libdir in bootstrap.toml will result in a broken Windows build because the DLLs would go to build/host/stage1/lib.
Although I don't know how much we should care about it since it doesn't work anyway. With this PR, libLLVM will correctly go to stage1/bin, but rustc_driver and std still go to stage1/lib.
Technically, all shared objects should be handled by sysroot_runtime_libdir_relative or similar function.
There was a problem hiding this comment.
I can change this PR to use relative_libdir for installing LLVM, if we don't care about install.libdir case and stage0 is not a problem.
There was a problem hiding this comment.
install.libdir is meant for the Unix world, not for Windows. Setting install.libdir on Windows probably already results in a broken build before this PR as rustc_driver.dll ends up in the wrong dir.
There was a problem hiding this comment.
Fair enough, removed the new helper.
Unlike other systems, Windows requires runtime libraries to be present in `PATH` or right next to the binary. So, we copy the library next to the binary as the easier solution.
mati865
force-pushed
the
gnullvm-llvm-dll
branch
from
6707f87 to
425ec35
Compare
| @@ -2533,7 +2533,7 @@ fn maybe_install_llvm( | |||
| ), | |||
| )] | |||
| pub fn maybe_install_llvm_target(builder: &Builder<'_>, target: TargetSelection, sysroot: &Path) { | |||
| let dst_libdir = sysroot.join("lib/rustlib").join(target).join("lib"); | |||
| let dst_libdir = sysroot.join("lib/rustlib").join(target).join(libdir(target)); | |||
There was a problem hiding this comment.
Rustc probably expects it in lib when linking. I don't think this copy of the LLVM dylib is every used at runtime.
There was a problem hiding this comment.
That wouldn't work well on Windows. With both GNU toolchains you can link to DLL directly, but it's a hit-or-miss situation. Linking via import library is the right way to do it.
With MSVC's link.exe and lld-link the situation is clearer - you cannot link DLL directly at all, the import library is the only way.
Maybe we don't need it at all and should bundle the import library instead on Windows?
There was a problem hiding this comment.
This function should be installing both the DLL and the import library into the rustc linker path. Putting just the import library in rustlib and having a single copy of LLVM.dll in $(rustc --print sysroot)/bin would make sense to me. Doesn't have to be in the same PR IMO.
There was a problem hiding this comment.
Currently it only installs the DLL. I'll revert this change when I'm back home.
4 participants
Continuation of #151795 towards #151774.
Unlike other systems, Windows requires runtime libraries to be present in
PATHor right next to the binary.So, we copy the library next to the binary as the easier solution.
Tested building
rust-opensslin debug and release modes, but the difference is within noise margin.