Consolidate many guidsForInstaller files into one

CHANGELOG.md

@@ -16,6 +16,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- [SIL.BuildTasks] Added MakeWixForDirTree.ConsolidatedGuidFile (optional param) naming a single file to hold the GUIDs for the whole tree, instead of a `.guidsForInstaller.xml` in every directory. Any per-directory files still present under RootDirectory are merged into it, so existing GUIDs carry over unchanged and installed components keep their identity; delete them from version control once the merged file has been committed. With CheckOnly the merge happens in memory only and the task reports an error naming the files whose GUIDs are not yet in the consolidated file.
+
+### Fixed
+
+- [SIL.BuildTasks] Fixed MakeWixForDirTree.CheckOnly deleting the previously generated wxs file. A check-only run outputs nothing, so it now leaves the existing file alone.
+
 ## [3.2.1] - 2026-08-31
 
 ### Security

SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs

@@ -21,28 +21,35 @@ internal class IdToGuidDatabase
 		private readonly ILogger _logger;
 		private readonly string _filename;
 		private readonly Dictionary<string, string> _guids = new Dictionary<string, string>();
+		// Where each GUID came from. Usually _filename, but entries merged in by
+		// ImportMissingFrom keep pointing at the file that actually supplied them.
+		private readonly Dictionary<string, string> _origins = new Dictionary<string, string>();
+		// Whether the GUID database has new data needing to be written
+		private bool _dirty;
 
 
 		#region Construction
 
-		private IdToGuidDatabase(string filename, ILogger logger)
+		private IdToGuidDatabase(string filename, ILogger logger, bool dirty = false)
 		{
 			_filename = filename;
 			_logger = logger;
+			_dirty = dirty;
 		}
 
 		public static IdToGuidDatabase Create(string filename, ILogger owner)
 		{
 			if (!File.Exists(filename))
-				return new IdToGuidDatabase(filename, owner);
+				return new IdToGuidDatabase(filename, owner, dirty: true);
 
-			var settings = new XmlReaderSettings {
+			var settings = new XmlReaderSettings
+			{
 				IgnoreComments = true,
 				IgnoreWhitespace = true
 			};
 			using (var rdr = XmlReader.Create(filename, settings))
 			{
-				var m = new IdToGuidDatabase(filename, owner);
+				var m = new IdToGuidDatabase(filename, owner, dirty: false);
 
 				// skip XML declaration
 				do
@@ -63,7 +70,7 @@ public static IdToGuidDatabase Create(string filename, ILogger owner)
 						if (id == null || guid == null)
 							throw new XmlException("Unexpected format");
 
-						m[id] = guid;
+						m.Set(id, guid, filename);
 					}
 					else if (rdr.NodeType == XmlNodeType.EndElement)
 					{
@@ -87,7 +94,18 @@ private string this[string id]
 				string ret;
 				return _guids.TryGetValue(id, out ret) ? ret : null;
 			}
-			set => _guids[id] = value;
+		}
+
+		private void Set(string id, string guid, string origin)
+		{
+			_guids[id] = guid;
+			_origins[id] = origin;
+		}
+
+		private string OriginOf(string id)
+		{
+			string origin;
+			return _origins.TryGetValue(id, out origin) ? origin : _filename;
 		}
 
 
@@ -109,13 +127,98 @@ public string GetGuid(string id, bool justCheckDontCreate)
 			{
 				_logger.LogMessage(MessageImportance.Low, "No GUID for " + id + " in " + _filename);
 				guid = Guid.NewGuid().ToString();
-				this[id] = guid;
+				Set(id, guid, _filename);
+				// MarkDirty() is not enough here. This GUID is about to be saved in the
+				// generated .wxs file, so we *must* ensure that it's persisted.
 				Write();
 			}
 
 			return guid?.ToUpper();
 		}
 
