[dotnet-linker] Use [DynamicDependency] attributes instead of manual marking when marking static registrar methods.#25018
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the dotnet-linker pipeline to preserve static-registrar-related delegate proxy Invoke methods by injecting [DynamicDependency] attributes, moving away from manual marking to support future removal of custom linker steps (per #17693).
Changes:
- Add
MarkForStaticRegistrarStep(anAssemblyModifierStep) that adds dynamic dependency attributes for delegate proxyInvokemethods. - Disable the existing
MarkForStaticRegistrarmark substep when the new step runs (via a flag inDerivedLinkContext). - Wire the new step into the MSBuild trimmer custom steps behind a new
_UseDynamicDependenciesForMarkStaticRegistrarproperty.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/dotnet-linker/MarkForStaticRegistrarStep.cs | New assembly-modifier step that injects dynamic dependency attributes to preserve delegate proxy Invoke methods for the static registrar. |
| tools/dotnet-linker/MarkForStaticRegistrar.cs | Skips the existing mark substep when the new pre-MarkStep custom step has run. |
| tools/common/DerivedLinkContext.cs | Adds a DidRunMarkForStaticRegistrarStep flag to coordinate between the two implementations. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Adds a new build property and enables the new step before MarkStep when dynamic dependencies are enabled. |
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.
d8b772e to
d7fde5c
Compare
…marking when marking static registrar methods. This makes it easier to move this code out of a custom linker step in the future. Contributes towards #17693.
653b289 to
4655a1a
Compare
|
Azure Pipelines: 2 pipeline(s) require an authorized user to comment /azp run to run. |
✅ [CI Build #4655a1a] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #4655a1a] Build passed (Detect API changes) ✅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 |
✅ [CI Build #4655a1a] Build passed (Build macOS tests) ✅Pipeline on Agent |
🚀 [CI Build #4655a1a] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 156 tests passed 🎉 Tests counts✅ cecil: 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 |
|
Azure Pipelines: 2 pipeline(s) require an authorized user to comment /azp run to run. |
This makes it easier to move this code out of a custom linker step in the future.
Contributes towards #17693.