Skip to content

[illink] refactor code sharing between ILLink and MSBuild tasks #9688

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
merged 11 commits into from
Feb 6, 2025

Conversation

jonathanpeppers
Copy link
Member

We already have a few trimmer steps that run under both ILLink and the <LinkAssembliesNoShrink/> MSBuild task:

  • AddKeepAlivesStep
  • FixAbstractMethodsStep
  • FixLegacyResourceDesignerStep

We want to move various parts of the build to become new trimmer steps like above.

Previously, we also had to share these steps with the Xamarin.Android linker pipeline, but that's no longer the case. We can now cleanup how these trimmer steps are shared between ILLink and MSBuild, reducing the need for #if ILLINK in many places.

I also removed various Mono linker source code that is unused.

We already have a few trimmer steps that run under both ILLink and the
`<LinkAssembliesNoShrink/>` MSBuild task:

* `AddKeepAlivesStep`
* `FixAbstractMethodsStep`
* `FixLegacyResourceDesignerStep`

We want to move various parts of the build to become new trimmer steps
like above.

Previously, we also had to share these steps with the Xamarin.Android
linker pipeline, but that's no longer the case. We can now cleanup how
these trimmer steps are shared between ILLink and MSBuild, reducing
the need for `#if ILLINK` in many places.

I also removed various Mono linker source code that is unused.
This reverts commit 90dfd12.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review February 4, 2025 15:44
Comment on lines -171 to -197
class FixLegacyResourceDesignerStep : MonoDroid.Tuner.FixLegacyResourceDesignerStep
{
readonly DirectoryAssemblyResolver resolver;
readonly TaskLoggingHelper logger;

public FixLegacyResourceDesignerStep (DirectoryAssemblyResolver resolver, TypeDefinitionCache cache, TaskLoggingHelper logger)
: base(cache)
{
this.resolver = resolver;
this.logger = logger;
}

public override void LogMessage (string message)
{
logger.LogDebugMessage ("{0}", message);
}

public override void LogError (int code, string message)
{
logger.LogCodedError ($"XA{code}", message);
}

public override AssemblyDefinition Resolve (AssemblyNameReference name)
{
return resolver.Resolve (name);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Main benefit is deleting all these.

Comment on lines -19 to -33
#if ILLINK
protected override void Process ()
{
cache = Context;
}
#else // !ILLINK
public AddKeepAlivesStep (IMetadataResolver cache)
{
this.cache = cache;
}

readonly
#endif // !ILLINK
IMetadataResolver cache;

Copy link
Member Author

Choose a reason for hiding this comment

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

And then removing stuff like this in every shared step.

Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

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

This looks ok. As long as the tests pass I'm cool 😎

@jonathanpeppers jonathanpeppers merged commit d3bbc0c into main Feb 6, 2025
58 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/peppers/trimmer/refactor branch February 6, 2025 15:03
grendello added a commit that referenced this pull request Feb 6, 2025
* main:
  [illink] refactor code sharing between ILLink and MSBuild tasks (#9688)
@github-actions github-actions bot locked and limited conversation to collaborators Mar 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants