Skip to content

Support VS 2017 - #42225

Merged
bors merged 1 commit into
rust-lang:masterfrom
brson:vs2017
Jun 3, 2017
Merged

Support VS 2017#42225
bors merged 1 commit into
rust-lang:masterfrom
brson:vs2017

Conversation

@brson

@brsonbrson commented May 25, 2017

Copy link
Copy Markdown
Contributor

Fixes#38584

This replaces all the MSVC linker logic with that from the 'gcc' crate. The code looks the same, but there could be regressions.

I've only tested this with x86_64.

r? @alexcrichton
cc @vadimcn@retep998

@brsonbrson added the relnotes Marks issues that should be documented in the release notes of the next release. label May 25, 2017
@brson

Copy link
Copy Markdown
ContributorAuthor

I'm seeing issues in cmake that i want to investigate still.

@brsonbrson closed this May 25, 2017
@brsonbrson reopened this May 25, 2017
@brson

Copy link
Copy Markdown
ContributorAuthor

Ok, I think this patch is sufficient to fix rustc at least. Both gcc-rs and cmake still need to be updated to be able to find msbuild correctly. I'll file separate issues on those.

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented May 25, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 88b9cf8 has been approved by alexcrichton

@brson

Copy link
Copy Markdown
ContributorAuthor

The rustup patch failed on i686.

@brson

Copy link
Copy Markdown
ContributorAuthor

@bors r- I'm still not confident about what's going on in i686

@brson

Copy link
Copy Markdown
ContributorAuthor

@bors r+ I want to see what bors thinks.

@bors

bors commented May 26, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 88b9cf8 has been approved by brson

@frewsxcvfrewsxcv mentioned this pull request May 26, 2017
@ishitatsuyuki

Copy link
Copy Markdown
Contributor

Rollup failure.

PS: @frewsxcv please be careful when rolling up non-rollup priority PRs, inspect the PR comments before adding it to list.

@brson

Copy link
Copy Markdown
ContributorAuthor

I'm not sure how the appveyor images are set up, but I suspect that after this lands we will be building Rust releases with VS 2017 instead of VS 2015.

@est31

Copy link
Copy Markdown
Member

The error from the rollup is:

error: linking with `link.exe` failed: exit code: 1171
|
= note: "C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\bin\\amd64_x86\\link.exe" "/NOLOGO" "/NXCOMPAT" "/LARGEADDRESSAWARE" "/SAFESEH" "/LIBPATH:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\i686-pc-windows-msvc\\release\\deps\\std-ceb42dcce624041e.0.o" "/OUT:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\i686-pc-windows-msvc\\release\\deps\\std-ceb42dcce624041e.dll" "/DEF:C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\lib.def" "C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\i686-pc-windows-msvc\\release\\deps\\std-ceb42dcce624041e.crate.metadata.o" "/OPT:REF,ICF" "/DEBUG" "/LIBPATH:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\i686-pc-windows-msvc\\release\\deps" "/LIBPATH:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\release\\deps" "/LIBPATH:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\native\\compiler-rt\\." "/LIBPATH:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "advapi32.lib" "ws2_32.lib" "userenv.lib" "shell32.lib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\librand-cbaabb050663bb1e.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libpanic_unwind-7a8f267e1f472275.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\liblibc-913fac93272423bb.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libcollections-0e863adcb979d99b.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libunwind-f8f46edb99891d80.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libstd_unicode-72685d3c05b2de5b.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\liballoc-2fe699c323a9fcd5.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\liballoc_system-1a9ffce435c6813e.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libcore-6796d5cf3cb4eed3.rlib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustc.RkAu4DLaw1LX\\libcompiler_builtins-13752c636400e0ca.rlib" "libcmt.lib" "/DLL" "/IMPLIB:C:\\projects\\rust\\build\\i686-pc-windows-msvc\\stage1-std\\i686-pc-windows-msvc\\release\\deps\\std-ceb42dcce624041e.dll.lib"
= note: LINK : fatal error LNK1171: unable to load mspdbcore.dll (error code: 126) 

The version number 15.0 corresponds to 2015, not 2017. And the linker is the only thing from VS we are running, right?

@brson

Copy link
Copy Markdown
ContributorAuthor

Huh. That's not the error I expected :-/

@retep998

retep998 commented May 26, 2017

Copy link
Copy Markdown
Contributor

The version number 15.0 corresponds to 2015, not 2017. And the linker is the only thing from VS we are running, right?

No, 15 corresponds to 2017 and 14 corresponds to 2015.

