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

msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx

@@ -1766,4 +1766,42 @@
     <data name="E7152_InvalidMtouchHttpClientHandler" xml:space="preserve">
         <value>Invalid value for 'MtouchHttpClientHandler' ('{0}', must be either 'NSUrlSessionHandler' or 'CFNetworkHandler' (or not set at all).</value>
     </data>
+
+    <data name="W7154" xml:space="preserve">
+        <value>The {0} item '{1}' imported from '{2}' was ignored, because there's already an existing item from the current project with the same LogicalName ('{3}')</value>
+        <comment>
+            {0}: the name of the MSBuild item in question (BundleResource, ImageAsset, SceneKitAsset, etc.).
+            {1}: path to a file name (of an item)
+            {2}: path to an assembly
+            {3}: value of the LogicalName metadata of the item
+        </comment>
+    </data>
+
+    <data name="W7155" xml:space="preserve">
+        <value>The {0} item '{1}' imported from '{2}' was ignored, because there's another item from a different assembly ({4}) with the same LogicalName ('{3}')</value>
+        <comment>
+            {0}: the name of the MSBuild item in question (BundleResource, ImageAsset, SceneKitAsset, etc.).
+            {1}: path to a file name (of a resource)
+            {2}: path to an assembly
+            {3}: value of the LogicalName metadata.
+            {4}: comma-separated list of assemblies.
+        </comment>
+    </data>
+
+    <data name="W7156" xml:space="preserve">
+        <value>The {0} item '{1}' was ignored, because there's another item with the same LogicalName ('{2}')</value>
+        <comment>
+            {0}: the name of the MSBuild item in question (BundleResource, ImageAsset, SceneKitAsset, etc.).
+            {1}: path to a file name (of a resource)
+            {2}: value of the LogicalName metadata.
+        </comment>
+    </data>
+
+    <data name="E7157" xml:space="preserve">
+        <value>The {0} item '{1}' does not have a 'LogicalName' metadata.</value>
+        <comment>
+            {0}: the name of the MSBuild item in question (BundleResource, ImageAsset, SceneKitAsset, etc.).
+            {1}: path to a file name (of a resource)
+        </comment>
+    </data>
 </root>

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

@@ -285,7 +285,15 @@ public override bool Execute ()
 			var items = new List<AssetInfo> ();
 			var specs = new PArray ();
 
-			var imageAssets = ImageAssets
+			var filteredImageAssets = ImageAssets
+				.Where (item => {
+					// Ignore MacOS .DS_Store files...
+					return !Path.GetFileName (item.ItemSpec).Equals (".DS_Store", StringComparison.OrdinalIgnoreCase);
+				});
+
+			filteredImageAssets = CollectBundleResources.ComputeLogicalNameAndDetectDuplicates (this, filteredImageAssets, ProjectDir, string.Empty, "ImageAsset").ToArray ();
+
+			var imageAssets = filteredImageAssets
 				.Select (imageAsset => {
 					var vpath = BundleResource.GetVirtualProjectPath (this, imageAsset);
 					var catalogFullPath = imageAsset.GetMetadata ("FullPath");
@@ -303,10 +311,6 @@ public override bool Execute ()
 
 					return new AssetInfo (imageAsset, vpath, catalog, catalogFullPath, assetType);
 				})
-				.Where (asset => {
-					// Ignore MacOS .DS_Store files...
-					return !Path.GetFileName (asset.VirtualProjectPath).Equals (".DS_Store", StringComparison.OrdinalIgnoreCase);
-				})
 				.Where (asset => {
 					if (string.IsNullOrEmpty (asset.Catalog)) {
 						Log.LogWarning (null, null, null, asset.Item.ItemSpec, 0, 0, 0, 0, MSBStrings.W0090, asset.Item.ItemSpec);

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

@@ -2,6 +2,7 @@
 using System.IO;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
@@ -97,11 +98,92 @@ bool ExecuteImpl ()
 
 			bundleResources.AddRange (UnpackedResources);
 
-			BundleResourcesWithLogicalNames = bundleResources.ToArray ();
+			var distinctBundleResources = VerifyLogicalNameUniqueness (Log, bundleResources, "BundleResource");
+
+			BundleResourcesWithLogicalNames = distinctBundleResources.ToArray ();
 
 			return !Log.HasLoggedErrors;
 		}
 
+		[return: NotNullIfNotNull (nameof (items))]
+		public static ITaskItem []? VerifyLogicalNameUniqueness (TaskLoggingHelper Log, IEnumerable<ITaskItem>? items, string itemName)
+		{
+			if (items is null)
+				return null;
+
+			var rv = new List<ITaskItem> ();
+			var groupedBundleResources = items.GroupBy (item => item.GetMetadata ("LogicalName"));
+			var reportedItems = new HashSet<string> (); // Keep track of items we've shown warnings for, to not show multiple warnings.
+
+			foreach (var group in groupedBundleResources) {
+				// No/empty LogicalName is not OK.
+				if (string.IsNullOrEmpty (group.Key)) {
+					foreach (var item in group)
+						Log.LogError (7157, item.ItemSpec, MSBStrings.E7157 /* The {0} item '{0}' does not have a 'LogicalName' metadata. */, itemName, item.ItemSpec);
+					continue;
+				}
+
+				// One item per LogicalName is OK.
+				if (group.Count () == 1) {
+					rv.AddRange (group);
+					continue;
+				}
+
+				// More than one item per LogicalName is not good at all.
+				var notBundledInAssembly = group.Where (item => string.IsNullOrEmpty (item.GetMetadata ("BundledInAssembly"))).ToArray ();
+				var bundledInAssembly = group.Where (item => !string.IsNullOrEmpty (item.GetMetadata ("BundledInAssembly"))).ToArray ();
+				if (notBundledInAssembly.Length == 1) {
+					// Only one not from a library
+					rv.AddRange (notBundledInAssembly);
+					// warn about ignoring all the other imported ones.
+					foreach (var item in bundledInAssembly) {
+						if (reportedItems.Add (item.ItemSpec)) {
+							Log.LogWarning (7154, item.ItemSpec, MSBStrings.W7154 /* The {0} item '{1}' imported from '{2}' was ignored, because there's already an existing item from the current project with the same LogicalName ('{3}'). */, itemName, item.ItemSpec, item.GetMetadata ("BundledInAssembly"), group.Key);
+						}
+					}
+					continue;
+				} else if (notBundledInAssembly.Length == 0) {
+					// none from the current assembly, but multiple imported ones. Don't add any of them (to have a predictable build).
+					// warn about ignoring all the other ones
+					foreach (var item in bundledInAssembly) {
+						if (reportedItems.Add (item.ItemSpec)) {
+							var others = bundledInAssembly.
+											Where (i => !object.ReferenceEquals (i, item)).
+											Select (i => Path.GetFileName (i.GetMetadata ("BundledInAssembly"))).
+											ToArray ();
+							Log.LogWarning (7155, item.ItemSpec, MSBStrings.W7155 /* The {0} item '{1}' imported from '{2}' was ignored, because there's another item from a different assembly ({4}) with the same LogicalName ('{3}'). */, itemName, item.ItemSpec, item.GetMetadata ("BundledInAssembly"), group.Key, string.Join (", ", others));
+						}
+					}
+					continue;
+				} else {
+					// more than one for the current project?
+					// don't add any of them (to have a predictable build).
+					// warn about them all.
+					foreach (var item in notBundledInAssembly) {
+						if (reportedItems.Add (item.ItemSpec)) {
+							Log.LogWarning (7156, item.ItemSpec, MSBStrings.W7156 /* The {0} item '{1}' was ignored, because there's another item with the same LogicalName ('{2}'). */, itemName, item.ItemSpec, group.Key);
+						}
+					}
+				}
+			}
+
+			return rv.ToArray ();
+		}
+
+		[return: NotNullIfNotNull (nameof (items))]
+		public static ITaskItem []? ComputeLogicalNameAndDetectDuplicates<U> (U task, IEnumerable<ITaskItem>? items, string projectDir, string resourcePrefix, string itemName) where U : Task, IHasProjectDir, IHasResourcePrefix, IHasSessionId
+		{
+			if (items is null)
+				return null;
+
+			var prefixes = BundleResource.SplitResourcePrefixes (resourcePrefix);
+			foreach (var item in items) {
+				var logicalName = BundleResource.GetLogicalName (task, item);
+				item.SetMetadata ("LogicalName", logicalName);
+			}
+			return VerifyLogicalNameUniqueness (task.Log, items, itemName);
+		}
+
 		public static bool TryCreateItemWithLogicalName<T> (T task, ITaskItem item, [NotNullWhen (true)] out TaskItem? itemWithLogicalName) where T : Task, IHasProjectDir, IHasResourcePrefix, IHasSessionId
 		{
 			itemWithLogicalName = null;

msbuild/Xamarin.MacDev.Tasks/Tasks/CompileSceneKitAssets.cs

@@ -141,17 +141,19 @@ public override bool Execute ()
 			var bundleResources = new List<ITaskItem> ();
 			var modified = new HashSet<string> ();
 			var items = new List<ITaskItem> ();
+			var sceneKitAssets = CollectBundleResources.ComputeLogicalNameAndDetectDuplicates (this, SceneKitAssets, ProjectDir, ResourcePrefix, "SceneKitAsset");
 
-			foreach (var asset in SceneKitAssets) {
+			foreach (var asset in sceneKitAssets) {
 				if (!File.Exists (asset.ItemSpec))
 					continue;
 
 				// get the .scnassets directory path
 				if (!TryGetScnAssetsPath (asset.ItemSpec, out var scnassets))
 					continue;
 
-				var bundleName = BundleResource.GetLogicalName (this, asset);
-				var output = new TaskItem (Path.Combine (intermediate, bundleName));
+				var logicalName = asset.GetMetadata ("LogicalName");
+				var bundleName = logicalName;
+				var output = new TaskItem (Path.Combine (intermediate, logicalName));
 
 				if (!modified.Contains (scnassets) && (!File.Exists (output.ItemSpec) || File.GetLastWriteTimeUtc (asset.ItemSpec) > File.GetLastWriteTimeUtc (output.ItemSpec))) {
 					// Base the new item on @asset, to get the `DefiningProject*` metadata too
@@ -161,7 +163,7 @@ public override bool Execute ()
 					scnassetsItem.ItemSpec = scnassets;
 
 					// .. and set LogicalName, the original one is for @asset
-					if (!TryGetScnAssetsPath (bundleName, out var logicalScnAssetsPath)) {
+					if (!TryGetScnAssetsPath (logicalName, out var logicalScnAssetsPath)) {
 						Log.LogError (null, null, null, asset.ItemSpec, MSBStrings.E7136 /* Unable to compute the path of the *.scnassets path from the item's LogicalName '{0}'. */ , bundleName);
 						continue;
 					}

msbuild/Xamarin.MacDev.Tasks/Tasks/CoreMLCompiler.cs

@@ -164,12 +164,13 @@ public override bool Execute ()
 			var mapping = new Dictionary<string, IDictionary> ();
 			var bundleResources = new List<ITaskItem> ();
 			var partialPlists = new List<ITaskItem> ();
+			var models = CollectBundleResources.ComputeLogicalNameAndDetectDuplicates (this, Models, ProjectDir, ResourcePrefix, "CoreMLModel");
 
-			if (Models.Length > 0) {
+			if (models.Length > 0) {
 				Directory.CreateDirectory (coremlcOutputDir);
 
-				foreach (var model in Models) {
-					var logicalName = BundleResource.GetLogicalName (this, model);
+				foreach (var model in models) {
+					var logicalName = model.GetMetadata ("LogicalName");
 					var bundleName = GetPathWithoutExtension (logicalName) + ".mlmodelc";
 					var outputPath = Path.Combine (coremlcOutputDir, bundleName);
 					var outputDir = Path.GetDirectoryName (outputPath);

msbuild/Xamarin.MacDev.Tasks/Tasks/IBTool.cs

@@ -181,16 +181,16 @@ static bool InterfaceDefinitionChanged (ITaskItem interfaceDefinition, ITaskItem
 			return !LogExists (log.ItemSpec) || File.GetLastWriteTimeUtc (log.ItemSpec) < File.GetLastWriteTimeUtc (interfaceDefinition.ItemSpec);
 		}
 
-		bool CompileInterfaceDefinitions (string baseManifestDir, string baseOutputDir, List<ITaskItem> compiled, IList<ITaskItem> manifests, out bool changed)
+		bool CompileInterfaceDefinitions (IEnumerable<ITaskItem> interfaceDefinitions, string baseManifestDir, string baseOutputDir, List<ITaskItem> compiled, IList<ITaskItem> manifests, out bool changed)
 		{
 			var mapping = new Dictionary<string, IDictionary> ();
 			var unique = new Dictionary<string, ITaskItem> ();
 			var targets = GetTargetDevices ().ToList ();
 
 			changed = false;
 
-			foreach (var item in InterfaceDefinitions) {
-				var bundleName = GetBundleRelativeOutputPath (item);
+			foreach (var item in interfaceDefinitions) {
+				var bundleName = item.GetMetadata ("LogicalName");
 				var manifest = new TaskItem (Path.Combine (baseManifestDir, bundleName));
 				var manifestDir = Path.GetDirectoryName (manifest.ItemSpec);
 				ITaskItem duplicate;
@@ -422,11 +422,18 @@ public override bool Execute ()
 			var compiled = new List<ITaskItem> ();
 			bool changed;
 
-			if (InterfaceDefinitions.Length > 0) {
+			foreach (var item in InterfaceDefinitions) {
+				// Note: we overwrite any existing LogicalName property, because interface definitions always go in the app bundle's root directory.
+				var bundleName = GetBundleRelativeOutputPath (item);
+				item.SetMetadata ("LogicalName", bundleName);
+			}
+			var interfaceDefinitions = CollectBundleResources.VerifyLogicalNameUniqueness (this.Log, InterfaceDefinitions, "InterfaceDefinition").ToArray ();
+
+			if (interfaceDefinitions.Length > 0) {
 				Directory.CreateDirectory (ibtoolManifestDir);
 				Directory.CreateDirectory (ibtoolOutputDir);
 
-				if (!CompileInterfaceDefinitions (ibtoolManifestDir, ibtoolOutputDir, compiled, outputManifests, out changed))
+				if (!CompileInterfaceDefinitions (interfaceDefinitions, ibtoolManifestDir, ibtoolOutputDir, compiled, outputManifests, out changed))
 					return false;
 
 				if (CanLinkStoryboards) {

msbuild/Xamarin.MacDev.Tasks/Tasks/ScnTool.cs

@@ -76,11 +76,12 @@ public override bool Execute ()
 			if (ShouldExecuteRemotely ())
 				return new TaskRunner (SessionId, BuildEngine4).RunAsync (this).Result;
 
+			var colladaAssets = CollectBundleResources.ComputeLogicalNameAndDetectDuplicates (this, ColladaAssets, ProjectDir, ResourcePrefix, "Collada");
 			var listOfArguments = new List<(IList<string> Arguments, ITaskItem Input)> ();
 			var bundleResources = new List<ITaskItem> ();
-			foreach (var asset in ColladaAssets) {
+			foreach (var asset in colladaAssets) {
 				var inputScene = asset.ItemSpec;
-				var logicalName = BundleResource.GetLogicalName (this, asset);
+				var logicalName = asset.GetMetadata ("LogicalName");
 				var outputScene = Path.Combine (DeviceSpecificIntermediateOutputPath, logicalName);
 				var args = GenerateCommandLineCommands (inputScene, outputScene);
 				listOfArguments.Add (new (args, asset));

msbuild/Xamarin.MacDev.Tasks/Tasks/TextureAtlas.cs

@@ -87,11 +87,12 @@ protected override IEnumerable<ITaskItem> EnumerateInputs ()
 			if (AtlasTextures is null)
 				yield break;
 
+			var atlasTextures = CollectBundleResources.ComputeLogicalNameAndDetectDuplicates (this, AtlasTextures, ProjectDir, ResourcePrefix, "AtlasTexture");
+
 			// group the atlas textures by their parent .atlas directories
-			foreach (var item in AtlasTextures) {
-				var vpp = BundleResource.GetVirtualProjectPath (this, item);
-				var atlasName = Path.GetDirectoryName (vpp);
+			foreach (var item in atlasTextures) {
 				var logicalName = item.GetMetadata ("LogicalName");
+				var atlasName = Path.GetDirectoryName (logicalName);
 
 				if (!atlases.TryGetValue (atlasName, out var atlas)) {
 					var atlasItem = new TaskItem (atlasName);

msbuild/Xamarin.Shared/Xamarin.Shared.targets

@@ -1296,7 +1296,6 @@ Copyright (C) 2018 Microsoft. All rights reserved.
 		</ItemGroup>
 	</Target>
 
-	<!-- TODO: check for duplicate items -->
 	<Target Name="_ComputeBundleResourceOutputPaths"
 		Condition="'$(_CanOutputAppBundle)' == 'true'"
 		DependsOnTargets="_CollectBundleResources;_GenerateBundleName;_DetectSigningIdentity;_ReadAppManifest">

tests/common/Configuration.cs

@@ -533,6 +533,14 @@ public static string GetBaseLibrary (TargetFramework targetFramework)
 			return Path.Combine (GetRefDirectory (targetFramework), GetBaseLibraryName (targetFramework));
 		}
 
+		public static IList<string> GetAllRuntimeIdentifiers ()
+		{
+			var rv = new List<string> ();
+			foreach (var platform in GetAllPlatforms ())
+				rv.AddRange (GetRuntimeIdentifiers (platform));
+			return rv;
+		}
+
 		public static IList<string> GetRuntimeIdentifiers (ApplePlatform platform)
 		{
 			return GetVariableArray ($"DOTNET_{platform.AsString ().ToUpper ()}_RUNTIME_IDENTIFIERS");

tests/common/DotNet.cs

@@ -63,9 +63,9 @@ public static ExecutionResult Restore (string project, Dictionary<string, string
 			return Execute ("restore", project, properties, false);
 		}
 
-		public static ExecutionResult AssertBuild (string project, Dictionary<string, string>? properties = null, TimeSpan? timeout = null)
+		public static ExecutionResult AssertBuild (string project, Dictionary<string, string>? properties = null, string? target = null, TimeSpan? timeout = null)
 		{
-			return Execute ("build", project, properties, true, timeout: timeout);
+			return Execute ("build", project, properties, true, target: target, timeout: timeout);
 		}
 
 		public static ExecutionResult AssertBuildFailure (string project, Dictionary<string, string>? properties = null)

tests/dotnet/AppWithDuplicatedResources/AppDelegate.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Runtime.InteropServices;
+
+using Foundation;
+
+namespace MySimpleApp {
+	public class Program {
+		static int Main (string [] args)
+		{
+			GC.KeepAlive (typeof (NSObject)); // prevent linking away the platform assembly
+
+			Console.WriteLine (Environment.GetEnvironmentVariable ("MAGIC_WORD"));
+
+			return args.Length;
+		}
+	}
+}

tests/dotnet/AppWithDuplicatedResources/MacCatalyst/AppWithDuplicatedResources.csproj

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project Sdk="Microsoft.NET.Sdk">
+	<PropertyGroup>
+		<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-maccatalyst</TargetFramework>
+	</PropertyGroup>
+	<Import Project="..\shared.csproj" />
+</Project>

tests/dotnet/AppWithDuplicatedResources/MacCatalyst/Makefile

@@ -0,0 +1 @@
+include ../shared.mk

tests/dotnet/AppWithDuplicatedResources/Makefile

@@ -0,0 +1,2 @@
+TOP=../../..
+include $(TOP)/tests/common/shared-dotnet-test.mk

tests/dotnet/AppWithDuplicatedResources/iOS/AppWithDuplicatedResources.csproj

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project Sdk="Microsoft.NET.Sdk">
+	<PropertyGroup>
+		<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-ios</TargetFramework>
+	</PropertyGroup>
+	<Import Project="..\shared.csproj" />
+</Project>

tests/dotnet/AppWithDuplicatedResources/iOS/Makefile

@@ -0,0 +1 @@
+include ../shared.mk

tests/dotnet/AppWithDuplicatedResources/macOS/AppWithDuplicatedResources.csproj

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project Sdk="Microsoft.NET.Sdk">
+	<PropertyGroup>
+		<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-macos</TargetFramework>
+	</PropertyGroup>
+	<Import Project="..\shared.csproj" />
+</Project>

tests/dotnet/AppWithDuplicatedResources/macOS/Makefile

@@ -0,0 +1 @@
+include ../shared.mk