[msbuild] Add validation for conflicting resources. Fixes #19029. #22996

rolfbjarne · 2025-06-04T18:25:56Z

Add validation for conflicting resources, showing a warning if the same
resource is found from multiple sources.

Fixes #19029.

mandel-macaque

Approving since I'm not going to block for the use of Count, I trust your call in that regard.

mandel-macaque · 2025-06-18T14:50:43Z

msbuild/Xamarin.MacDev.Tasks/Tasks/CollectBundleResources.cs

+				// More than one item per LogicalName is not good at all.
+				var notBundledInAssembly = group.Where (item => string.IsNullOrEmpty (item.GetMetadata ("BundledInAssembly")));
+				var bundledInAssembly = group.Where (item => !string.IsNullOrEmpty (item.GetMetadata ("BundledInAssembly")));
+				if (notBundledInAssembly.Count () == 1) {


You are using Count several times in the IEnumerable, that means that the notBundledInAssembly is being iterated several times. Why not do a ToArray and then use Length? Otherwise in the worst case scenario we are iterating over the collection 3 times.

Copilot

Pull Request Overview

This PR implements enhanced duplicate resource detection by adding new validation logic and warnings when conflicting resources are encountered. Key changes include:

Updating MSBuild tasks (e.g. TextureAtlas, IBTool, ScnTool, CoreMLCompiler, CompileSceneKitAssets, ACTool, and CollectBundleResources) to compute logical names and detect duplicates.
Enhancing project configurations and test setups for different platforms (macOS, iOS, MacCatalyst) to incorporate the new validation.
Adding new methods and error/warning messages in common utilities and localization resource files.

Reviewed Changes

Copilot reviewed 20 out of 89 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/dotnet/AppWithDuplicatedResources/shared.csproj	Added resource item groups with duplicate resource conditions.
tests/dotnet/AppWithDuplicatedResources/macOS/.csproj, iOS/.csproj, MacCatalyst/*.csproj	Updated project files to import shared settings.
tests/common/DotNet.cs	Updated AssertBuild signature with an extra ‘target’ parameter.
tests/common/Configuration.cs	Introduced GetAllRuntimeIdentifiers for runtime identifier aggregation.
msbuild/Xamarin.Shared/Xamarin.Shared.targets	Removed an obsolete TODO comment regarding duplicate items.
msbuild/Xamarin.MacDev.Tasks/Tasks/*	Modified various tasks to incorporate duplicate resource detection and proper logical naming.
msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx	Added new error and warning messages related to resource duplicate conflicts.

Comments suppressed due to low confidence (1)

tests/common/DotNet.cs:66

Consider updating the method documentation for 'AssertBuild' to describe the new 'target' parameter and its intended usage.

		public static ExecutionResult AssertBuild (string project, Dictionary<string, string>? properties = null, string? target = null, TimeSpan? timeout = null)

msbuild/Xamarin.MacDev.Tasks/Tasks/ACTool.cs

…-if-duplicate-bundle-resources

vs-mobiletools-engineering-service2 · 2025-06-18T16:20:16Z

✅ [CI Build #120071c] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T16:22:29Z

✅ [PR Build #120071c] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T16:49:04Z

✅ API diff for current PR / commit

.NET ( No breaking changes )

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

✅ API diff vs stable

.NET ( No breaking changes )

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T16:51:54Z

✅ [CI Build #120071c] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T17:04:20Z

💻 [CI Build #120071c] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T17:07:32Z

💻 [CI Build #120071c] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T17:09:02Z

💻 [CI Build #120071c] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T17:09:48Z

💻 [CI Build #120071c] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

✅ All tests on macOS arm64 - Mac Sequoia (15) passed.

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

vs-mobiletools-engineering-service2 · 2025-06-18T17:53:36Z

🚀 [CI Build #120071c] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 115 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 8 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 120071cf20a465894d99dc63b32f862807cbad93 [PR build]

This comment has been minimized.

Sign in to view

rolfbjarne force-pushed the dev/rolf/msbuild-warn-if-duplicate-bundle-resources branch from 7360f12 to 777706c Compare June 5, 2025 07:20

This comment has been minimized.

Sign in to view

[msbuild] Add validation for conflicting resources. Fixes #19029.

e824020

Fixes #19029.

rolfbjarne force-pushed the dev/rolf/msbuild-warn-if-duplicate-bundle-resources branch from 777706c to e824020 Compare June 5, 2025 10:09

Auto-format source code

3d042c6

This comment has been minimized.

Sign in to view

rolfbjarne marked this pull request as ready for review June 16, 2025 17:23

rolfbjarne requested review from emaf and mauroa as code owners June 16, 2025 17:23

dalexsoto approved these changes Jun 17, 2025

View reviewed changes

mandel-macaque approved these changes Jun 18, 2025

View reviewed changes

Avoid iterating over the same enumerable more than once.

b5a1006

rolfbjarne requested a review from Copilot June 18, 2025 15:00

Copilot AI reviewed Jun 18, 2025

View reviewed changes

msbuild/Xamarin.MacDev.Tasks/Tasks/ACTool.cs Show resolved Hide resolved

emaf approved these changes Jun 18, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into dev/rolf/msbuild-warn…

120071c

…-if-duplicate-bundle-resources

rolfbjarne merged commit a0e276b into main Jun 19, 2025
44 checks passed

rolfbjarne deleted the dev/rolf/msbuild-warn-if-duplicate-bundle-resources branch June 19, 2025 06:50

[msbuild] Add validation for conflicting resources. Fixes #19029. #22996

[msbuild] Add validation for conflicting resources. Fixes #19029. #22996

Uh oh!

Conversation

rolfbjarne commented Jun 4, 2025

Uh oh!

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.

mandel-macaque left a comment

Choose a reason for hiding this comment

Uh oh!

mandel-macaque Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

✅ [CI Build #120071c] Build passed (Build packages) ✅

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

✅ [PR Build #120071c] Build passed (Detect API changes) ✅

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

✅ [CI Build #120071c] Build passed (Build macOS tests) ✅

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

💻 [CI Build #120071c] Tests on macOS X64 - Mac Sonoma (14) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

💻 [CI Build #120071c] Tests on macOS M1 - Mac Monterey (12) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

💻 [CI Build #120071c] Tests on macOS M1 - Mac Ventura (13) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025

💻 [CI Build #120071c] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Jun 18, 2025