[Hot Reload] Add assembly modification safety net. Fixes #26072 - #26417
rolfbjarne wants to merge 25 commits into
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.
|
Only run the shared test-project Hot Reload propagation targets for .NET 11 and newer, where the workload defines the computation target. Set the registrar using the Registrar property for the P/Invoke wrapper generator test so that Hot Reload compatibility is determined correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…afety-net # Conflicts: # tests/assembly-preparer/ManagedRegistrarStepTests.cs
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Per-assembly type map companions previously embedded the user assembly name verbatim. A comma was interpreted as the start of assembly display-name qualifiers, so ILLink could not resolve the companion and then modified the root type map as if it referenced a missing assembly. Encode ambiguous names into a collision-safe hexadecimal form and use the resulting identity consistently when creating, referencing, and locating companion assemblies. 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.
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 |
🔥 [CI Build #6bfada4] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 263 tests passed. Failures❌ monotouch tests (iOS)1 tests failed, 24 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ 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 |
Add the HotReloadCompatibleBuild MSBuild default and enforce that reloadable user assemblies are not modified during compatible builds.
The safety net reports a normal linker error when a Copy assembly would be saved. NativeAOT builds default HotReloadCompatibleBuild to false, and the build-property documentation now describes the behavior.
Fixes #26072
🤖 Pull request created by Copilot