+		/// <summary>
+		/// Copies in every entry this database does not already have, and saves if
+		/// anything was added. Used to consolidate the per-directory files into a
+		/// single one: File Ids encode the last 50 chars of the relative path (for
+		/// example "mercurial.lib.dulwich._pack.pyd"), so they are highly likely to
+		/// be unique across the tree (unless a really long filename is reused), and
+		/// suffixes to file IDs help ensure uniqueness, allowing existing GUIDs to
+		/// be merged without renaming anything.
+		/// With justCheckDontCreate the entries are still merged in memory, so that
+		/// GetGuid finds them, but nothing is written to disk.
+		/// </summary>
+		public void ImportMissingFrom(IdToGuidDatabase other, bool justCheckDontCreate)
+		{
+			if (other == null || ReferenceEquals(other, this))
+				return;
+
+			var added = false;
+			foreach (var pair in other._guids)
+			{
+				var existing = this[pair.Key];
+				if (existing == null)
+				{
+					Set(pair.Key, pair.Value, other._filename);
+					added = true;
+				}
+				else if (!string.Equals(existing, pair.Value, StringComparison.OrdinalIgnoreCase))
+				{
+					// Cannot happen while Ids stay path-derived, but silently preferring
+					// one GUID over another would be a nasty way to find out otherwise.
+					_logger.LogError(string.Format(
+						"Conflicting GUIDs for {0}: {1} (from {2}) and {3} (from {4}). Keeping the first.",
+						pair.Key, existing, OriginOf(pair.Key), pair.Value, other._filename));
+				}
+			}
+
+			if (added && !justCheckDontCreate) MarkDirty();
+		}
+
+		public void FinalizeImport(bool justCheckDontCreate)
+		{
+			if (!justCheckDontCreate) Flush();
+		}
+
+		/// <summary>
+		/// Logs an error for every entry this database holds only in memory, because it
+		/// was merged in from another file rather than read from its own. Deleting those
+		/// files without a run that writes this one would lose the GUIDs.
+		///
+		/// Only meaningful after a run that has not written: once Write() has run, the
+		/// entries are in the file even though _origins still records where they began.
+		/// </summary>
+		public void ReportEntriesMissingFromFile()
+		{
+			var sources = new SortedSet<string>(StringComparer.OrdinalIgnoreCase);
+			var count = 0;
+			foreach (var pair in _origins)
+			{
+				if (string.Equals(pair.Value, _filename, StringComparison.OrdinalIgnoreCase))
+					continue;
+
+				sources.Add(pair.Value);
+				count++;
+			}
+
+			if (count == 0)
+				return;
+
+			_logger.LogError(string.Format(
+				"{0} is missing {1} GUID(s) still only held in {2}. Run without CheckOnly to write them, and commit the result, before deleting those files.",
+				_filename, count, string.Join(", ", sources)));
+		}
+
+		private void MarkDirty()
+		{
+			_dirty = true;
+		}
+
+		private void Flush()
+		{
+			if (_dirty) Write();
+			_dirty = false;
+		}
+
 		private void Write()
 		{
 			var settings = new XmlWriterSettings {
@@ -124,7 +227,9 @@ private void Write()
 				Encoding = Encoding.UTF8
 			};
 
-			using (var writer = XmlWriter.Create(_filename, settings))
+			var tempFilename = _filename + ".tmp";
+			var backupFilename = _filename + ".bak";
+			using (var writer = XmlWriter.Create(tempFilename, settings))
 			{
 				writer.WriteComment("This file is generated and then updated by an MSBuild task.  It preserves the automatically-generated guids assigned files that will be installed on user machines. So it should be held in source control.");
 				writer.WriteStartElement("InstallerMetadata");
@@ -137,6 +242,25 @@ private void Write()
 				}
 				writer.WriteEndElement(); // end InstallerMetadata
 			}
+			if (File.Exists(_filename))
+			{
+				File.Replace(tempFilename, _filename, backupFilename);
+				// If that didn't throw an exception, it's now safe to delete the backup file
+				// Catch and suppress any errors caused by deleting the backup file; if that ever happens it should not fail builds
+				try
+				{
+					File.Delete(backupFilename);
+				}
+				catch (Exception e)
+				{
+					_logger.LogMessage(MessageImportance.Normal, "Warning: deleting backup file " + backupFilename + " caused exception: " + e.Message);
+					_logger.LogMessage(MessageImportance.Normal, "Continuing, but this might be a sign of filesystem issues. Investigate if possible.");
+				}
+			}
+			else
+			{
+				File.Move(tempFilename, _filename);
+			}
 		}
 
 		#endregion

SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs

@@ -39,6 +39,7 @@ public class MakeWixForDirTree : Task, ILogger
 		private readonly Dictionary<string, int> _suffixes = new Dictionary<string, int>();
 		private readonly DateTime _refDate = DateTime.MinValue;
 		private bool _filesChanged;
+		private IdToGuidDatabase _sharedGuidDatabase;
 
 		private const string Xmlns = "http://schemas.microsoft.com/wix/2006/wi";
 
@@ -95,6 +96,23 @@ public string IgnoreRegExPattern
 		/// </summary>
 		public string InstallerSourceDirectory { get; set; }
 
