Repository navigation
[Hot Reload] Add assembly modification safety net. Fixes #26072 - #26417
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a Hot Reload compatibility “safety net” to prevent reloadable (Copy) assemblies from being re-saved during HotReloadCompatibleBuild builds, and updates build defaults/documentation so NativeAOT builds don’t opt into this mode by default.
Changes:
- Add a guard in
AppBundleRewriter.SaveAssemblythat reports an error and skips saving when HotReloadCompatibleBuild is enabled for a reloadable (Copy) assembly. - Default
$(HotReloadCompatibleBuild)to Debug for non-NativeAOT builds, and tofalsefor NativeAOT. - Update build-property documentation to reflect the NativeAOT default behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/dotnet-linker/AppBundleRewriter.cs | Adds the enforcement check that blocks saving reloadable assemblies during Hot Reload compatible builds. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Adjusts MSBuild defaults so NativeAOT doesn’t enable HotReloadCompatibleBuild by default. |
| docs/building-apps/build-properties.md | Documents the updated default (non-NativeAOT Debug => true; otherwise false). |
Suppressed comments (2)
tools/dotnet-linker/AppBundleRewriter.cs:1507
- 🤖 💡 Error message — The error text says the assembly “was modified”, but this guard is actually about preventing any reloadable assembly from being saved/re-serialized (which can happen even for metadata-preservation reasons). Consider wording that explains the byte-for-byte constraint and points to $(HotReloadCompatibleBuild) as the controlling switch.
configuration.Logger.LogError (ErrorHelper.CreateError (99, $"The assembly '{assembly.Name.Name}' is reloadable, but was modified during a Hot Reload compatible build."));
tools/dotnet-linker/AppBundleRewriter.cs:1509
- 🤖 💡 Testing — This new safety-net behavior (failing when a reloadable/Copy assembly would be saved under HotReloadCompatibleBuild) doesn’t appear to have a regression test. Given there’s already a dedicated assembly-preparer test harness (tests/assembly-preparer/BaseClass.cs), it would be good to add a focused test that exercises this specific failure mode and asserts on the emitted error (and code, once it’s dedicated).
if (configuration.HotReloadCompatibleBuild && action == AssemblyAction.Copy && assembly != PlatformAssembly) {
configuration.Logger.LogError (ErrorHelper.CreateError (99, $"The assembly '{assembly.Name.Name}' is reloadable, but was modified during a Hot Reload compatible build."));
return;
}
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.
…o reloadable assemblies
PreserveSmartEnumConversionsStep injects [DynamicDependency] attributes into the
assembly that references a smart enum. That assembly may be reloadable (i.e. not
trimmed), in which case the injection breaks Hot Reload - and it now trips the
newly added safety net (MT0099), which broke the 'link sdk' test build:
ILLINK error MT0099: The assembly 'bindings-test' is reloadable, but was
modified during a Hot Reload compatible build.
Use the mark handler (PreserveSmartEnumConversionsHandler) instead when doing a
Hot Reload compatible build without the assembly-preparer: it marks the
conversion methods when the referencing method is marked, without modifying any
assemblies (and without any app size cost). When the assembly-preparer is used,
the step already emits a root descriptor xml file instead, so nothing changes
there.
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.
…static registrar
The managed static registrar emits registrar code into every user assembly, which
breaks Hot Reload (and trips the recently added safety net, which broke the
'dont link' test build):
MSBuild : error MT99: The assembly 'dont link' is reloadable, but was modified
during a Hot Reload compatible build.
Hot Reload can't work in that case at all (unlike with the trimmable static
registrar, which emits this code into separate companion assemblies), so default
$(HotReloadCompatibleBuild) to false when the managed static registrar is used (a
user-specified value still wins, as before).
This means the property can't be computed at evaluation time anymore, because the
registrar isn't known until the SelectRegistrar target has run, so compute it in a
new _ComputeHotReloadCompatibleBuild target instead, and make the consumers depend
on that target:
* _ComputeLinkerInputs in the SDK (it computes
$(_UseDynamicDependenciesForSmartEnumPreservation) and the linker's
HotReloadCompatibleBuild option from it).
* _AddHotReloadCompatibleBuildDefine in the tests (it defines
HOTRELOAD_COMPATIBLE_BUILD). The condition had to be moved from the target to
the property group, because a target's condition is evaluated before its
dependencies are built.
Additionally, SelectRegistrar only runs for projects that produce an app bundle,
so the referenced test library projects would keep the (potentially different)
default, even though it's the app's value that decides whether their assemblies
are modified. So pass the app's value on to every referenced project.
Also update the documentation for $(HotReloadCompatibleBuild) accordingly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…afety-net # Conflicts: # dotnet/targets/Xamarin.Shared.Sdk.targets # tests/common/shared-dotnet.csproj
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.
Constructor factory methods modify their containing assembly, so skip generating them for copied assemblies in Hot Reload compatible builds. Add coverage that verifies a reloadable assembly remains unchanged. 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.
Only treat ProjectReference items with an Include attribute as project references. This allows cloned projects to retain valid Update items without crashing XHarness project traversal. 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.
Grant relocated callbacks access to helper assemblies instead of relying on visibility rewrites, and construct generic wrappers through reflection when reloadable assemblies cannot contain injected factories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Gate the generic constructor reflection fallbacks behind the HotReloadCompatible trimmer feature switch so non-Hot-Reload builds remove them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0bb3455e-b9bf-452b-80f7-53e6b3079583
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.
HotReloadCompatibleBuild propagation gives transitive project references multiple MSBuild configurations that still share their output paths. Parallel project-reference builds can consequently compile MonoTouch.Dialog twice into the same DLL or PDB, causing intermittent MSB3026 copy retries. Disable parallel project-reference builds for BundleStructure so the first configuration finishes before the equivalent configuration is checked and skipped as up-to-date. This prevents the conflicting writers without hiding successful-copy retry warnings globally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2bdd0ea7-c3c5-45ed-97c0-7ca42d9f5b05
This comment has been minimized.
This comment has been minimized.
… net Add an undocumented $(_HotReloadModifiedAssemblySafetyNet) MSBuild property (default: true) that can be set to false to disable the error raised when a reloadable assembly is modified during a Hot Reload compatible build. This is only meant as an escape hatch in case the safety net itself produces a false positive. 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.
rolfbjarne
added
ready-to-review
Ensure the trimming state is initialized before Hot Reload compatibility selects the registrar. This prevents early target execution from caching assemblies as untrimmed and disabling export attribute removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c2a5ee38-078a-480d-b532-f862155b3fd9
✅ 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 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #6bfada4] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 264 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Hot Reload requires that user (reloadable) assemblies stay byte-for-byte unchanged, but several linker steps happily re-serialize them. This PR adds a safety net that catches those cases at build time, and fixes all the violations the safety net found.
The safety net
AppBundleRewriter.SaveAssemblynow reports a normal linker error (MT0099) when a Hot Reload compatible build is about to save an assembly whose linker action isCopy(i.e. a reloadable, non-trimmed user assembly). The platform assembly is exempt, since it's never reloadable.The safety net is on by default, and can be turned off with the undocumented
$(_HotReloadModifiedAssemblySafetyNet)MSBuild property. That escape hatch exists only in case the safety net itself has a false positive - it's deliberately not a public knob.$(HotReloadCompatibleBuild)defaultsThe default computation moved out of the static property block into a new
_ComputeHotReloadCompatibleBuildtarget, so it can depend on_ComputePublishTrimmedandSelectRegistrar:falsefalse$(_BundlerDebug)The managed static registrar modifies user assemblies by design, so it can never be Hot Reload compatible, and NativeAOT doesn't support Hot Reload at all. The build-property documentation was updated accordingly.
Fixes for what the safety net caught
[DynamicDependency]injection:PreserveSmartEnumConversionsStepinjects attributes into the assembly that references the smart enum, which may be reloadable. For Hot Reload compatible builds we now use the mark handler instead (it doesn't modify assemblies). This doesn't apply when the assembly-preparer is used, since it emits a root descriptor XML file instead.Link.EnsureVisible: instead of flipping accessibility flags on members in reloadable assemblies, the accessed assembly is now recorded on the registrar companion assembly (which is where the relocated trampolines live anyway).RegistrarCompanionAssembly.GetNameis now shared betweenManagedRegistrarStepandTrimmableRegistrarStep, and escapes assembly names containing a comma (a comma starts the qualifier section of an assembly display name, so it would otherwise produce an unparseable type map entry).Runtime feature switch
A new
ObjCRuntime.Runtime.HotReloadCompatiblefeature switch is emitted as aRuntimeHostConfigurationOptionand surfaced asRuntime.HotReloadCompatible(with[FeatureSwitchDefinition], so it's trimmed away in non-Hot-Reload builds).It's used in
ConstructNSObject<T>andConstructINativeObject<T>: when the static interface factory method isn't available (which happens for generic/reloadable types in a Hot Reload compatible build), we now fall back to reflection to find theIntPtr/NativeHandle(andbool owns) constructor rather than throwing. Ownership is still handled correctly for theNSObjectpath.Test infrastructure
HotReloadCompatibleTestin the .NET unit tests, plus fixes for existing Hot Reload compatibility test regressions.ProjectReferencemetadata updates, andBundleStructureproject references are serialized to avoid races.Fixes #26072
🤖 Pull request created by Copilot