Update SDK to 23523.1

[amcasey]

Regular BuildOps update

[ghost]

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

[wtgodbe]

This failure is the one where multiple tools in a dotnet-tools.json file causes a concurrent access issue - was fixed in 8.0 by dotnet/sdk#35884. @JL03-Yue has that fix been ported to main yet?

[amcasey]

@wtgodbe It looks like it has.

[wtgodbe]

SDK -> Installer had been blocked for a really long time & only just got unblocked yesterday - I'm gonna try bumping this to the latest

[amcasey]

SDK -> Installer had been blocked for a really long time & only just got unblocked yesterday - I'm gonna try bumping this to the latest

It no longer compiles 😆

[amcasey]

Looks like maybe a trimmer annotation changed?

src\JSInterop\Microsoft.JSInterop\src\Infrastructure\DotNetDispatcher.cs(391,67): error IL2111: Method 'ScanTypeForCallableMethods(Type)' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method. [src\JSInterop\Microsoft.JSInterop\src\Microsoft.JSInterop.csproj]

[wtgodbe]

@eerhardt do you know anything about the trimming issue Andrew mentioned above?

[amcasey]

I haven't worked on trimmer warnings in a while, but the error seems superficially reasonable: we're operating on the type of a value of type object and we're enumerating its methods. It's not immediately obvious how the trimmer could know to preserve the methods of that type.

[amcasey]

This looks like the original trimmer clean-up pass: #41610

Edit: and the suppression appears to still be in place
Edit 2: removing all the suppressions from that file had no apparent effect, so maybe that's a red herring.

[amcasey]

Suppressing with an attribute works, but I wasn't sure what justification to give (and the XML file doesn't contain one?). @eerhardt? Or @JamesNK?

[UnconditionalSuppressMessage("Trimming", "IL2111", Justification = "Already suppressed in Microsoft.JSInterop.WarningSuppressions.xml, but that stopped working in https://github.com/dotnet/aspnetcore/pull/51588.")]

[eerhardt]

We shouldn't be suppressing this. This might be a bug in the latest ILLink analyzer (quite possibly from dotnet/runtime#92724 ?).

@sbomer - can you take a look? Just pull this PR and .\eng\build.cmd from the root of the repo. You should see:

C:\git\aspnetcore\src\JSInterop\Microsoft.JSInterop\src\Infrastructure\DotNetDispatcher.cs(391,67): error IL2111: Method 'ScanTypeForCallableMethods(
Type)' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of
 the requirements of the method. [C:\git\aspnetcore\src\JSInterop\Microsoft.JSInterop\src\Microsoft.JSInterop.csproj]

Note that this method is a local / nested method, so I'm not sure how it could be "accessed via reflection".

aspnetcore/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs

Lines 388 to 405 in 82ff924

	private static (MethodInfo methodInfo, Type[] parameterTypes) GetCachedMethodInfo(IDotNetObjectReference objectReference, string methodIdentifier)
	{
	var type = objectReference.Value.GetType();
	var assemblyMethods = _cachedMethodsByType.GetOrAdd(type, ScanTypeForCallableMethods);
	if (assemblyMethods.TryGetValue(methodIdentifier, out var result))
	{
	return result;
	}
	else
	{
	throw new ArgumentException($"The type '{type.Name}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")].");
	}
	static Dictionary<string, (MethodInfo, Type[])> ScanTypeForCallableMethods([DynamicallyAccessedMembers(JSInvokable)] Type type)
	{
	var result = new Dictionary<string, (MethodInfo, Type[])>(StringComparer.Ordinal);
	foreach (var method in type.GetMethods(BindingFlags.Instance | BindingFlags.Public))

BTW - @amcasey, there are other build errors in this PR:

C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\src\HubConnection.cs(866,47): error CA2007: Consider calling ConfigureAwait on the awaited t
ask (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2007) [C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\sr
c\Microsoft.AspNetCore.SignalR.Client.Core.csproj::TargetFramework=net9.0]
C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\src\HubConnection.cs(866,47): error CA2007: Consider calling ConfigureAwait on the awaited t
ask (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2007) [C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\sr
c\Microsoft.AspNetCore.SignalR.Client.Core.csproj::TargetFramework=netstandard2.0]
C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\src\HubConnection.cs(866,47): error CA2007: Consider calling ConfigureAwait on the awaited t
ask (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2007) [C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\sr
c\Microsoft.AspNetCore.SignalR.Client.Core.csproj::TargetFramework=netstandard2.1]
C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\src\HubConnection.cs(866,47): error CA2007: Consider calling ConfigureAwait on the awaited t
ask (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2007) [C:\git\aspnetcore\src\SignalR\clients\csharp\Client.Core\sr
c\Microsoft.AspNetCore.SignalR.Client.Core.csproj::TargetFramework=net462]

[eerhardt]

Ah, I looked some more, and this is basically dotnet/linker#2790, but now it is also in the ILLink Roslyn Analyzer, and not just in illink.exe.

It looks like we did suppress this warning in the Microsoft.JSInterop.WarningSuppressions.xml file, but the ILLink Roslyn Analyzer doesn't respect/know about that. Also the warnings coming from the 2 tools are different when analyzing this code. The warning from the ILLink Roslyn Analyzer is warning about the local function. The warning from illink.exe appears to be coming from the outer function.

So maybe a #pragma warning disable is appropriate here?

cc @vitek-karas @agocke @MichalStrehovsky

[amcasey]

FYI @BrennanConroy for the SignalR errors.

[BrennanConroy]

Just add .ConfigureAwait(false) on line 866 of HubConnection.

[JamesNK]

So maybe a #pragma warning disable is appropriate here?

Replace XML suppression with pragma: 533a66c (#51588)

[sbomer]

So maybe a #pragma warning disable is appropriate here?

Agreed.

Also the warnings coming from the 2 tools are different when analyzing this code. The warning from the ILLink Roslyn Analyzer is warning about the local function. The warning from illink.exe appears to be coming from the outer function.

The Roslyn analyzer warning happens at this line:
var assemblyMethods = _cachedMethodsByType.GetOrAdd(type, ScanTypeForCallableMethods);
The warning text mentions ScanTypeForCallableMethods, but the warning itself is produced from the outer function just like in illink.