[msbuild] Fix multiple resource prefixes support. Fixes #20968 - #26020
[msbuild] Fix multiple resource prefixes support. Fixes #20968#26020rolfbjarne wants to merge 12 commits into
Conversation
Add unit tests and an E2E test to verify that semicolon-separated multiple prefixes in AppBundleResourcePrefix (and the legacy IPhoneResourcePrefix/XamMacResourcePrefix) work correctly. Unit tests (in ResourcePrefixTest.cs): - Property value propagation with multiple prefixes - AppBundleResourcePrefix with multiple prefixes - BundleResource item inclusion from single/multiple prefix directories - BundleResource items when only the second prefix has files - BundleResource items in subdirectories of multiple prefixes - Three or more semicolon-separated prefixes E2E test (AppWithMultipleResourcePrefixes): - On-disk project with AppBundleResourcePrefix set to 'Resources;PlatformResources' - Resource files in both directories (including subdirectories) - Builds for all platforms and validates the app bundle contains resources from both prefix directories These tests exercise the behavior described in issue #20968, where the $(_ResourcePrefix)\\**\\* glob pattern in the BundleResource Include may not expand correctly for multiple semicolon-separated values. Fixes #20968 Co-authored-by: Copilot App <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.
The BundleResource Include pattern used $(_ResourcePrefix)\\**\\* which doesn't work when _ResourcePrefix contains multiple semicolon-separated paths (e.g. 'Resources;PlatformResources'). MSBuild treats the semicolons as item separators, so only the last path would get the glob suffix applied, and the first paths would be treated as literal file includes. Fix this by converting the _ResourcePrefix property into an item list (_ResourcePrefixItem) and using MSBuild item transforms (@(_ResourcePrefixItem->'%(Identity)\\**\\*')) to generate the correct glob pattern for each prefix path. The same approach is applied to all the Exclude patterns. The C# task layer (BundleResource.SplitResourcePrefixes) already handles multiple prefixes correctly, so no changes are needed there. Co-authored-by: Copilot App <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.
Item transforms (@(Item->'transform')) don't expand globs in MSBuild -
the resulting strings are treated as literal file paths. Instead, use
$(_ResourcePrefix.Replace(';','\**\*;'))\**\* which produces a
property expansion that MSBuild properly evaluates as glob patterns.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes MSBuild default BundleResource globbing so that _ResourcePrefix can contain multiple semicolon-separated resource prefix paths (from AppBundleResourcePrefix and legacy prefix properties), and adds unit/E2E coverage to prevent regressions.
Changes:
- Update
Microsoft.Sdk.DefaultItems.template.propsto correctly apply\**\*(and exclusion patterns) to each semicolon-separated prefix entry. - Add unit tests validating multi-prefix property propagation and
BundleResourceitem inclusion across multiple prefix directories. - Add a new end-to-end test project (
AppWithMultipleResourcePrefixes) with resources split across two prefixes and validated in the built app bundle.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet/UnitTests/ResourcePrefixTest.cs | Adds unit tests + E2E validation for multiple resource prefixes and bundle contents. |
| dotnet/targets/Microsoft.Sdk.DefaultItems.template.props | Fixes BundleResource Include/Exclude patterns to work with semicolon-separated resource prefixes. |
| tests/dotnet/AppWithMultipleResourcePrefixes/AppDelegate.cs | Minimal app entrypoint for the E2E build test project. |
| tests/dotnet/AppWithMultipleResourcePrefixes/shared.csproj | Shared test project settings, including AppBundleResourcePrefix=Resources;PlatformResources. |
| tests/dotnet/AppWithMultipleResourcePrefixes/shared.mk | Shared make include for running the E2E test project via existing dotnet test infrastructure. |
| tests/dotnet/AppWithMultipleResourcePrefixes/shared.plist | Shared (minimal) partial app manifest for the E2E test project. |
| tests/dotnet/AppWithMultipleResourcePrefixes/iOS/AppWithMultipleResourcePrefixes.csproj | iOS project wrapper importing shared settings. |
| tests/dotnet/AppWithMultipleResourcePrefixes/iOS/Makefile | iOS test project make include. |
| tests/dotnet/AppWithMultipleResourcePrefixes/iOS/Resources/SharedResource.txt | Resource file from the first prefix for iOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/iOS/PlatformResources/PlatformResource.txt | Resource file from the second prefix for iOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/iOS/PlatformResources/SubDir/SubDirResource.txt | Subdirectory resource under the second prefix for iOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/tvOS/AppWithMultipleResourcePrefixes.csproj | tvOS project wrapper importing shared settings. |
| tests/dotnet/AppWithMultipleResourcePrefixes/tvOS/Makefile | tvOS test project make include. |
| tests/dotnet/AppWithMultipleResourcePrefixes/tvOS/Resources/SharedResource.txt | Resource file from the first prefix for tvOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/tvOS/PlatformResources/PlatformResource.txt | Resource file from the second prefix for tvOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/tvOS/PlatformResources/SubDir/SubDirResource.txt | Subdirectory resource under the second prefix for tvOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/MacCatalyst/AppWithMultipleResourcePrefixes.csproj | Mac Catalyst project wrapper importing shared settings. |
| tests/dotnet/AppWithMultipleResourcePrefixes/MacCatalyst/Makefile | Mac Catalyst test project make include. |
| tests/dotnet/AppWithMultipleResourcePrefixes/MacCatalyst/Resources/SharedResource.txt | Resource file from the first prefix for Mac Catalyst. |
| tests/dotnet/AppWithMultipleResourcePrefixes/MacCatalyst/PlatformResources/PlatformResource.txt | Resource file from the second prefix for Mac Catalyst. |
| tests/dotnet/AppWithMultipleResourcePrefixes/MacCatalyst/PlatformResources/SubDir/SubDirResource.txt | Subdirectory resource under the second prefix for Mac Catalyst. |
| tests/dotnet/AppWithMultipleResourcePrefixes/macOS/AppWithMultipleResourcePrefixes.csproj | macOS project wrapper importing shared settings. |
| tests/dotnet/AppWithMultipleResourcePrefixes/macOS/Makefile | macOS test project make include. |
| tests/dotnet/AppWithMultipleResourcePrefixes/macOS/Resources/SharedResource.txt | Resource file from the first prefix for macOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/macOS/PlatformResources/PlatformResource.txt | Resource file from the second prefix for macOS. |
| tests/dotnet/AppWithMultipleResourcePrefixes/macOS/PlatformResources/SubDir/SubDirResource.txt | Subdirectory resource under the second prefix for macOS. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MSBuild glob expansion only works for literal strings and simple
property expansions in Include attributes. Item transforms and property
functions both produce strings that are treated as literal identities
without glob expansion or semicolon splitting.
Fix multi-prefix by:
- In DefaultItems.props: keep the original glob for single-prefix only
(via Contains(';') condition), and populate a _ResourcePrefixItem list
for multi-prefix.
- In Xamarin.Shared.targets: add _ExpandMultipleResourcePrefixes target
that uses batched glob expansion (%(_ResourcePrefixItem.Identity))
inside a Target ItemGroup, which correctly expands globs per prefix.
Also update unit tests to pass the target name to -getItem: so that
target-created items are visible in the query results.
Co-authored-by: Copilot App <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.
Using %() metadata references in Exclude attributes within target ItemGroups can cause MSB5029 parsing errors in some MSBuild versions. Switch to Include followed by separate Remove entries, which achieves the same result without the parsing issue. Co-authored-by: Copilot App <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.
When using %(_ResourcePrefixItem.Identity) batching for the Include, %(FullPath) in the Link metadata resolves to the _ResourcePrefixItem's FullPath (the directory) instead of the newly included BundleResource item's FullPath. This caused Link to be set to just 'Resources' instead of 'Resources/SharedResource.txt', breaking the logical name computation in CollectBundleResources. Remove Link entirely — the CollectBundleResources task computes the logical name correctly from the item's Identity (which is already relative to the project directory) combined with the ResourcePrefix property. Co-authored-by: Copilot App <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.
|
/review |
|
❌ .NET for Apple Platforms PR Reviewer failed. Please review the logs for details. |
rolfbjarne
left a comment
There was a problem hiding this comment.
❌ Reject — 1 error.
The multi-prefix expansion is well covered across platforms, and CI is green. However, the exclusion pass currently mutates the complete BundleResource item group, so projects using multiple prefixes can lose explicitly declared resources. Please scope the removals to an intermediate set of newly globbed default items before adding that set to BundleResource.
…iphoneresourceprefix-maybe-supports-mult-7b2bb4
- Use intermediate _DefaultBundleResourceCandidate item for exclusions so that Remove operations don't affect explicitly declared BundleResource items (fixes scoping issue). - Use -arm64 instead of -x64 in E2E test cases. - Remove null-forgiving operator in GetBundleResourceIdentities. Co-authored-by: Copilot App <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.
Instead of expanding multi-prefix resources in a target (which prevents
items from showing up in the IDE without building), use
Split(';')[N] property functions to extract each prefix individually
in the DefaultItems.props file. This way, all BundleResource items are
available at evaluation time.
Up to 3 semicolon-separated resource prefix paths are supported.
An error is raised at build time if more than 3 are specified.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23a9f378-955d-436d-8d29-4c754cac5f88
|
Addressed the review comment in adaac25: moved multi-prefix expansion out of the target and into the |
Define _ResourcePrefix_1, _ResourcePrefix_2, _ResourcePrefix_3 properties
in Xamarin.Shared.props, and use those simple property references in
DefaultItems.template.props instead of repeating the verbose
$([System.String]::new('$(_ResourcePrefixPadded)').Split(';')[N])
expression everywhere. Much cleaner and easier to read.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23a9f378-955d-436d-8d29-4c754cac5f88
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.
Use '$(_ResourcePrefix_N)' != '' conditions instead of checking _ResourcePrefixCount. This handles edge cases like 'prefix;;' where empty entries between semicolons should be ignored. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23a9f378-955d-436d-8d29-4c754cac5f88
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.
…ybe-supports-mult-7b2bb4
✅ 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 #785a112] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 203 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 |
This PR adds support for multiple semicolon-separated values in
AppBundleResourcePrefix(and the legacyIPhoneResourcePrefix/XamMacResourcePrefixproperties).Previously, setting e.g.
<AppBundleResourcePrefix>Resources;PlatformResources</AppBundleResourcePrefix>didn't work correctly because the$(_ResourcePrefix)\**\*glob pattern inDefaultItems.propstreated the semicolons as MSBuild item separators — only the last path got the glob suffix, while earlier paths were treated as literal file includes.Fix
Convert
_ResourcePrefixinto an intermediate item list (_ResourcePrefixItem) and use MSBuild item transforms (@(_ResourcePrefixItem->'%(Identity)\**\*')) to generate correct glob patterns for each prefix path. The same approach is applied to all Exclude patterns.The C# task layer (
BundleResource.SplitResourcePrefixes) already handled multiple prefixes correctly, so no changes were needed there.Tests
BundleResourceitem inclusion from multiple prefix directoriesAppWithMultipleResourcePrefixes) that builds an app with two resource prefix directories and validates the app bundle contains resources from bothFixes #20968
🤖 Pull request created by Copilot