note: LINK : fatal error LNK1171: unable to load mspdbcore.dll (error code: 126)

This error is caused when you're running a cross linker but you forgot to add the host toolchain to PATH (because the cross linker folder depends on some DLLs from the host toolchain folder). The existing code for 11 12 and 14 does this correctly. More specifically, rustc seems to set the PATH for the linker later on, which means whatever Command I returned would have its PATH overridden so I couldn't at the host toolchain folder that way. I fix this I hacked in some code to add the host toolchain folder to the PATH that was being set later on, and your new version shelling out to gcc doesn't do that correctly.

Comment threadsrc/librustc_trans/back/link.rs Outdated

@retep998retep998May 26, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is right here. path is not supposed to be the directory with the linker, but rather the directory of the host toolchain (aka the folder with the linker whose target and host are both the same as the host of the linker that you are using). You'll have to teach gcc how to provide that information so you can pull it out here. And yes, you'll have to do this for VS 2017 too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @retep998

@est31

Copy link
Copy Markdown
Member

No, 15 corresponds to 2017 and 14 corresponds to 2015.

Right. I have mistyped. The version number of the pasted text is 14.0 though, so it still corresponds to 2015.

@brson
brsonforce-pushed the vs2017 branch 2 times, most recently from 5eee90f to 1d177d5CompareMay 26, 2017 18:50
@brson

Copy link
Copy Markdown
ContributorAuthor

@bors r+

I want to see what bors thinks again.

This is currently using a git replacement of the gcc crate.

@bors

bors commented May 26, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 1d177d5 has been approved by brson

@brson

Copy link
Copy Markdown
ContributorAuthor

@bors r+

I changed the travis config to fail so this won't land with the crate replacement.

@bors

bors commented May 26, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 0e845bf has been approved by brson

@brson

Copy link
Copy Markdown
ContributorAuthor

@bors r+

@bors

bors commented Jun 1, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 5ffc7bb with merge d390024...

@Mark-Simulacrum

Copy link
Copy Markdown
Member

AppVeyor passed; I'm canceling this. https://ci.appveyor.com/project/rust-lang/rust/build/1.0.3454

@bors retry
@bors r-

@brson

brson commented Jun 1, 2017

Copy link
Copy Markdown
ContributorAuthor

This is ready to go now.

r? @alexcrichton

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Jun 1, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit da100fe has been approved by alexcrichton

bors added a commit to rust-lang/rustup that referenced this pull request Jun 1, 2017
VS 2017 updates
Once rust-lang/rust#42225 lands rustup should build correctly under VS 2017.
r? @Diggsey
bors added a commit to rust-lang/rustup that referenced this pull request Jun 1, 2017
VS 2017 updates
Once rust-lang/rust#42225 lands rustup should build correctly under VS 2017.
r? @Diggsey
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors rollup

We know that this passes AppVeyor, and Travis PR passed (and I think we've passed regular travis before).

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 1, 2017
Support VS 2017
Fixesrust-lang#38584
This replaces all the MSVC linker logic with that from the 'gcc' crate. The code looks the same, but there could be regressions.
I've only tested this with x86_64.
r? @alexcrichton
cc @vadimcn@retep998
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 2, 2017
Support VS 2017
Fixesrust-lang#38584
This replaces all the MSVC linker logic with that from the 'gcc' crate. The code looks the same, but there could be regressions.
I've only tested this with x86_64.
r? @alexcrichton
cc @vadimcn@retep998
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 2, 2017
Support VS 2017
Fixesrust-lang#38584
This replaces all the MSVC linker logic with that from the 'gcc' crate. The code looks the same, but there could be regressions.
I've only tested this with x86_64.
r? @alexcrichton
cc @vadimcn@retep998
bors added a commit that referenced this pull request Jun 2, 2017
Rollup of 10 pull requests
- Successful merges: #41981, #42225, #42310, #42319, #42335, #42343, #42355, #42360, #42370, #42372
- Failed merges:
bors added a commit that referenced this pull request Jun 2, 2017
Rollup of 10 pull requests
- Successful merges: #41981, #42225, #42310, #42319, #42335, #42343, #42355, #42360, #42370, #42372
- Failed merges:
@bors
bors merged commit da100fe into rust-lang:masterJun 3, 2017
@rust-langrust-lang deleted a comment Feb 11, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotesMarks issues that should be documented in the release notes of the next release.S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@brson@alexcrichton@bors@ishitatsuyuki@est31@retep998@kennytm@Mark-Simulacrum@shepmaster@carols10cents