[msbuild/tools] Move post-trimming custom trimmer steps to a post-ILLink MSBuild task. by rolfbjarne · Pull Request #25742 · dotnet/macios

rolfbjarne · 2026-06-18T16:43:00Z

WIP WIP WIP.

Contributes towards #17693.

Copilot

Pull request overview

This PR is a WIP step toward replacing custom ILLink linker steps by moving the “post-trimming” work into an MSBuild task (assembly preparer mode), enabling a trimmer-agnostic pipeline (ILLink and future NativeAOT scenarios).

Changes:

Introduces an MSBuild-driven post-processing phase for prepared assemblies (PostProcessAssemblies + _PostprocessAssemblies target).
Refactors/extends assembly-preparer to load assemblies, run pre/post steps, and save results (new steps + expanded pipeline).
Adapts several linker steps to run both in the traditional linker pipeline and in assembly-preparer mode; adds a Windows remote test for assembly preparer.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
tools/linker/RemoveUserResourcesSubStep.cs	Updates the step to work in both linker and assembly-preparer pipelines.
tools/linker/MonoTouch.Tuner/ListExportedSymbols.cs	Adds ASSEMBLY_PREPARER support for the step.
tools/dotnet-linker/Steps/RegistrarStep.cs	Adjusts registrar filtering behavior for assembly-preparer mode.
tools/dotnet-linker/Steps/GenerateReferencesStep.cs	Switches to `StringUtils.IsNullOrEmpty` for consistency.
tools/dotnet-linker/Steps/ExtractBindingLibrariesStep.cs	Uses `PathUtils.AbsoluteToRelative` instead of `Path.GetRelativePath`.
tools/dotnet-linker/LinkerConfiguration.cs	Extends configuration for assembly-preparer mode (AssemblyInfos, etc).
tools/common/Application.cs	Adds an explicit `IsPostProcessingAssemblies` flag for assembly-preparer mode.
tools/assembly-preparer/SaveAssembliesStep.cs	New step to write assemblies after processing (and strip crossgen markers).
tools/assembly-preparer/LoadAssembliesStep.cs	New step to load assemblies + determine trimming action.
tools/assembly-preparer/ComputeMethodOverridesStep.cs	New step to precompute method overrides into annotations.
tools/assembly-preparer/CollectFieldsStep.cs	New step to collect exported field symbols (compat for InlineDlfcn).
tools/assembly-preparer/AssemblyPreparer.cs	Splits pipeline into Prepare vs PostProcess and runs step lists accordingly.
tools/assembly-preparer/assembly-preparer.csproj	Wires additional steps and dependencies into the assembly-preparer tool.
tests/dotnet/UnitTests/WindowsTest.cs	Adds a remote Windows test validating assembly-preparer behavior/artifacts.
msbuild/Xamarin.Shared/Xamarin.Shared.targets	Adds `PostProcessAssemblies` and introduces `_PostprocessAssemblies` target.
msbuild/Xamarin.MacDev.Tasks/Tasks/PrepareAssemblies.cs	Adds a post-processing mode invocation path for `AssemblyPreparer`.
dotnet/targets/Xamarin.Shared.Sdk.targets	Avoids running existing custom trimmer steps when post-processing is enabled.

rolfbjarne · 2026-06-25T07:54:20Z

/review

github-actions · 2026-06-25T07:55:29Z

❌ .NET for Apple Platforms PR Reviewer failed to deliver outputs. Please review the logs for details.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

 		public bool IsError (IToolLog? log)
 		{
-			if (!Warning)
-				return true;
-			if (log is null)
-				return false;
-			return ErrorHelper.GetWarningLevel (log, Code) == ErrorHelper.WarningLevel.Error;
+			return GetWarningLevel (log) == ErrorHelper.WarningLevel.Error;
 		}


+	<Target
+		Name="_PostprocessAssemblies"
+		Condition="'$(PrepareAssemblies)' == 'true' And '$(RuntimeIdentifiers)' == ''"
+		Inputs="@(_AssembliesToPostProcess)"
+		Outputs="@(_AssembliesToPostProcess->'$(_PostprocessedAssembliesDirectory)%(Filename)%(Extension)')"
+		BeforeTargets="_ComputeStripAssemblyIL;_LoadLinkerOutput"


 				var assemblyDefinition = assembly.Assembly;
 				if (assemblyDefinition is null) {
+					if (!assembly.IsCILAssembly) {
+						// Non-managed assembly, already handled by LoadAssembliesStep (OutputPath = InputPath).
+						continue;
+					}
 					exceptions.Add (ErrorHelper.CreateError (99, $"Assembly definition is null for {assembly.InputPath}"));
 					return;
 				}


