-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Update SDK #34236
Update SDK #34236
Conversation
This ingests the global imports change from the SDK
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.targets))\Directory.Build.targets" /> | ||
|
||
<ItemGroup> | ||
<Import Remove="System.Net.Http" /> |
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.
?
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.
There is a type name conflict between System.Net.Http.HttpMethod and Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod.
Seems like this was the only conflict in our repo so I just removed it for kestrel.
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.
Ok, so this removes the default using System.Net.Http
?
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.
Yes.
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.
This is an interesting problem. It finally bit in the butt.
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.
I'm impressed that this is the only instance.
@@ -5,5 +5,6 @@ | |||
<IsPackable>false</IsPackable> | |||
<DisablePackageReferenceRestrictions>true</DisablePackageReferenceRestrictions> | |||
<Nullable>disable</Nullable> | |||
<DisableImplicitNamespaceImports>true</DisableImplicitNamespaceImports> |
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.
Why not just web?
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.
The tools use Microsoft.Build.Tasks which conflicts with System.Threading.Tasks. Since it's just tools, I just disabled everything instead of just the specific conflicts.
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.
Why does it conflict? Because we don't have the new compiler that hides these conflict warnings? Can you clarify?
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.
It's not a warning, it's an error for ambiguous types.
error CS0104: 'Task' is an ambiguous reference between 'Microsoft.Build.Utilities.Task' and 'System.Threading.Tasks.Task
This ingests the change dotnet/sdk#18459 into our repo.
FYI @captainsafia @Tratcher since this will overlap your ops rotations, probably.