AssemblyName.ProcessorArchitecture no longer returns the correct result

[ericstj]

Description

Starting in .NET 7.0 AssemblyName.ProcessorArchitecture is not correct.

Build tools like ResolveAssemblyReferences in MSBuild used this to determine if an application was referencing an assembly for a different architecture than what it targeted. https://github.com/dotnet/msbuild/blob/3777dcaf7edb3e86a070037ba53e742dd1872873/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs#L2578-L2599 cc @rainersigwald

I noticed that starting with .NET 7.0 MSBuild stopped warning in this case. I was able to trace it down to #63915 which moved the AssemblyName.GetAssemblyName implementation to use the one in System.Reflection.Metadata

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReader.netstandard.cs

Lines 14 to 45 in 3dbc850

	internal AssemblyName GetAssemblyName(StringHandle nameHandle, Version version, StringHandle cultureHandle, BlobHandle publicKeyOrTokenHandle, AssemblyHashAlgorithm assemblyHashAlgorithm, AssemblyFlags flags)
	{
	string name = GetString(nameHandle);
	// compat: normalize Nil culture name to "" to match original behavior of AssemblyName.GetAssemblyName()
	string cultureName = (!cultureHandle.IsNil) ? GetString(cultureHandle) : "";
	var hashAlgorithm = (Configuration.Assemblies.AssemblyHashAlgorithm)assemblyHashAlgorithm;
	// compat: original native implementation used to guarantee that publicKeyOrToken is never null in this scenario.
	byte[]? publicKeyOrToken = !publicKeyOrTokenHandle.IsNil ? GetBlobBytes(publicKeyOrTokenHandle) : Array.Empty<byte>();
	var assemblyName = new AssemblyName()
	{
	Name = name,
	Version = version,
	CultureName = cultureName,
	#pragma warning disable SYSLIB0037 // AssemblyName.HashAlgorithm is obsolete
	HashAlgorithm = hashAlgorithm,
	#pragma warning restore
	Flags = GetAssemblyNameFlags(flags),
	ContentType = GetContentTypeFromAssemblyFlags(flags)
	};
	bool hasPublicKey = (flags & AssemblyFlags.PublicKey) != 0;
	if (hasPublicKey)
	{
	assemblyName.SetPublicKey(publicKeyOrToken);
	}
	else
	{
	assemblyName.SetPublicKeyToken(publicKeyOrToken);
	}
	return assemblyName;

That version never set ProcessorArchitecture, for whatever reason, but it could. So that might be the fix for this if we want to fix it.

@jkotas you obsoleted this member in #59061. Given what you said about this, should we also proceed with obsoleting the PlatformTarget support in the compiler and references as well? If not, shouldn't we bring back this part of AssemblyName since our tools support building assemblies with it, and using it?

Reproduction Steps

Run attached project.
archConsole.zip

Expected behavior

It emits Amd64 for both the net6.0 and net7.0 builds.

Actual behavior

It emits Amd64 for the net6.0 build and None for the net7.0 build.

Regression?

Yes, from .NET 6.0

Known Workarounds

Calculate the ProcessorArchitecture of the assembly yourself? This would imply we find all the places in the dotnet tooling (like MSBuild or the compiler) that might use this property and fix them to avoid this regression.

Configuration

.NET 7.0 RC2, x64

Other information

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ericstj]

cc @VSadov

[rainersigwald]

Related discussion in #74040

[jkotas]

I do not think that this was intentional behavior change. We should be able to make it return what it returned before if it is important. @VSadov ?

Given what you said about this, should we also proceed with obsoleting the PlatformTarget support in the compiler and references as well?

The target support has to stay in compiler for as long the compiler targets .NET Framework.

Compiler targeting is superfluous in .NET Core. It was replaced by RID targeting. Use of compiler targeting in .NET Core only leads to problems. For example, see warning that we have added in dotnet/sdk#27776 .

Calculate the ProcessorArchitecture of the assembly yourself? This would imply we find all the places in the dotnet tooling (like MSBuild or the compiler) that might use this property and fix them to avoid this regression.

Note that the code fragment that you have linked won't work well for .NET Framework Arm64 targeting. The only way to make it work well for .NET Framework Arm64 targeting is to switch to reading PE header Machine architecture directly.

[VSadov]

I do not think that this was intentional behavior change.

no, I think this should be fixed for compat reasons.

[VSadov]

Will keep this open to track backporting to 7.0

[karelz]

@VSadov looks like the backporting to 7.0 didn't happen. Can we close it as 8.0 now?

[VSadov]

Yes, since 7.0 is out of support, I think this is no longer actionable.