+		/// <summary>
+		/// Optional path to a single file holding the GUIDs for the whole tree,
+		/// instead of a .guidsForInstaller.xml in every directory.
+		///
+		/// The per-directory default becomes unwieldy for large payloads: a tree
+		/// with a hundred directories needs a hundred files, all of which must be
+		/// kept in version control forever, because an entry has to outlive the
+		/// file it describes so that upgrades can still remove it.
+		///
+		/// On the first run with this set, any per-directory files still present
+		/// under RootDirectory are merged in, so existing GUIDs carry over
+		/// unchanged and installed components keep their identity. The old files
+		/// are left on disk; delete them from version control once the merge has
+		/// been verified. This file itself is never emitted as a component.
+		/// </summary>
+		public string ConsolidatedGuidFile { get; set; }
+
 		[Output, Required]
 		public string OutputFilePath { get; set; }
 
@@ -129,7 +147,9 @@ public override bool Execute()
 			*/
 			//instead, start afresh every time.
 
-			if(File.Exists(OutputFilePath))
+			// A check-only run outputs nothing, so it must not delete the file it would
+			// otherwise have regenerated.
+			if(!CheckOnly && File.Exists(OutputFilePath))
 			{
 				File.Delete(OutputFilePath);
 			}
@@ -138,6 +158,8 @@ public override bool Execute()
 
 			try
 			{
+				SetupConsolidatedGuidFile();
+
 				var doc = new XmlDocument();
 				var elemWix = doc.CreateElement("Wix", Xmlns);
 				doc.AppendChild(elemWix);
@@ -258,14 +280,64 @@ public void LogWarning(string s)
 			Log.LogWarning(s);
 		}
 
+		/// <summary>
+		/// Opens the consolidated GUID file, if one was asked for, and seeds it from
+		/// any per-directory files left over from before the switch.
+		/// </summary>
+		private void SetupConsolidatedGuidFile()
+		{
+			if (string.IsNullOrEmpty(ConsolidatedGuidFile))
+				return;
+
+			ConsolidatedGuidFile = Path.GetFullPath(ConsolidatedGuidFile);
+			_sharedGuidDatabase = IdToGuidDatabase.Create(ConsolidatedGuidFile, this);
+
+			foreach (var legacy in Directory.GetFiles(Path.GetFullPath(RootDirectory),
+				FileNameOfGuidDatabase, SearchOption.AllDirectories))
+			{
+				if (string.Equals(legacy, ConsolidatedGuidFile, StringComparison.OrdinalIgnoreCase))
+					continue;
+
+				LogMessage(MessageImportance.Normal,
+					"Merging GUIDs from {0} into {1}", legacy, ConsolidatedGuidFile);
+				_sharedGuidDatabase.ImportMissingFrom(IdToGuidDatabase.Create(legacy, this), CheckOnly);
+			}
+
+			// Ensure shared GUID database is fully written after importing, if needed
+			_sharedGuidDatabase.FinalizeImport(CheckOnly);
+
+			// The merge above satisfies every GetGuid lookup from memory, so without this
+			// a check-only run would report the metadata up-to-date while the consolidated
+			// file on disk is still empty or stale.
+			if (CheckOnly)
+				_sharedGuidDatabase.ReportEntriesMissingFromFile();
+		}
+
+		private IdToGuidDatabase GetGuidDatabaseFor(string dirPath)
+		{
+			return _sharedGuidDatabase
+				?? IdToGuidDatabase.Create(Path.Combine(dirPath, FileNameOfGuidDatabase), this);
+		}
+
+		private bool IsGuidDatabaseFile(string path)
+		{
+			if (path.Contains(FileNameOfGuidDatabase))
+				return true;
+
+			if (string.IsNullOrEmpty(ConsolidatedGuidFile))
+				return false;
+
+			return string.Equals(Path.GetFullPath(path), ConsolidatedGuidFile, StringComparison.OrdinalIgnoreCase);
+		}
+
 		private void ProcessDir(XmlNode parent, string dirPath, string outerDirectoryId)
 		{
 			LogMessage(MessageImportance.Low, "Processing dir {0}", dirPath);
 
 			var doc = parent.OwnerDocument;
 			var files = new List<string>();
 
-			var guidDatabase = IdToGuidDatabase.Create(Path.Combine(dirPath, FileNameOfGuidDatabase), this);
+			var guidDatabase = GetGuidDatabaseFor(dirPath);
 
 			SetupDirectoryPermissions(parent, outerDirectoryId, doc, guidDatabase);
 
@@ -274,7 +346,7 @@ private void ProcessDir(XmlNode parent, string dirPath, string outerDirectoryId)
 			foreach (var f in Directory.GetFiles(dirPath))
 			{
 				if (_fileMatchPattern.IsMatch(f) && !_ignoreFilePattern.IsMatch(f) && !_ignoreFilePattern.IsMatch(Path.GetFileName(f)) && !_exclude.ContainsKey(f.ToLower())
-					&& !f.Contains(FileNameOfGuidDatabase) )
+					&& !IsGuidDatabaseFile(f) )
 					files.Add(f);
 			}