github-actions

Review Summary

This PR refactors the build system to move post-trimming custom trimmer steps into a new PrepareAssemblies MSBuild task, introducing a PostProcessing mode. The changes span MSBuild targets, the assembly preparer tool, and various linker steps.

Issues Found

⚠️ 3 warnings — MSBuild syntax error, compilation errors in conditional branches, duplicate null check
💡 5 suggestions — Unused property, nullable pattern violation, performance improvement, code organization

Key Concerns

MSBuild parameter type mismatch (line 3539 in Xamarin.Shared.targets): Using $(_AssembliesToPrepare) instead of @(_AssembliesToPrepare) will pass a string instead of an item array
Conditional compilation issues in RemoveUserResourcesSubStep.cs: The App property is not available in the ASSEMBLY_PREPARER branch but is accessed without guards
Dead code: Unreachable null check in SaveAssembliesStep.cs and unused OriginalAssemblies property

Positive Notes

✅ Comprehensive refactoring with matching changes across MSBuild, linker steps, and tools
✅ Proper error handling additions (MSBStrings.E0192)
✅ Good separation of concerns between preparation and post-processing phases
✅ Test coverage added for the new functionality

Recommendation

The core architecture looks sound, but please address the MSBuild syntax error and conditional compilation issues before merging. The suggestion-level items can be addressed at your discretion.

Generated by .NET for Apple Platforms PR Reviewer for issue #25742 · 65.9 AIC · ⌖ 9.28 AIC · ⊞ 5K
_{Comment /review to run again}

github-actions · 2026-06-25T08:01:43Z

 		public ITaskItem? OptionsFile { get; set; }
 		#endregion

+		public ITaskItem [] OriginalAssemblies { get; set; } = [];


🤖 💡 Code organization — The OriginalAssemblies property is defined but never used in the code.

This appears to be dead code. Either:

Remove it if it's not needed, or

Document why it's defined but unused (e.g., if it's needed for future functionality or external tooling)

Rule: Code Organization — Remove unused code

github-actions · 2026-06-25T08:01:43Z

 				// * with and without the ".dylib" extension
 				var app = LinkerConfiguration.GetInstance (Context).Application;
-				var monoLibraryVariations = app.MonoLibraries.
+				var monoLibraryVariationsEnumerable = app.MonoLibraries.


🤖 💡 Performance — The LINQ chain is unnecessarily split across two statements. The intermediate monoLibraryVariationsEnumerable variable is only used once and doesn't improve readability.

The original pattern was more idiomatic (using .ToHashSet() at the end of the chain). However, if the change is for .NET compatibility reasons, consider consolidating:

var monoLibraryVariations = new HashSet<string> ( app.MonoLibraries. Where (v => v.EndsWith (".dylib", StringComparison.OrdinalIgnoreCase) || v.EndsWith (".a", StringComparison.OrdinalIgnoreCase)). Select (v => Path.GetFileNameWithoutExtension (v)). Select (v => v.StartsWith ("lib", StringComparison.OrdinalIgnoreCase) ? v.Substring (3) : v) );

Rule: Code organization — Reduce unnecessary intermediate variables

github-actions · 2026-06-25T08:01:43Z

