Skip to content

Rustc no longer builds for msvc 17 #48749

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

Closed
snf opened this issue Mar 5, 2018 · 4 comments
Closed

Rustc no longer builds for msvc 17 #48749

snf opened this issue Mar 5, 2018 · 4 comments
Labels
C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows

Comments

@snf
Copy link
Contributor

snf commented Mar 5, 2018

It appears to be since this commit yesterday: d69b248#diff-9a46b13a265be2ded97369dddf5037f5

@alexcrichton can you have a look at it?, discussing in IRC, it seems that changing the line 88 back from "build/bin" to only "bin" does the work.

@alexcrichton
Copy link
Member

@snf can you provide more logs for this build?

@ollie27
Copy link
Member

ollie27 commented Mar 5, 2018

https://pastebin.mozilla.org/9079141 was the log posted on IRC. The issue is that when using MSBuild as the cmake generator llvm-config.exe is located in llvm\bin\llvm-config.exe and llvm\build\Release\bin\llvm-config.exe not llvm\build\bin\llvm-config.exe.

@retep998
Copy link
Member

retep998 commented Mar 5, 2018

Also please never hardcode / in paths. That should be dir.join("build").join("bin"); due to certain conditions where / is treated as a literal character and not a path separator, such as \\?\ paths.

@estebank estebank added A-build O-windows-msvc Toolchain: MSVC, Operating system: Windows labels Mar 5, 2018
@alexcrichton
Copy link
Member

Ah thanks @ollie27! Yes I can look into fixing this.

@TimNN TimNN added the C-bug Category: This is a bug. label Mar 6, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 9, 2018
For LLD integration the path to `llvm-config` needed to change to inside the
build directory itself (for whatever reason) but the build directory is
different on MSBuild than it is on `ninja` for MSVC builds, so the path to
`llvm-config.exe` was actually wrong and not working!

This commit removes the `Build::llvm_config` function in favor of the source of
truth, the `Llvm` build step itself. The build step was then updated to find the
right build directory for MSBuild as well as `ninja` for where `llvm-config.exe`
is located.

Closes rust-lang#48749
bors added a commit that referenced this issue Mar 9, 2018
rustbuild: Fix MSBuild location of `llvm-config.exe`

For LLD integration the path to `llvm-config` needed to change to inside the
build directory itself (for whatever reason) but the build directory is
different on MSBuild than it is on `ninja` for MSVC builds, so the path to
`llvm-config.exe` was actually wrong and not working!

This commit removes the `Build::llvm_config` function in favor of the source of
truth, the `Llvm` build step itself. The build step was then updated to find the
right build directory for MSBuild as well as `ninja` for where `llvm-config.exe`
is located.

Closes #48749
bors added a commit that referenced this issue Mar 9, 2018
rustbuild: Fix MSBuild location of `llvm-config.exe`

For LLD integration the path to `llvm-config` needed to change to inside the
build directory itself (for whatever reason) but the build directory is
different on MSBuild than it is on `ninja` for MSVC builds, so the path to
`llvm-config.exe` was actually wrong and not working!

This commit removes the `Build::llvm_config` function in favor of the source of
truth, the `Llvm` build step itself. The build step was then updated to find the
right build directory for MSBuild as well as `ninja` for where `llvm-config.exe`
is located.

Closes #48749
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows
Projects
None yet
Development

No branches or pull requests

6 participants