From 33bf502ea31a1eb0d1670b7bda02d9abde3be6a5 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 19 Aug 2026 11:24:58 +0700 Subject: [PATCH 01/12] Consolidate many guidsForInstaller files into one Code generated by Claude Code, checked and improved by Robin Munn. In the process of updating Mercurial4Chorus, we seem likely to end up with a hundred .guidsForInstaller.xml files scattered around the win/ directory tree. This is unwieldy, and it would be better if there could be a single file at the root of the directory tree containing all the GUIDs. The implementation keeps the existing .guidsForInstaller.xml file rather than deleting them; deleting them can be done manually once they are actually redundant. This allows a gradual move to a consolidated GUID file by first setting the ConsolidatedGuidFile property, then using it in a build, and finally committing the consolidated file and deleting the now-redundant scattered files. --- .../MakeWixForDirTree/IdToGuidDatabase.cs | 34 ++++++++++ .../MakeWixForDirTree/MakeWixForDirTree.cs | 64 ++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 055b159a..b1d23fd6 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -116,6 +116,40 @@ public string GetGuid(string id, bool justCheckDontCreate) return guid?.ToUpper(); } + /// + /// 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 whole relative path (for example + /// "mercurial.lib.dulwich._pack.pyd"), so they are unique across the tree + /// and the existing GUIDs can be merged without renaming anything. + /// + public void ImportMissingFrom(IdToGuidDatabase other) + { + if (other == null || ReferenceEquals(other, this)) + return; + + var added = false; + foreach (var pair in other._guids) + { + var existing = this[pair.Key]; + if (existing == null) + { + this[pair.Key] = pair.Value; + 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, _filename, pair.Value, other._filename)); + } + } + + if (added) Write(); + } + private void Write() { var settings = new XmlWriterSettings { diff --git a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs index 11f02593..882cb326 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs @@ -39,6 +39,7 @@ public class MakeWixForDirTree : Task, ILogger private readonly Dictionary _suffixes = new Dictionary(); 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 /// public string InstallerSourceDirectory { get; set; } + /// + /// 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. + /// + public string ConsolidatedGuidFile { get; set; } + [Output, Required] public string OutputFilePath { get; set; } @@ -135,6 +153,7 @@ public override bool Execute() } SetupExclusions(); + SetupConsolidatedGuidFile(); try { @@ -258,6 +277,47 @@ public void LogWarning(string s) Log.LogWarning(s); } + /// + /// Opens the consolidated GUID file, if one was asked for, and seeds it from + /// any per-directory files left over from before the switch. + /// + 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)); + } + } + + 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); @@ -265,7 +325,7 @@ private void ProcessDir(XmlNode parent, string dirPath, string outerDirectoryId) var doc = parent.OwnerDocument; var files = new List(); - var guidDatabase = IdToGuidDatabase.Create(Path.Combine(dirPath, FileNameOfGuidDatabase), this); + var guidDatabase = GetGuidDatabaseFor(dirPath); SetupDirectoryPermissions(parent, outerDirectoryId, doc, guidDatabase); @@ -274,7 +334,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); } From 474c53e6e8ff825d27d70b1c5796fae382f08f11 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Fri, 28 Aug 2026 08:27:49 +0700 Subject: [PATCH 02/12] Consolidated GUID code now honors CheckOnly flag This fixes the bug where running in CheckOnly mode would have still written to the consolidated database, if one was specified. Now the code that consolidates the GUIDs will still consolidate them into an in-memory database in CheckOnly mode (so that GetGuid can succeed) but will not write to disk when CheckOnly is true. --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 6 ++++-- SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index b1d23fd6..5d6f5cd3 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -122,8 +122,10 @@ public string GetGuid(string id, bool justCheckDontCreate) /// single one: File Ids encode the whole relative path (for example /// "mercurial.lib.dulwich._pack.pyd"), so they are unique across the tree /// and the existing GUIDs can 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. /// - public void ImportMissingFrom(IdToGuidDatabase other) + public void ImportMissingFrom(IdToGuidDatabase other, bool justCheckDontCreate) { if (other == null || ReferenceEquals(other, this)) return; @@ -147,7 +149,7 @@ public void ImportMissingFrom(IdToGuidDatabase other) } } - if (added) Write(); + if (added && !justCheckDontCreate) Write(); } private void Write() diff --git a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs index 882cb326..bc2a1e90 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs @@ -297,7 +297,7 @@ private void SetupConsolidatedGuidFile() LogMessage(MessageImportance.Normal, "Merging GUIDs from {0} into {1}", legacy, ConsolidatedGuidFile); - _sharedGuidDatabase.ImportMissingFrom(IdToGuidDatabase.Create(legacy, this)); + _sharedGuidDatabase.ImportMissingFrom(IdToGuidDatabase.Create(legacy, this), CheckOnly); } } From 28ab16eb40de669635352125bec46acf2421bc36 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Fri, 28 Aug 2026 11:44:42 +0700 Subject: [PATCH 03/12] Track GUID origin correctly, fix CheckOnly Two bugfixes for MakeWixForDirTree: - ImportMissingFrom now reports which .guidsForInstaller.xml file the missing GUID originally came from - CheckOnly now verifies that the consolidated GUID file actually has the GUIDs; previously it would have passed in consolidated-file mode even if the consolidated file was empty and the GUIDs were still in the individual .guidsForInstaller.xml files. Finally, one bugfix for a bug that pre-dated the PR: using CheckOnly would have deleted the generated .wxs files, because the deletion happened every time but the .wxs file was only written if CheckOnly was false. Now the deletion is also skipped in CheckOnly mode. Code written by Claude, then verbosity slightly toned down by Robin Munn. Commit message by Robin Munn because Claude was WAY too wordy. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 8 +++ .../MakeWixForDirTree/IdToGuidDatabase.cs | 53 +++++++++++++++++-- .../MakeWixForDirTree/MakeWixForDirTree.cs | 10 +++- 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8965af96..b465d436 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 5d6f5cd3..e6b1616c 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -21,6 +21,9 @@ internal class IdToGuidDatabase private readonly ILogger _logger; private readonly string _filename; private readonly Dictionary _guids = new Dictionary(); + // 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 _origins = new Dictionary(); #region Construction @@ -63,7 +66,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 +90,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,7 +123,7 @@ 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); Write(); } @@ -136,7 +150,7 @@ public void ImportMissingFrom(IdToGuidDatabase other, bool justCheckDontCreate) var existing = this[pair.Key]; if (existing == null) { - this[pair.Key] = pair.Value; + Set(pair.Key, pair.Value, other._filename); added = true; } else if (!string.Equals(existing, pair.Value, StringComparison.OrdinalIgnoreCase)) @@ -145,13 +159,42 @@ public void ImportMissingFrom(IdToGuidDatabase other, bool justCheckDontCreate) // 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, _filename, pair.Value, other._filename)); + pair.Key, existing, OriginOf(pair.Key), pair.Value, other._filename)); } } if (added && !justCheckDontCreate) Write(); } + /// + /// 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. + /// + public void ReportEntriesMissingFromFile() + { + var sources = new SortedSet(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 Write() { var settings = new XmlWriterSettings { diff --git a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs index bc2a1e90..ed17ac9d 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs @@ -147,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); } @@ -299,6 +301,12 @@ private void SetupConsolidatedGuidFile() "Merging GUIDs from {0} into {1}", legacy, ConsolidatedGuidFile); _sharedGuidDatabase.ImportMissingFrom(IdToGuidDatabase.Create(legacy, this), 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) From a3422fdcd334419f940e77ca49a680a7e3a7d8a0 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 31 Aug 2026 14:57:16 +0700 Subject: [PATCH 04/12] Document truncated file IDs in comment Comment was misleading before --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index e6b1616c..9740c552 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -133,9 +133,11 @@ public string GetGuid(string id, bool justCheckDontCreate) /// /// 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 whole relative path (for example - /// "mercurial.lib.dulwich._pack.pyd"), so they are unique across the tree - /// and the existing GUIDs can be merged without renaming anything. + /// 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. /// From 1e82cf154a26d72fc1070c30f2cfab6424bb67de Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 31 Aug 2026 14:58:23 +0700 Subject: [PATCH 05/12] Speed up first import of consolidated GUID files Instead of writing a full copy of the consolidated GUID file after every legacy file is imported, we now mark the consolidated file as dirty if anything new was imported, then write the whole thing once after the full import finishes. The GetGuid() method, though, still writes the whole file after any new GUIDs are assigned, because otherwise we could end up losing data if the task crashes at just the wrong moment. Writing the XML files also becomes safe: a new file is written and then the old one is replaced atomically. --- .../MakeWixForDirTree/IdToGuidDatabase.cs | 39 ++++++++++++++++--- .../MakeWixForDirTree/MakeWixForDirTree.cs | 6 ++- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 9740c552..1a509bd7 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -24,28 +24,32 @@ internal class IdToGuidDatabase // 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 _origins = new Dictionary(); + // Whether the GUID database has new data needing to be written + private readonly 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 @@ -124,6 +128,8 @@ public string GetGuid(string id, bool justCheckDontCreate) _logger.LogMessage(MessageImportance.Low, "No GUID for " + id + " in " + _filename); guid = Guid.NewGuid().ToString(); 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(); } @@ -165,7 +171,12 @@ public void ImportMissingFrom(IdToGuidDatabase other, bool justCheckDontCreate) } } - if (added && !justCheckDontCreate) Write(); + if (added && !justCheckDontCreate) MarkDirty(); + } + + public void FinalizeImport(bool justCheckDontCreate) + { + if (!justCheckDontCreate) Flush(); } /// @@ -197,6 +208,17 @@ public void ReportEntriesMissingFromFile() _filename, count, string.Join(", ", sources))); } + private void MarkDirty() + { + _dirty = true; + } + + public void Flush() + { + if (_dirty) Write(); + _dirty = false; + } + private void Write() { var settings = new XmlWriterSettings { @@ -205,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"); @@ -218,6 +242,9 @@ private void Write() } writer.WriteEndElement(); // end InstallerMetadata } + File.Replace(tempFilename, _filename, backupFilename); + // If that didn't throw an exception, it's now safe to delete the backup file + File.Delete(backupFilename); } #endregion diff --git a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs index ed17ac9d..9a8b91d0 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/MakeWixForDirTree.cs @@ -155,10 +155,11 @@ public override bool Execute() } SetupExclusions(); - SetupConsolidatedGuidFile(); try { + SetupConsolidatedGuidFile(); + var doc = new XmlDocument(); var elemWix = doc.CreateElement("Wix", Xmlns); doc.AppendChild(elemWix); @@ -302,6 +303,9 @@ private void SetupConsolidatedGuidFile() _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. From 3462508e76c298a1db054cdd0fe4728e7b40cf69 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 31 Aug 2026 15:13:47 +0700 Subject: [PATCH 06/12] Dirty flag should not be readonly Copy-and-paste error --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 1a509bd7..662d5ef6 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -25,7 +25,7 @@ internal class IdToGuidDatabase // ImportMissingFrom keep pointing at the file that actually supplied them. private readonly Dictionary _origins = new Dictionary(); // Whether the GUID database has new data needing to be written - private readonly bool _dirty; + private bool _dirty; #region Construction From 6d518d7ea96468c2f9d8191819289f1e3a4cf692 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 31 Aug 2026 15:35:01 +0700 Subject: [PATCH 07/12] Don't let backup file deletion fail a build --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 662d5ef6..ed5b12a1 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -244,7 +244,8 @@ private void Write() } File.Replace(tempFilename, _filename, backupFilename); // If that didn't throw an exception, it's now safe to delete the backup file - File.Delete(backupFilename); + // 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 { } } #endregion From 1d608fb5a2c3cda2741acd6f32435a6e5f112a31 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 31 Aug 2026 15:40:36 +0700 Subject: [PATCH 08/12] Don't throw if _filename doesn't exist yet This would all be much simpler if we were on .NET Core 3.0 or later, but for .NET Framework we have to do a complicated dance to replicate what would be a simple `mv tmpfile.xml realfile.xml` on Linux. --- .../MakeWixForDirTree/IdToGuidDatabase.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index ed5b12a1..768943db 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -242,10 +242,17 @@ private void Write() } writer.WriteEndElement(); // end InstallerMetadata } - 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 { } + 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 { } + } + else + { + File.Move(tempFilename, _filename); + } } #endregion From ecc87605ac9dec8f06a4f832ad73dc5b2fd861e4 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 1 Sep 2026 09:13:36 +0700 Subject: [PATCH 09/12] No reason for Flush() to be public --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 768943db..17bb0b90 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -213,7 +213,7 @@ private void MarkDirty() _dirty = true; } - public void Flush() + private void Flush() { if (_dirty) Write(); _dirty = false; From c598178f5174caf24e30de6088256adbd3ec6bc4 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 2 Sep 2026 10:05:22 +0700 Subject: [PATCH 10/12] Add warning message if backup deletion fails Then continue the build, because that's not a fatal error. Though it might be a sign of filesystem issues that will eventually cause the build to fail. --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index 17bb0b90..ed1ebb9b 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -247,7 +247,15 @@ private void Write() 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 { } + try + { + File.Delete(backupFilename); + } + catch (e) + { + _logger.LogMessage("Warning: deleting backup file " + backupFilename + " caused exception: " + e.Message); + _logger.LogMessage("Continuing, but this might be a sign of filesystem issues. Investigate if possible."); + } } else { From 730d090ab012411b2684dfd275906eda166d68bf Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 2 Sep 2026 10:08:09 +0700 Subject: [PATCH 11/12] Proper catch syntax --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index ed1ebb9b..dfbeb37b 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -251,7 +251,7 @@ private void Write() { File.Delete(backupFilename); } - catch (e) + catch (Exception e) { _logger.LogMessage("Warning: deleting backup file " + backupFilename + " caused exception: " + e.Message); _logger.LogMessage("Continuing, but this might be a sign of filesystem issues. Investigate if possible."); From 3fd8622d25d9f04a03e902863affdede91dd857e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 2 Sep 2026 10:11:18 +0700 Subject: [PATCH 12/12] Another syntax error fixed --- SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs index dfbeb37b..0622830b 100644 --- a/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs +++ b/SIL.BuildTasks/MakeWixForDirTree/IdToGuidDatabase.cs @@ -253,8 +253,8 @@ private void Write() } catch (Exception e) { - _logger.LogMessage("Warning: deleting backup file " + backupFilename + " caused exception: " + e.Message); - _logger.LogMessage("Continuing, but this might be a sign of filesystem issues. Investigate if possible."); + _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