[targets] Improve/fix post-processing item collection for dylibs and frameworks#24721
[targets] Improve/fix post-processing item collection for dylibs and frameworks#24721rolfbjarne wants to merge 8 commits intomainfrom
Conversation
…frameworks Fix several issues in the _CollectItemsForPostProcessing target: * Fix broken MSBuild syntax: '$(AppBundleExtension/' was missing the closing ')' for the property expression. * Fix broken condition: '%(_ResolvedNativeReference.Kind' was missing the closing ')'. * Use unqualified %(Kind) metadata in conditions, because %(ItemType.Metadata) doesn't work in Condition attributes on items created with @(Item->'...') transforms. * Change dylib source from _ResolvedNativeReference (which only contains frameworks) to _FileNativeReference (which contains dylibs). * Add _AppContentsRelativePathForPostProcessing property with trailing '/' separator to fix path concatenation for macOS (where _AppContentsRelativePath has no trailing separator). * Set DSymName directly on framework and dylib items to avoid MSB4096 batching error when items without Kind metadata (the executable) are in the same item group. * Add tests for dylib and framework post-processing items on iOS and macOS. Improvements: * Make it easier to plug into this mechanism for other code to add items for post processing. * Fix an issue where dynamic native references were collected as if they were frameworks. * Simplify some defaults by computing them globally. * Add tests.
Move AssertExpectedDSyms to TestBaseClass for reuse across test classes. Add dSYM verification to DylibPostProcessingItems and FrameworkPostProcessingItems tests: since these are Debug builds, verify that no dSYM directories are generated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The BundleStructure project includes Framework.With.Dots.framework, which causes dsymutil to fail because the framework binary name is truncated. This prevents dSYMs from being generated for all items in the app bundle. Add an ignored test to make it easy to reproduce the problem. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes and refactors the _CollectItemsForPostProcessing MSBuild target used to collect binaries (frameworks/dylibs/executable) for dsymutil + stripping, and adds unit tests to validate the collected items and dSYM outputs across iOS and macOS.
Changes:
- Refactors
_CollectItemsForPostProcessingdependencies and item collection to correctly handle frameworks vs dylibs and fix prior MSBuild syntax/condition issues. - Adds more robust dSYM assertions (bundle-wide) and new binlog-based tests validating
_PostProcessingItementries for dylibs/frameworks. - Extends app size tests to validate expected dSYM directories in the build output.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/dotnet/UnitTests/TestBaseClass.cs | Adds AssertExpectedDSyms helper to validate dSYM directories expected from bundle contents. |
| tests/dotnet/UnitTests/PostBuildTest.cs | Adds binlog-based tests verifying post-processing items for dylibs/frameworks and expected DSymName metadata. |
| tests/dotnet/UnitTests/AppSizeTest.cs | Adds dSYM presence/absence validation to app size build assertions. |
| msbuild/Xamarin.Shared/Xamarin.Shared.targets | Refactors post-processing item collection (frameworks/dylibs), adds path helper property, and sets DSymName defaults/overrides. |
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…sing The dSYMSourcePath for dylibs was computed using GetDirectoryName, which returns the parent directory. For a dylib at /path/to/libfoo.dylib, this resulted in /path/to.dSYM instead of the correct /path/to/libfoo.dylib.dSYM. Fix by using %(Identity).dSYM directly for dylibs, and add dSYMSourcePath validation to both the DylibPostProcessingItems and FrameworkPostProcessingItems tests. 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.
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.
The .NET runtime dylibs (from Microsoft.NETCore.App.Runtime.* NuGet packages) may contain symbols referenced by indirect symbol table entries that can't be stripped. Set NoSymbolStrip=true for these items in _CollectItemsForPostProcessing. 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.
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.
Frameworks are now properly collected for post-processing (strip), which causes their binaries to be synced back to the Windows bin directory during remote builds. The FrameworksInRuntimesNativeDirectory test frameworks have paths of 118 characters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ [CI Build #ad898b6] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #ad898b6] 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 #ad898b6] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #ad898b6] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #ad898b6] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #ad898b6] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #ad898b6] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #ad898b6] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #ad898b6] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 131 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Fix several issues in the _CollectItemsForPostProcessing target:
Improvements: