[msbuild/tools] Move post-trimming custom trimmer steps to a post-ILLink MSBuild task.#25742
[msbuild/tools] Move post-trimming custom trimmer steps to a post-ILLink MSBuild task.#25742rolfbjarne wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
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+_PostprocessAssembliestarget). - 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. |
| @@ -44,6 +54,7 @@ protected override void EndProcess () | |||
| base.EndProcess (); | |||
| } | |||
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #8a3a11e] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 61 tests failed, 159 tests passed. Failures❌ dotnettests tests (iOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (Multiple platforms)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (tvOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ generator tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ interdependent-binding-projects tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (iOS)5 tests failed, 10 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (MacCatalyst)4 tests failed, 11 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (macOS)10 tests failed, 11 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (tvOS)4 tests failed, 11 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)21 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)1 tests failed, 23 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)2 tests failed, 22 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)1 tests failed, 20 tests passed.Failed tests
Html Report (VSDrops) Download ❌ windows tests3 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ xcframework tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) [Download](https://devdiv.visualstudio.com/DevDiv/_apis/build/builds/14417896/artif\n\nThe message from CI is too large for the GitHub comments. You can find the full results here. |
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
force-pushed
the
dev/rolf/assembly-postparer
branch
from
723af88 to
35a2cb4
Compare
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
force-pushed
the
dev/rolf/assembly-postparer
branch
from
35a2cb4 to
93d6db3
Compare
This comment has been minimized.
This comment has been minimized.
rolfbjarne
force-pushed
the
dev/rolf/assembly-postparer
branch
from
967268c to
9fbe861
Compare
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.
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.
| File.Copy (assembly.InputPath, assembly.OutputPath, true); | ||
| var pdb = Path.ChangeExtension (assembly.InputPath, ".pdb"); | ||
| if (File.Exists (pdb)) | ||
| File.Copy (pdb, Path.ChangeExtension (assembly.OutputPath, ".pdb"), true); |
There was a problem hiding this comment.
Don't copy if the target location is already up-to-date.
Also needs to copy .config files.
We used to have a CopyAssembly method that did this:
macios/tools/common/Assembly.cs
Lines 729 to 775 in 1a2b3e3
…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>
rolfbjarne
force-pushed
the
dev/rolf/assembly-postparer
branch
from
418f3e7 to
ad43321
Compare
✅ [PR Build #ad43321] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #ad43321] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #ad43321] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #ad43321] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 41 tests failed, 166 tests passed. Failures❌ monotouch tests (iOS)20 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)20 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
WIP WIP WIP.
Contributes towards #17693.