Move to generating msbuild choose elements to get if-else selection behavior for sources in projects so that we don't get erroneous duplicate files in cases where there are both profile and host platform criteria (#9952)

A recent commit revealed that in cases where we select based on a mix of host platform and profile, genproj csproj files can end up with duplicate sources because the existing <ItemGroup Condition= approach could make multiple groups match for a given compile when we really just want one.

This PR changes to generating a cascade of msbuild <Choose> elements, which give if-else selection to ensure that we only ever build a single set of files.

1 files changed, 97 insertions, 31 deletions

diff --git a/msvc/scripts/genproj.cs b/msvc/scripts/genproj.cs
index efd4bdd3f9a..72efda06e0b 100755
--- a/msvc/scripts/genproj.cs
+++ b/msvc/scripts/genproj.cs
@@ -857,6 +857,18 @@ public class MsbuildGenerator {
 		return SlnGenerator.profiles.Contains (profile);
 	}
 
+	private void GenerateSourceItems (XmlWriter writer, IEnumerable<string> fileNames, HashSet<string> commonFiles) {
+		foreach (var file in fileNames.OrderBy (f => f, StringComparer.Ordinal)) {
+			// FIXME: Is this needed?
+			if ((commonFiles != null) && commonFiles.Contains (file))
+				continue;
+
+			writer.WriteStartElement ("Compile");
+			writer.WriteAttributeString ("Include", file);
+			writer.WriteEndElement ();
+		}	
+	}
+
 	private StringBuilder GenerateSourceItemGroups (
 		string output_name, 
 		string profile,
@@ -864,6 +876,19 @@ public class MsbuildGenerator {
 		string groupConditional
 	) {
 		var result = new StringBuilder ();
+		var xmlWriterSettings = new XmlWriterSettings () {
+			ConformanceLevel = ConformanceLevel.Fragment,
+			WriteEndDocumentOnClose = true,
+			CheckCharacters = true,
+			Encoding = Encoding.UTF8,
+			Indent = true,
+			IndentChars = "  ",
+			NewLineChars = NewLine,
+			NewLineHandling = NewLineHandling.Replace,
+			NewLineOnAttributes = false,
+			OmitXmlDeclaration = true
+		};
+		var xmlWriter = XmlWriter.Create (result, xmlWriterSettings);
 		var parseResult = ReadSources (sources_file_name);
 
 		var hostPlatformNames = GetSourcesParser ().AllHostPlatformNames;
@@ -880,7 +905,7 @@ public class MsbuildGenerator {
 				.OrderBy (s => s, StringComparer.Ordinal)
 				.Distinct ()
 			let fileNames = new HashSet<string> (matches)
-			orderby target.Key.profile, target.Key.hostPlatform
+			orderby target.Key.profile descending, target.Key.hostPlatform descending
 			select (key: target.Key, fileNames: fileNames)).ToList ();
 
 		var commonFiles = targetFileSets.Aggregate (
@@ -892,44 +917,85 @@ public class MsbuildGenerator {
 					files.IntersectWith (targetSet.fileNames);
 				return files;
 			}
-		).ToList ();
-
+		);
 
-		result.Append ($"  <ItemGroup>{NewLine}");
-		foreach (var file in commonFiles.OrderBy (f => f, StringComparer.Ordinal))
-			result.Append ($"    <Compile Include=\"{file}\" />{NewLine}");
+		xmlWriter.WriteComment ("Common files");
+		xmlWriter.WriteStartElement ("ItemGroup");
+		GenerateSourceItems (xmlWriter, commonFiles, null);
 
 		if (commonFiles.Any (f => f.EndsWith("build\\common\\Consts.cs"))) {
 			var genconstsRelativePath = "$(SolutionDir)\\msvc\\scripts\\genconsts.csproj";
-			result.Append ($"    <ProjectReference Include=\"{genconstsRelativePath}\">{NewLine}");
-			result.Append ($"      <Name>genconsts</Name>");
-			result.Append ($"      <Project>{SlnGenerator.genconsts_csproj_guid}</Project>");
-			result.Append ($"      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>");
-			result.Append ($"      <CopyToOutputDirectory>Never</CopyToOutputDirectory>");
-			result.Append ($"      <Private>False</Private>");
-			result.Append ($"    </ProjectReference>{NewLine}");			
+			xmlWriter.WriteComment ("Genconsts dependency because this project includes Consts.cs");
+			xmlWriter.WriteStartElement ("ProjectReference");
+			xmlWriter.WriteAttributeString ("Include", genconstsRelativePath);
+			xmlWriter.WriteElementString ("Name", "genconsts");
+			xmlWriter.WriteElementString ("Project", SlnGenerator.genconsts_csproj_guid);
+			xmlWriter.WriteElementString ("ReferenceOutputAssembly", "false");
+			xmlWriter.WriteElementString ("CopyToOutputDirectory", "Never");
+			xmlWriter.WriteElementString ("Private", "false");
+			xmlWriter.WriteEndElement();
   		}
-  		
-		result.Append ($"  </ItemGroup>{NewLine}");
-
-		foreach (var set in targetFileSets) {
-			if ((set.key.hostPlatform == null) && (set.key.profile == null)) {
-				result.Append ($"  <ItemGroup>{NewLine}");
-			} else if (set.key.hostPlatform == null) {
-				result.Append ($"  <ItemGroup Condition=\" '$(Platform)' == '{set.key.profile}' \">{NewLine}");
-			} else if (set.key.profile == null) {
-				result.Append ($"  <ItemGroup Condition=\" '$(HostPlatform)' == '{set.key.hostPlatform}' \">{NewLine}");
-			} else {
-				result.Append ($"  <ItemGroup Condition=\" '$(Platform)|$(HostPlatform)' == '{set.key.profile}|{set.key.hostPlatform}' \">{NewLine}");
-			}
 
-			set.fileNames.ExceptWith (commonFiles);
+  		xmlWriter.WriteEndElement ();
+  		xmlWriter.WriteComment ("End of common files");
 
-			foreach (var file in set.fileNames.OrderBy (f => f, StringComparer.Ordinal))
-				result.Append ($"    <Compile Include=\"{file}\" />{NewLine}");
+  		// FIXME: Is this right if the profile/platform pair are not null,null? It probably is
+  		if (targetFileSets.Count != 1) {
+			var profileGroups = (from tfs in targetFileSets 
+				group tfs by tfs.key.profile into sets
+				select sets).ToList ();
 
-			result.Append ($"  </ItemGroup>{NewLine}");
-		}
+			xmlWriter.WriteComment ("Per-profile files");
+			if (profileGroups.Count > 1)
+				xmlWriter.WriteStartElement ("Choose");
+
+			foreach (var profileGroup in profileGroups) {
+				if (profileGroups.Count == 1) {
+				} else if (profileGroup.Key == null) {
+					xmlWriter.WriteStartElement ("Otherwise");
+				} else {
+					xmlWriter.WriteStartElement ("When");
+					xmlWriter.WriteAttributeString ("Condition", $"'$(Platform)' == '{profileGroup.Key}'");
+				}
+
+				var hostPlatforms = profileGroup.ToList ();
+				if (hostPlatforms.Count == 1) {
+					xmlWriter.WriteStartElement ("ItemGroup");
+					GenerateSourceItems (xmlWriter, hostPlatforms[0].fileNames, commonFiles);
+					xmlWriter.WriteEndElement ();
+				} else {
+					xmlWriter.WriteComment ("Per-host-platform files");
+					xmlWriter.WriteStartElement ("Choose");
+
+					foreach (var set in hostPlatforms) {
+						if (set.key.hostPlatform == null) {
+							xmlWriter.WriteStartElement ("Otherwise");
+						} else {
+							xmlWriter.WriteStartElement ("When");
+							xmlWriter.WriteAttributeString ("Condition", $"'$(HostPlatform)' == '{set.key.hostPlatform}'");
+						}
+
+						xmlWriter.WriteStartElement ("ItemGroup");
+						GenerateSourceItems (xmlWriter, set.fileNames, commonFiles);
+						xmlWriter.WriteEndElement ();
+
+						xmlWriter.WriteEndElement();
+					}
+
+					xmlWriter.WriteEndElement ();
+					xmlWriter.WriteComment ("End of per-host-platform files");
+				}
+
+				if (profileGroups.Count > 1)
+					xmlWriter.WriteEndElement ();
+			}
+
+			if (profileGroups.Count > 1)
+				xmlWriter.WriteEndElement ();
+			xmlWriter.WriteComment ("End of per-profile files");
+  		}
+
+		xmlWriter.Close ();
 
 		return result;
 	}