[Breaking change]: ILVerify.IResolver takes AssemblyNameInfo parameters

KalleOlaviNiemitalo commented 3 months ago

Description

In the ILVerify.IResolver interface defined in the Microsoft.ILVerification package, the type of the first parameter of each method is now System.Reflection.Metadata.AssemblyNameInfo rather than System.Reflection.AssemblyName.

Version

.NET 9 Preview 5

Previous behavior

The type of the first parameter was System.Reflection.AssemblyName:

namespace ILVerify
{
    public interface IResolver
    {
        PEReader ResolveAssembly(AssemblyName assemblyName);
        PEReader ResolveModule(AssemblyName referencingAssembly, string fileName);
    }
}

New behavior

The type of the first parameter is System.Reflection.Metadata.AssemblyNameInfo:

namespace ILVerify
{
    public interface IResolver
    {
        PEReader ResolveAssembly(AssemblyNameInfo assemblyName);
        PEReader ResolveModule(AssemblyNameInfo referencingAssembly, string fileName);
    }
}

Type of breaking change

[X] Binary incompatible: Existing binaries might encounter a breaking change in behavior, such as failure to load or execute, and if so, require recompilation.
[X] Source incompatible: When recompiled using the new SDK or component or to target the new runtime, existing source code might require source changes to compile successfully.
[ ] Behavioral change: Existing binaries might behave differently at run time.

Reason for change

“Avoids dependency on AssemblyName.Culture and globalization (ICU)” (https://github.com/dotnet/runtime/pull/102036)

Recommended action

If your application references the Microsoft.ILVerification package and defines a type that implements the ILVerify.IResolver interface, change the implementation to take AssemblyNameInfo parameters.

Feature area

SDK

Affected APIs

ILVerify.IResolver.ResolveAssembly
ILVerify.IResolver.ResolveModule

These methods are not overloaded.

Associated WorkItem - 292110

KalleOlaviNiemitalo commented 3 months ago

@jkotas, I filed this because I maintain some code that implements the interface and will be broken by the change. Can you verify that the feature area is correct?

jkotas commented 3 months ago

Looks good to me. Thank you!

KalleOlaviNiemitalo commented 3 months ago

I tried to send email as instructed here

https://github.com/dotnet/docs/blob/29f0af3a3d39416ba940c83f9465baa0551df0ed/.github/ISSUE_TEMPLATE/02-breaking-change.yml?plain=1#L114

but my email was rejected.

gewarren commented 2 months ago

@jkotas @adamsitnik I don't see this Microsoft.ILVerification package being indexed for the .NET API documentation. Should it be?

jkotas commented 2 months ago

Microsoft.ILVerification is a package for specific very advanced users. We have number of packages like that. https://www.nuget.org/packages/Microsoft.Diagnostics.Runtime. is another package in this category.

APIs in these packages do not go through API reviews, they are not documented on https://learn.microsoft.com/, and we are more flexible with breaking changes in these packages compared to core .NET APIs. I think it is the right place to be. We want to keep these packages very low-cost.

If you think that it does not make sense to publish breaking changes in these packages as part of .NET release notes, I would be fine with that. We can find an alternative.

gewarren commented 2 months ago

@PriyaPurkayastha What do you think👆?

KalleOlaviNiemitalo commented 2 months ago

If this breaking change is not added to .NET release notes, I hope it will be in the package release notes instead. Currently, those only link to https://go.microsoft.com/fwlink/?LinkID=799421, which redirects to https://github.com/dotnet/core/tree/main/release-notes.

gewarren commented 2 months ago

Since these are undocumented APIs, it doesn't make sense to document a breaking change to a parameter type. I've assigned the issue to Jan in case you want to add it to the package release notes.

jkotas commented 2 months ago

I hope it will be in the package release notes instead

We can do that: https://github.com/dotnet/runtime/pull/106193

dotnet / docs