[assembly-preparer] Use a trimmer feature switch for DynamicRegistrationSupported. - #25939
Conversation
…ionSupported. When running as a step in the assembly-preparer, RegistrarRemovalTrackingStep used to rewrite Runtime.get_DynamicRegistrationSupported in the platform assembly (to return 'false') when it determined the dynamic registrar could be removed. That required the assembly-preparer to modify - and thus re-save - the platform assembly. Instead, compute the value and surface it to MSBuild, which enables the 'ObjCRuntime.Runtime.DynamicRegistrationSupported' trimmer feature switch so that ILLink hardcodes the property (the same way it already does for e.g. ObjCRuntime.Runtime.IsManagedStaticRegistrar). This way the assembly-preparer doesn't have to touch any assembly for this optimization. How it works: * Added get_DynamicRegistrationSupported substitution entries (feature 'ObjCRuntime.Runtime.DynamicRegistrationSupported') to the ILLink.Substitutions.*.xml for all four platforms. * RegistrarRemovalTrackingStep still computes Optimizations.RemoveDynamicRegistrar, but under ASSEMBLY_PREPARER it no longer rewrites the assembly - it reports the value via a new generic "MSBuild output property" mechanism instead. The real ILLink path (when PrepareAssemblies isn't enabled) is unchanged and still rewrites the assembly. Generic MSBuild output mechanism: * LinkerConfiguration.SetOutputForMSBuild (name, value) collects output properties, and FlushOutputForMSBuild writes them - alphabetically sorted, one 'Name=Value' per line - to a file. There's a separate file for the preparation and post-processing passes (MSBuildOutputFile / MSBuildPostProcessOutputFile linker options, picked via Application.IsPostProcessingAssemblies) so the two passes don't clobber each other. * The files are added to FileWrites (when they exist after the assembly-preparer has run). * The _SetDynamicRegistrationSupportedFeature target (which runs after _PrepareAssemblies and before the trimmer) parses the 'DynamicRegistrationSupported' property out of the preparation pass's output file and adds the RuntimeHostConfigurationOption. It's not incremental-gated and the file is persisted, so the feature switch is still set correctly when _PrepareAssemblies is skipped because its outputs are up-to-date. User override: * Added a user-overridable $(DynamicRegistrationSupported) MSBuild property (documented in build-properties.md). It's passed to the assembly-preparer via the DynamicRegistrationSupported linker option (mapped to the RemoveDynamicRegistrar optimization), so the user value always wins in the _SetDynamicRegistrationSupportedFeature target. * When the user sets $(DynamicRegistrationSupported), the value doesn't need to be computed, so RegistrarRemovalTrackingStep is skipped entirely in the assembly-preparer's preparation pass. Verified by building a test app (managed-static, link SdkOnly, PrepareAssemblies=true) in three configurations: default (the step runs and writes DynamicRegistrationSupported=false, ILLink gets '--feature ObjCRuntime.Runtime.DynamicRegistrationSupported false'), DynamicRegistrationSupported=true and DynamicRegistrationSupported=false (the step is skipped - empty output file - and ILLink gets the user value). Added a DynamicRegistrationSupportedTest build test asserting the user value flows through to the feature switch. monotouch-test (release|linksdk, PrepareAssemblies=true PostProcessAssemblies=true) passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
…uild.
FlushOutputForMSBuild unconditionally called Directory.CreateDirectory (ItemsDirectory),
but ItemsDirectory isn't set when running in the assembly-preparer (e.g. from the
assembly-preparer tests), which made every test throw:
System.ArgumentException : The value cannot be an empty string. (Parameter 'path')
Guard the item-file writing behind a non-empty ItemsDirectory check, the same way the
MSBuild output-property file writing already guards its directory.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
There was a problem hiding this comment.
Pull request overview
This PR updates the assembly-preparer + MSBuild pipeline so the “dynamic registrar can be removed” decision is communicated via an ILLink feature switch (ObjCRuntime.Runtime.DynamicRegistrationSupported) instead of rewriting the platform assembly during the preparation pass. This keeps the assembly-preparer from modifying/resaving the platform assembly while still enabling ILLink to hardcode Runtime.DynamicRegistrationSupported for trimming.
Changes:
- Add
ObjCRuntime.Runtime.DynamicRegistrationSupportedsubstitutions to all platformILLink.Substitutions.*.xmlfiles. - Extend
LinkerConfigurationwith a generic “MSBuild output properties” file mechanism and wireRegistrarRemovalTrackingStep(assembly-preparer path) to emitDynamicRegistrationSupported=true|false. - Add MSBuild plumbing to read the assembly-preparer output and set the corresponding
RuntimeHostConfigurationOption, plus a build test and documented MSBuild property.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/linker/RegistrarRemovalTrackingStep.cs | In assembly-preparer builds, stop rewriting the platform assembly; instead emit an MSBuild output property for DynamicRegistrationSupported. |
| tools/dotnet-linker/LinkerConfiguration.cs | Add MSBuild output property collection + output files (per pass), and parse user-provided DynamicRegistrationSupported. |
| tools/assembly-preparer/AssemblyPreparer.cs | Conditionally skip RegistrarRemovalTrackingStep when the user explicitly sets $(DynamicRegistrationSupported); ensure prep pass flushes MSBuild outputs. |
| tests/dotnet/UnitTests/DynamicRegistrationSupportedTest.cs | New unit test asserting a user-provided $(DynamicRegistrationSupported) flows to the trimmer feature switch item. |
| src/ILLink.Substitutions.iOS.xml | Add substitutions for get_DynamicRegistrationSupported() keyed off the new feature switch. |
| src/ILLink.Substitutions.tvOS.xml | Same substitution additions for tvOS. |
| src/ILLink.Substitutions.macOS.xml | Same substitution additions for macOS. |
| src/ILLink.Substitutions.MacCatalyst.xml | Same substitution additions for Mac Catalyst. |
| msbuild/Xamarin.Shared/Xamarin.Shared.targets | Track the assembly-preparer MSBuild output file as an output and add it to FileWrites for the preparation pass (and FileWrites for postprocess). |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Add _SetDynamicRegistrationSupportedFeature target to read the assembly-preparer output file and set RuntimeHostConfigurationOption; plumb output file paths into linker args. |
| docs/building-apps/build-properties.md | Document new user-overridable DynamicRegistrationSupported build property. |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #752c69d] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #752c69d] 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 #752c69d] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #752c69d] 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 macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
PR #25939 fixed the incremental-build failure (issue #25938) for the `_PrepareAssemblies` target by adding `$(_AssemblyPreparerMSBuildOutputFile)` to its `Outputs` (breaking the 1:1 `Inputs`/`Outputs` correspondence, so MSBuild rebuilds the target all-or-nothing instead of partially). The exact same problem still affects `_PostprocessAssemblies`, which kept its 1:1 `Inputs`/`Outputs` transform: on an incremental (second, no-change) build with `PrepareAssemblies=true`, `MtouchLink=SdkOnly` and `Registrar=managed-static`, MSBuild runs `_PostprocessAssemblies` partially and passes only the out-of-date subset of assemblies to the assembly-preparer, which then fails to resolve inter-assembly references (MT2362 "Could not find Microsoft.iOS in the list of registered assemblies"). Add `$(_AssemblyPostProcessorMSBuildOutputFile)` (which the assembly-preparer already always writes for the post-processing pass) to the `_PostprocessAssemblies` `Outputs`, mirroring the `_PrepareAssemblies` fix from #25939. This makes the target rebuild all-or-nothing. Also add a second variation to the PrepareAssembliesTest regression test (dynamic registrar / no linking / Debug) to cover more configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#25938 (#25943) An incremental (second, no-source-change) build with `PrepareAssemblies=true` could fail with `MT2362` ("Could not find <platform> in the list of registered assemblies"), even though the initial clean build succeeded. Root cause: the `_PrepareAssemblies` and `_PostprocessAssemblies` targets used 1:1 `Inputs`/`Outputs` item transforms, which let MSBuild run them as partial incremental builds. In partial mode MSBuild passes only the out-of-date subset of assemblies to the `PrepareAssemblies` task, but the assembly-preparer needs the complete assembly closure to resolve inter-assembly references, so resolution fails. `_PrepareAssemblies` was already fixed as a side effect of #25939, which added `$(_AssemblyPreparerMSBuildOutputFile)` to its `Outputs` (breaking the 1:1 correspondence, so MSBuild rebuilds the target all-or-nothing). This PR applies the same fix to `_PostprocessAssemblies`, which still ran partially: add `$(_AssemblyPostProcessorMSBuildOutputFile)` (which the assembly-preparer already always writes for the post-processing pass) to its `Outputs`. Also add a `PrepareAssembliesTest` regression test that builds MySimpleApp twice (no changes in between) under two configurations: - managed-static / SdkOnly / Release (the original repro) - dynamic / None / Debug Fixes #25938 🤖 Pull request created by Copilot --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When running as a step in the assembly-preparer,
RegistrarRemovalTrackingStepused torewrite
Runtime.get_DynamicRegistrationSupportedin the platform assembly (to returnfalse) when it determined the dynamic registrar could be removed. That required theassembly-preparer to modify - and thus re-save - the platform assembly.
Instead, compute the value and surface it to MSBuild, which enables the
ObjCRuntime.Runtime.DynamicRegistrationSupportedtrimmer feature switch so that ILLinkhardcodes the property (the same way it already does for e.g.
ObjCRuntime.Runtime.IsManagedStaticRegistrar). This way the assembly-preparer doesn'thave to touch any assembly for this optimization.
How it works
get_DynamicRegistrationSupportedsubstitution entries (featureObjCRuntime.Runtime.DynamicRegistrationSupported) to theILLink.Substitutions.*.xmlfor all four platforms.
RegistrarRemovalTrackingStepstill computesOptimizations.RemoveDynamicRegistrar, butunder
ASSEMBLY_PREPARERit no longer rewrites the assembly - it reports the value via anew generic "MSBuild output property" mechanism instead. The real ILLink path (when
PrepareAssembliesisn't enabled) is unchanged and still rewrites the assembly.Generic MSBuild output mechanism
LinkerConfiguration.SetOutputForMSBuild (name, value)collects output properties, andFlushOutputForMSBuildwrites them - alphabetically sorted, oneName=Valueper line -to a file. There's a separate file for the preparation and post-processing passes so the
two passes don't clobber each other.
FileWrites(when they exist after the assembly-preparer has run)._SetDynamicRegistrationSupportedFeaturetarget (which runs after_PrepareAssembliesand before the trimmer) parses the
DynamicRegistrationSupportedproperty out of thepreparation pass's output file and adds the
RuntimeHostConfigurationOption. It's notincremental-gated and the file is persisted, so the feature switch is still set correctly
when
_PrepareAssembliesis skipped because its outputs are up-to-date.User override
$(DynamicRegistrationSupported)MSBuild property (documented inbuild-properties.md); the user value always wins.$(DynamicRegistrationSupported), the value doesn't need to be computed,so
RegistrarRemovalTrackingStepis skipped entirely in the preparation pass.Testing
Added a
DynamicRegistrationSupportedTestbuild test asserting the user value flows throughto the feature switch. Also verified manually (default + explicit true/false) and with
monotouch-test (
release|linksdk,PrepareAssemblies=true PostProcessAssemblies=true).Note
While working on this I ran into a pre-existing incremental-build failure with
PrepareAssemblies=true(unrelated to this change); filed as #25938.🤖 Pull request created by Copilot