-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Most recent build tools update broke support for building on VS2017 #7592
Comments
I can't repro this at the moment. I don't get very far when trying to build in a VS2017 developer command prompt.
|
@chcosta, the Windows Build instructions (https://github.com/dotnet/coreclr/blob/master/Documentation/building/windows-instructions.md) were updated this morning to include the minimum install requirements for VS2017. |
Following those instructions has not affected my build result in any way. :( |
Ah, I see. The version of CMAKE you have installed doesn't support VS2017. The instructions should probably explicitly call out whichever version added that support (not sure off the top of my head). |
Looks like 3.7.2 is the minimum required CMAKE version, which matches what the documentation says. Is that what version you are on @chcosta? |
My bad, I only got through the VS2017 section. I'm on 3.6.1. Perhaps you can add a note to the VS2017 section and explicitly call that out? |
It looks like the MSBuild v15.0 toolset has defined In MSBuild v14.0, Microsoft.CSharp.CurrentVersion.targets would import Microsoft.CSharp.Core.targets which was in the same directory.
In MSBuild v15.0, if
For us, this leads to a mismatch because the Roslyn path we use in buildtools is not on MSBuild v15.0. You can work around the issue by explicitly specifying |
@chcosta, I'm not familiar with the buildtools setup (it kind-of looks like we are manually doing a lot of MSBuild stuff, rather than just allowing the CompilerToolset to do it for us), but is there any reason why we can't modify the targets to check the VisualStudioVersion and set CSharpCoreTargetsPath if we are on VS2017? |
Also note, that I didn't actually look at what changed in BuildTools between the last update (around 1/9), and my change. I only looked into the problem as it exists today. |
Are you trying to override the compiler only or the compiler + build task? For both here is a pointer to how we do it for our own builds. If this stops working we can't check in so it's probably a good item to depend on 😄 https://github.com/dotnet/roslyn/blob/master/build/Targets/Settings.props#L121 |
We will need to override the compiler and build task in order to use the latest and greatest compiler features. We will try and use the same pattern you guys are doing. |
@chcosta can you please try setting the CSharpCoreTargetsPath to the right location in BuildTools targets files where we import the other changes. |
Using
Fails differently with
|
The problem you're hitting is using
|
@chcosta yep doing |
@chcosta Is there an ETA for this? |
Later this week. It is still near the top of my list |
This same error seems to have crept back in sometime in the past several days. |
The following commit broke support for building on VS2017: 8f23fca. Note: The support for building on VS2017 was validated before the commit, but was actually merged just after the aforementioned was merged.
The failure resembles the following:
I have confirmed that reverting the commit resolves the issue (but I don't think we should revert it). Instead, we should find out what in the new toolset has broken the VS2017 support and update the toolset again.
The text was updated successfully, but these errors were encountered: