Skip to content

Expose extension points needed by UnityLinker. #90688

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

Merged

Conversation

mrvoorhe
Copy link
Contributor

  • Expose stages of MarkStep for customization

  • Add back MethodsWithOverrideInformation

  • Expose TypeMapInfo and EnsureProcessed

  • Re-expose a way to pass in a different UnintializedContextFactory

  • Move AssemblyResolver creation to UnintializedContextFactory so that we can have control over the resolver again.

  • Make AssemblyResolver more extensible

  • We need to search winmd files.

  • We control over setting up the ReaderParameters

  • Expose some methods we need to override.

  • Make KeepMembersForDebugger a property so we can turn it on or off.

  • Make MarkAssembly virtual

  • Re-expose ShouldMarkInterfaceImplementation

@mrvoorhe mrvoorhe requested a review from marek-safar as a code owner August 16, 2023 19:17
@ghost ghost added area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework community-contribution Indicates that the PR has been added by a community member labels Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Expose stages of MarkStep for customization

  • Add back MethodsWithOverrideInformation

  • Expose TypeMapInfo and EnsureProcessed

  • Re-expose a way to pass in a different UnintializedContextFactory

  • Move AssemblyResolver creation to UnintializedContextFactory so that we can have control over the resolver again.

  • Make AssemblyResolver more extensible

  • We need to search winmd files.

  • We control over setting up the ReaderParameters

  • Expose some methods we need to override.

  • Make KeepMembersForDebugger a property so we can turn it on or off.

  • Make MarkAssembly virtual

  • Re-expose ShouldMarkInterfaceImplementation

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework, area-Tools-ILLink

Milestone: -

@mrvoorhe mrvoorhe force-pushed the linker-expose-extension-points branch from e162037 to 68b4b5a Compare August 16, 2023 19:19
@mrvoorhe
Copy link
Contributor Author

This is probably going to fail the ref api checks. I forget the command to update the suppressions, but I remember the job that fails tells you the command to run. If I get that failure I'll update the PR.

@vitek-karas
Copy link
Member

This is probably going to fail the ref api checks. I forget the command to update the suppressions, but I remember the job that fails tells you the command to run. If I get that failure I'll update the PR.

I don't know about the api checks, but there's an intentional difference between members marked as public and the surface exposed through the ref assemblies. I would expect this change to NOT modify the ref assemblies. The ref assemblies are solely for custom steps - basically an attempt to minimize the API surface exposed to custom steps (with an eventual goal of removing custom steps altogether).

@sbomer might know details about the ref assembly generation - if it does become a problem.

@sbomer
Copy link
Member

sbomer commented Aug 16, 2023

The api compat validation changed with the move to dotnet/runtime, and it now checks that public APIs in the implementation are also in the ref assembly. As @vitek-karas described, that's not what we want, so we use the suppressions.

The build failure is showing:

error : API breaking changes found. If those are intentional, the APICompat suppression file can be updated by rebuilding with '/p:ApiCompatGenerateSuppressionFile=true'

Last time you updated the suppressions it looked right to me. I assume this is how you did it.

@mrvoorhe mrvoorhe force-pushed the linker-expose-extension-points branch from 68b4b5a to 1b96b9b Compare August 17, 2023 16:10
@mrvoorhe
Copy link
Contributor Author

@vitek-karas @sbomer I think this PR is ready. There are failures but I don't think they are related.

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

* Expose stages of MarkStep for customization

* Add back `MethodsWithOverrideInformation`

* Expose `TypeMapInfo` and `EnsureProcessed`

* Re-expose a way to pass in a different `UnintializedContextFactory`

* Move AssemblyResolver creation to `UnintializedContextFactory` so that we can have control over the resolver again.

* Make AssemblyResolver more extensible

* We need to search winmd files.
* We control over setting up the ReaderParameters
* Expose some methods we need to override.

* Make `KeepMembersForDebugger` a property so we can turn it on or off.

* Make MarkAssembly virtual

* Re-expose `ShouldMarkInterfaceImplementation`
@mrvoorhe mrvoorhe force-pushed the linker-expose-extension-points branch from 1b96b9b to a432565 Compare August 22, 2023 20:44
@mrvoorhe
Copy link
Contributor Author

@vitek-karas This PR is ready to be merged.

@vitek-karas vitek-karas merged commit a3b1c63 into dotnet:main Aug 24, 2023
@vitek-karas
Copy link
Member

Thanks a lot @mrvoorhe

@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants