-
Notifications
You must be signed in to change notification settings - Fork 790
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
Move build to net 5.0 #11019
Move build to net 5.0 #11019
Conversation
You'll also need to revert/undo #10986 as part of this. |
I think we can revert that once, we have successfully inserted :-) |
I think the issue is the nunit adapter |
5063530
to
c427c01
Compare
@@ -437,6 +437,11 @@ try { | |||
$buildTool = InitializeBuildTool | |||
$toolsetBuildProj = InitializeToolset | |||
TryDownloadDotnetFrameworkSdk | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brettfo , this was the main problem, the ci sets DOTNET_ROOT to a the global location. Here we set it to the local download location first, so that the local and global frameworks are selectable by dotnet.exe
let isNetCoreAppProcess = System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription.StartsWith ".NET Core" | ||
if isNetCoreAppProcess then PrimaryAssembly.System_Runtime | ||
else PrimaryAssembly.Mscorlib | ||
let includesSystem_Runtime = dllReferences |> Seq.exists(fun f -> Path.GetFileName(f).Equals("system.runtime.dll",StringComparison.InvariantCultureIgnoreCase)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vzarytovskii ,
this is what fixed the il baseline issue. Here we set the primary assembly differently on NET5.0 than netcoreapp3.1.
The change is to set the primary assembly to system_runtime whenever the developer specifies system.runtime.dll is one of the references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah that makes sense. We were relying on tfm name/description.
Fixes #10576
netcoreapp3.1 is trippy old fashioned. we need to be using the new hotness net5.0