Upgrade our .NET SDK to 5.0 RC 1 #48404
Conversation
| "allowPrerelease": true, | ||
| "rollForward": "major" | ||
| }, | ||
| "tools": { | ||
| "dotnet": "5.0.100-preview.8.20417.9", | ||
| "dotnet": "5.0.100-rc.1.20452.10", | ||
| "vs": { | ||
| "version": "16.8" | ||
| }, |
There was a problem hiding this comment.
Should the next line for xcopy-msbuild be updated to preview 3 instead of preview 2?
There was a problem hiding this comment.
(I think the answer is no because we only rebuild that xcopy package when we need to...)
There was a problem hiding this comment.
We only do it when we need to, but the likelihood is that we'll need to. @jaredpar, is there a new version? If not, can you make one?
|
One the build failures is related to this breaking change. |
| @@ -33,7 +33,9 @@ private static async Task<int> Main(string[] args) | |||
| Application.Run(); | |||
| }); | |||
|
|
|||
| #pragma warning disable CA1416 // analyzer is warning this API is only available on Windows, but this project only targets windows. | |||
There was a problem hiding this comment.
Using a Debug.Assert sounds more neat than a suppression.
There was a problem hiding this comment.
This appears to be an analyzer bug actually...
There was a problem hiding this comment.
if the project targets windows it shouldn't warn for calling windows only API, there were MSBuild bug related to version less target platform dotnet/sdk#13494, might be caused from that
There was a problem hiding this comment.
@buyaa-n The project target both, net5.0-windows and net472:
<TargetFrameworks>net472;net5.0-windows</TargetFrameworks>There was a problem hiding this comment.
The analzyer is off by default for net472
There was a problem hiding this comment.
I'm guessing the net472 isn't being counted as "windows" and I guess maybe there's Mono scenarios that apply here.
|
There's going to be some changes in the function pointer tests necessary. We also need to update https://github.com/dotnet/roslyn/blob/master/docs/contributing/Building,%20Debugging,%20and%20Testing%20on%20Windows.md#developing-with-visual-studio-2019. Please also make sure to send out an email to the wider team about up'ing the requirements. |
|
@Youssef1313 Unfortunately there's also a bug that prevents it from correctly handling multi-targeted projects and the fix isn't in RC1, haven't looked at what we'll do for that yet. |
|
(or at least I don't think the fix is in RC1...) |
I believe that's correct, it should be in RC2 |
jasonmalinowski
force-pushed
the
fix-integration-tests
branch
from
9b0c82c to
a30757d
Compare
| @@ -502,12 +502,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic | |||
| #If DEBUG Then | |||
| ' Compile time asserts. | |||
| Private Const s_delegateRelaxationLevelMask_AssertZero = SmallFieldMask.DelegateRelaxationLevelMask - ConversionKind.DelegateRelaxationLevelMask | |||
| Private _delegateRelaxationLevelMask_Assert1(s_delegateRelaxationLevelMask_AssertZero) As Boolean | |||
| Private _delegateRelaxationLevelMask_Assert2(-s_delegateRelaxationLevelMask_AssertZero) As Boolean | |||
| Private ReadOnly _delegateRelaxationLevelMask_Assert1(s_delegateRelaxationLevelMask_AssertZero) As Boolean | |||
There was a problem hiding this comment.
The IDE Analyzer MakeFieldReadOnly is already enabled. Why it didn't catch these?
There was a problem hiding this comment.
The updated analyzer catches more cases.
| @@ -15,6 +16,7 @@ namespace Microsoft.CodeAnalysis.Interactive | |||
| { | |||
| internal static class InteractiveHostEntryPoint | |||
| { | |||
| [SupportedOSPlatform("windows")] | |||
There was a problem hiding this comment.
Can this be surrounded by #if NET5_0 instead of having to add the PlatformAttributes.cs file? or this won't work?
There was a problem hiding this comment.
I prefer to localize the directives to PlatformAttributes.cs and have the code itself look uniform and, where possible, modern.
| @@ -15,6 +16,7 @@ namespace Microsoft.CodeAnalysis.Interactive | |||
| { | |||
| internal static class InteractiveHostEntryPoint | |||
| { | |||
| [SupportedOSPlatform("windows")] | |||
There was a problem hiding this comment.
Is there not a constant for these?
There was a problem hiding this comment.
I didn't see one. dotnet/runtime seems to use strings.
ghost
added this to the
|
Merging as this is needed to unblock integration test running in master. |
No description provided.