+		bool ModifyAssembly (AssemblyDefinition assembly)
+#endif
+		{
+			if (App.Profile.IsProductAssembly (assembly) || App.Profile.IsSdkAssembly (assembly))


🤖 ⚠️ Error handling — Similar to the issue at line 66, App is accessed without the proper conditional compilation guard. In the #if ASSEMBLY_PREPARER branch, this should use Configuration.Application.Profile:

if (Configuration.Application.Profile.IsProductAssembly (assembly) || Configuration.Application.Profile.IsSdkAssembly (assembly)) return false;

Rule: Bugs & correctness — Compilation errors in conditional branches

github-actions · 2026-06-25T08:01:43Z

+			InputAssemblies="@(_AssembliesToPostProcess)"
+			MakeReproPath="$(_PostprocessAssembliesMakeReproPath)"
+			OptionsFile="$(_CustomLinkerOptionsFile)"
+			OriginalAssemblies="$(_AssembliesToPrepare)"


🤖 ⚠️ MSBuild tasks — The OriginalAssemblies parameter is set to $(_AssembliesToPrepare) but this references a property (string) instead of an item group. The parameter is declared as ITaskItem[] which expects an item group, not a property.

This should be:

OriginalAssemblies="@(_AssembliesToPrepare)"

Note the @() instead of $() to pass the item group correctly.

Rule: MSBuild conventions — Property vs Item Group syntax

github-actions · 2026-06-25T08:01:43Z


 				var assemblyDefinition = assembly.Assembly;
 				if (assemblyDefinition is null) {
+					if (!assembly.IsCILAssembly) {


🤖 ⚠️ Error handling — Duplicate check: !assembly.IsCILAssembly is checked again at line 30 after it was already checked at line 26.

The inner check on line 30 is unreachable and should be removed:

var assemblyDefinition = assembly.Assembly; if (assemblyDefinition is null) { exceptions.Add (ErrorHelper.CreateError (99, $"Assembly definition is null for {assembly.InputPath}")); return; }

Rule: Code organization — Remove dead code

github-actions · 2026-06-25T08:01:43Z

@@ -35,6 +36,10 @@ public class PrepareAssemblies : XamarinTask {
 		public ITaskItem? OptionsFile { get; set; }


🤖 💡 Code organization — The OptionsFile property has [Required] attribute but is nullable (ITaskItem?), which violates the MSBuild task nullable pattern.

Per the review rules: [Required] properties must be non-nullable with a default value (public ITaskItem OptionsFile { get; set; } = ...;). If this property can genuinely be null in some scenarios, either:

Remove the [Required] attribute and add null checks in the code, or

Provide a non-null default value

Rule: MSBuild Task Conventions — [Required] properties

github-actions · 2026-06-25T08:01:43Z

+				return xammac_prefixes;
 			default:
-				Report (ErrorHelper.CreateError (71, Errors.MX0071, LinkContext.App.Platform, LinkContext.App.ProductName));
+				Report (ErrorHelper.CreateError (71, Errors.MX0071, platform, App.ProductName));


🤖 ⚠️ Error handling — The App property is accessed without a null check or guard, but in the #if ASSEMBLY_PREPARER branch there is no App property defined (only in the #else branch at line 50).

This will cause a compilation error when ASSEMBLY_PREPARER is defined. The method GetPrefixes should use Configuration.Application instead:

Report (ErrorHelper.CreateError (71, Errors.MX0071, platform, Configuration.Application.ProductName));

Rule: Bugs & correctness — Compilation errors in conditional branches

…n configured for an exception.

Three #if guards in StaticRegistrar.cs excluded managed static registrar code from the ASSEMBLY_PREPARER build: 1. GenerateCallToUnmanagedCallersOnlyMethod call (line ~3967): Without this, the registrar always generated old-style token-based trampolines instead of calling the UnmanagedCallersOnly methods created by ManagedRegistrarStep. 2. GenerateCallToUnmanagedCallersOnlyMethod definition (line ~4197): The method itself was excluded. 3. TryCreateTokenReference managed-static branch (line ~5173): Without this, type lookups fell through to old-style metadata token creation, which fails at runtime because the managed static registrar can't resolve method tokens (error MX8054). Changed all three guards from '#if !LEGACY_TOOLS && !ASSEMBLY_PREPARER' to '#if !LEGACY_TOOLS'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace the hardcoded error string with MSBStrings.E0192, following the standard pattern for MSBuild task error reporting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address review comment: skip copy when target is already up-to-date (timestamp comparison), and also copy .config files alongside assemblies and PDBs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Log when assemblies are copied and when they're skipped (up-to-date). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The PrepareAssemblies PostProcess pass modifies the (post-trimmed) assemblies, so those modifications must end up in the AOT/R2R-compiled output. For CoreCLR the ReadyToRun compilation runs inside 'ComputeFilesToPublish' (via 'CreateReadyToRunImages'), which happened *before* '_PostprocessAssemblies' (hooked on '_ComputeStripAssemblyIL'/'_LoadLinkerOutput'). As a result crossgen ran first and stubbed the method IL bodies to 'throw null' (the real code lives in the R2R native image), and then '_PostprocessAssemblies' rewrote those assemblies with Cecil, discarding the native code but keeping the stubs. At runtime the '<Module>' cctor (managed-static registration) executed the stub and threw a NullReferenceException, surfacing at startup as 'xamarin_bridge_call_runtime_initialize: failed to create delegate' / TypeLoadException. Hook '_PrepareForReadyToRunCompilation' (which computes the list of assemblies to crossgen) so the post-processed assemblies are the ones that get compiled. The existing '_ComputeStripAssemblyIL'/'_LoadLinkerOutput' hooks are kept for Mono, where the AOT compiler runs later (in '_CompileNativeExecutable'). Fixes 'make clean build run-bare -C "tests/linker/dont link/dotnet/MacCatalyst" TEST_VARIATION=prepare-assemblies|release|coreclr CONFIG=Release'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

vs-mobiletools-engineering-service2 · 2026-06-26T16:00:31Z

🔥 [PR Build #3edc3ee] Build failed (Detect API changes) 🔥

Build failed for the job 'Detect API changes' (with job status 'Failed')

Pipeline on Agent
Hash: 3edc3ee885e4dd57d216512db139e3d83f8d77ee [PR build]

vs-mobiletools-engineering-service2 · 2026-06-26T16:03:52Z

🔥 Unable to find the contents for the comment: D:\a\1\s\change-detection\results\gh-comment.md does not exist :fire

Pipeline on Agent
Hash: 3edc3ee885e4dd57d216512db139e3d83f8d77ee [PR build]

vs-mobiletools-engineering-service2 · 2026-06-26T16:06:51Z

✅ [PR Build #3edc3ee] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 3edc3ee885e4dd57d216512db139e3d83f8d77ee [PR build]

vs-mobiletools-engineering-service2 · 2026-06-26T16:31:59Z

✅ [PR Build #3edc3ee] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 3edc3ee885e4dd57d216512db139e3d83f8d77ee [PR build]

vs-mobiletools-engineering-service2 · 2026-06-26T17:56:14Z

🚀 [CI Build #3edc3ee] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 199 tests passed 🎉

Tests counts

✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker (iOS): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (MacCatalyst): All 15 tests passed. Html Report (VSDrops) Download
✅ linker (macOS): All 21 tests passed. Html Report (VSDrops) Download
✅ linker (tvOS): All 15 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 17 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 18 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

✅ Linux build succeeded

Pipeline on Agent
Hash: 3edc3ee885e4dd57d216512db139e3d83f8d77ee [PR build]

rolfbjarne requested a review from Copilot June 18, 2026 16:43

rolfbjarne added the copilot label Jun 18, 2026

Copilot started reviewing on behalf of rolfbjarne June 18, 2026 16:43 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

rolfbjarne force-pushed the dev/rolf/assembly-postparer branch from 723af88 to 35a2cb4 Compare June 22, 2026 17:05

This comment has been minimized.

Sign in to view

rolfbjarne force-pushed the dev/rolf/assembly-postparer branch from 35a2cb4 to 93d6db3 Compare June 23, 2026 17:19

This comment has been minimized.

Sign in to view

rolfbjarne requested a review from Copilot June 25, 2026 07:54

Copilot started reviewing on behalf of rolfbjarne June 25, 2026 07:54 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

github-actions Bot reviewed Jun 25, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rolfbjarne and others added 9 commits June 26, 2026 17:48

[assembly-preparer] Did I forget about you?

9797aea

[tools] Honor the 'ProductException.Warning' field if nothing has bee…

6293f03

…n configured for an exception.

Add remoting test, this has been failing often.

499364e

[tools] Post-processor

28c2419

Use proper error code for PrepareAssemblies fallback error

6025839

Replace the hardcoded error string with MSBStrings.E0192, following the standard pattern for MSBuild task error reporting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Improve assembly copy in SaveAssembliesStep

fab5b43

Address review comment: skip copy when target is already up-to-date (timestamp comparison), and also copy .config files alongside assemblies and PDBs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add logging to SaveAssembliesStep copy operations

3781d5f

Log when assemblies are copied and when they're skipped (up-to-date). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rolfbjarne force-pushed the dev/rolf/assembly-postparer branch from ad43321 to 3edc3ee Compare June 26, 2026 15:49

This comment has been minimized.

Sign in to view

		@@ -35,6 +36,10 @@ public class PrepareAssemblies : XamarinTask {
		public ITaskItem? OptionsFile { get; set; }

Uh oh!

Conversation

rolfbjarne commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne commented Jun 25, 2026

Uh oh!

github-actions Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review Summary

Issues Found

Key Concerns

Positive Notes

Recommendation

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

github-actions Bot commented Jun 25, 2026 •

edited

Loading