Fix anonymous Directory Id generation to be consistent

Fixes 7861

Author: robmen

src/wix/WixToolset.Core.WindowsInstaller/Bind/CreateWindowsInstallerDataFromIRCommand.cs

@@ -62,7 +62,8 @@ public WindowsInstallerData Execute()
 
         private void AddSectionToData()
         {
-            var cellsByTableAndRowId = new Dictionary<string, List<WixCustomTableCellSymbol>>();
+            var directoryRowsById = new Dictionary<string, Row>(StringComparer.Ordinal);
+            var cellsByTableAndRowId = new Dictionary<string, List<WixCustomTableCellSymbol>>(StringComparer.Ordinal);
 
             foreach (var symbol in this.Section.Symbols)
             {
@@ -107,7 +108,7 @@ private void AddSectionToData()
                         break;
 
                     case SymbolDefinitionType.Directory:
-                        this.AddDirectorySymbol((DirectorySymbol)symbol);
+                        this.AddDirectorySymbol((DirectorySymbol)symbol, directoryRowsById);
                         break;
 
                     case SymbolDefinitionType.DuplicateFile:
@@ -497,9 +498,9 @@ private void AddDialogSymbol(DialogSymbol symbol)
             this.Data.EnsureTable(this.TableDefinitions["ListBox"]);
         }
 
-        private void AddDirectorySymbol(DirectorySymbol symbol)
+        private void AddDirectorySymbol(DirectorySymbol symbol, Dictionary<string, Row> directoryRowsById)
         {
-            (var name, var parentDir) = this.AddDirectorySubdirectories(symbol);
+            (var name, var parentDir) = this.AddDirectorySubdirectories(symbol, directoryRowsById);
 
             var shortName = symbol.ShortName;
             var sourceShortname = symbol.SourceShortName;
@@ -524,10 +525,7 @@ private void AddDirectorySymbol(DirectorySymbol symbol)
 
             var defaultDir = String.IsNullOrEmpty(sourceName) || sourceName == targetName ? targetName : targetName + ":" + sourceName;
 
-            var row = this.CreateRow(symbol, "Directory");
-            row[0] = symbol.Id.Id;
-            row[1] = parentDir;
-            row[2] = defaultDir;
+            this.CreateOrAddDirectoryRow(directoryRowsById, symbol, symbol.Id.Id, parentDir, defaultDir);
 
             if (OutputType.Module == this.Data.Type)
             {
@@ -1146,7 +1144,7 @@ private void AddWixActionSymbol(WixActionSymbol symbol)
                 }
                 else
                 {
-                    var after = (null == symbol.Before);
+                    var after = null == symbol.Before;
                     row[2] = after ? symbol.After : symbol.Before;
                     row[3] = after ? 1 : 0;
                 }
@@ -1271,7 +1269,7 @@ private void AddWixModuleSymbol(WixModuleSymbol symbol)
             }
         }
 
-        private void AddWixPackageSymbol(WixPackageSymbol symbol)
+        private void AddWixPackageSymbol(WixPackageSymbol _)
         {
             // TODO: Remove the following from the compiler and do it here instead.
             //this.AddProperty(sourceLineNumbers, new Identifier(AccessModifier.Global, "Manufacturer"), manufacturer, false, false, false, true);
@@ -1319,11 +1317,10 @@ private void EnsureModuleIgnoredTable(IntermediateSymbol symbol, string ignoredT
             }
         }
 
-        private (string, string) AddDirectorySubdirectories(DirectorySymbol symbol)
+        private (string, string) AddDirectorySubdirectories(DirectorySymbol symbol, Dictionary<string, Row> directoryRowsById)
         {
             var directory = symbol.Name.Trim(PathSeparatorChars);
             var parentDir = symbol.ParentDirectoryRef ?? (symbol.Id.Id == "TARGETDIR" ? null : "TARGETDIR");
-            var directoryRows = this.Data.TryGetTable("Directory", out var table) ? table.Rows.ToDictionary(row => row.FieldAsString(0)) : new Dictionary<string, Row>();
 
             var start = 0;
             var end = directory.IndexOfAny(PathSeparatorChars);
@@ -1337,18 +1334,11 @@ private void EnsureModuleIgnoredTable(IntermediateSymbol symbol, string ignoredT
                 {
                     path = Path.Combine(path, subdirectoryName);
 
-                    var id = this.BackendHelper.GenerateIdentifier("d", symbol.ParentDirectoryRef, path);
+                    var id = this.BackendHelper.GenerateIdentifier("d", symbol.ParentDirectoryRef, path, /*shortName:*/ null, /*sourceName:*/ null, /*shortSourceName:*/ null);
                     var shortnameSubdirectory = this.BackendHelper.IsValidShortFilename(subdirectoryName, false) ? null : this.CreateShortName(subdirectoryName, false, "Directory", symbol.ParentDirectoryRef);
+                    var defaultDir = CreateMsiFilename(shortnameSubdirectory, subdirectoryName);
 
-                    if (!directoryRows.ContainsKey(id))
-                    {
-                        var subdirectoryRow = this.CreateRow(symbol, "Directory");
-                        subdirectoryRow[0] = id;
-                        subdirectoryRow[1] = parentDir;
-                        subdirectoryRow[2] = CreateMsiFilename(shortnameSubdirectory, subdirectoryName);
-
-                        directoryRows.Add(id, subdirectoryRow);
-                    }
+                    this.CreateOrAddDirectoryRow(directoryRowsById, symbol, id, parentDir, defaultDir);
 
                     parentDir = id;
                 }
@@ -1362,6 +1352,25 @@ private void EnsureModuleIgnoredTable(IntermediateSymbol symbol, string ignoredT
             return (name, parentDir);
         }
 
+        private Row CreateOrAddDirectoryRow(Dictionary<string, Row> directoryRowsById, DirectorySymbol symbol, string id, string parentDir, string defaultDir)
+        {
+            if (!directoryRowsById.TryGetValue(id, out var directoryRow))
+            {
+                directoryRow = this.CreateRow(symbol, "Directory");
+                directoryRow[0] = id;
+                directoryRow[1] = parentDir;
+                directoryRow[2] = defaultDir;
+
+                directoryRowsById.Add(id, directoryRow);
+            }
+            else if (directoryRow.FieldAsString(1) != parentDir || directoryRow.FieldAsString(2) != defaultDir)
+            {
+                throw new WixException(WindowsInstallerBackendErrors.UnexpectedAnonymousDirectoryCollision(symbol.SourceLineNumbers, symbol.Id.Id, parentDir, defaultDir, directoryRow.SourceLineNumbers, directoryRow.FieldAsString(1), directoryRow.FieldAsString(2)));
+            }
+
+            return directoryRow;
+        }
+
         private void EnsureRequiredTables()
         {
             // check for missing table and add them or display an error as appropriate
@@ -1412,7 +1421,7 @@ private void ReportIllegalTables()
                             "Upgrade" == table.Name ||
                             "WixMerge" == table.Name)
                         {
-                            foreach (Row row in table.Rows)
+                            foreach (var row in table.Rows)
                             {
                                 this.Messaging.Write(ErrorMessages.UnexpectedTableInMergeModule(row.SourceLineNumbers, table.Name));
                             }
@@ -1609,8 +1618,10 @@ private string CreateShortName(string longName, bool keepExtension, params strin
             longName = longName.ToLowerInvariant();
 
             // collect all the data
-            var strings = new List<string>(1 + args.Length);
-            strings.Add(longName);
+            var strings = new List<string>(1 + args.Length)
+            {
+                longName
+            };
             strings.AddRange(args);
 
             // prepare for hashing
@@ -1625,8 +1636,10 @@ private string CreateShortName(string longName, bool keepExtension, params strin
             }
 
             // generate the short file/directory name without an extension
-            var shortName = new StringBuilder(Convert.ToBase64String(hash));
-            shortName.Length = 8;
+            var shortName = new StringBuilder(Convert.ToBase64String(hash))
+            {
+                Length = 8
+            };
             shortName.Replace('+', '-').Replace('/', '_');
 
             if (keepExtension)

src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendErrors.cs

@@ -39,6 +39,11 @@ public static Message InvalidWindowsInstallerWixpdbForValidation(string wixpdbPa
             return Message(null, Ids.InvalidWindowsInstallerWixpdbForValidation, "The validation .wixpdb file: {0} was not from a Windows Installer database build (.msi or .msm). Verify that the output type was actually an MSI Package or Merge Module.", wixpdbPath);
         }
 
+        public static Message UnexpectedAnonymousDirectoryCollision(SourceLineNumber sourceLineNumbers, string id, string parentDir, string defaultDir, SourceLineNumber existingSourceLineNumbers, string existingParentDir, string existingDefaultDir)
+        {
+            return Message(sourceLineNumbers, Ids.UnexpectedAnonymousDirectoryCollision, "This should not happen. The first directory id '{0}' uses parent directory '{1}' with DefaultDir '{2}'. The colliding directory uses parent directory '{3}' with DefaultDir '{4}' from line: {5}", id, parentDir, defaultDir, existingParentDir, existingDefaultDir, existingSourceLineNumbers.ToString());
+        }
+
         public static Message UnknownDecompileType(string decompileType, string filePath)
         {
             return Message(null, Ids.UnknownDecompileType, "Unknown decompile type '{0}' from input: {1}", decompileType, filePath);
@@ -65,6 +70,7 @@ public enum Ids
             UnknownDecompileType = 7504,
             UnknownValidationTargetFileExtension = 7505,
             InvalidWindowsInstallerWixpdbForValidation = 7506,
+            UnexpectedAnonymousDirectoryCollision = 7507,
         } // last available is 7999. 8000 is BurnBackendErrors.
     }
 }

src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs

@@ -162,6 +162,45 @@ public void CanGet64bitProgramFiles6432Folder()
             }
         }
 
+        [Fact]
+        public void CanGetAnonymousDirectories()
+        {
+            var folder = TestData.Get("TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var wixlibPath = Path.Combine(baseFolder, "bin", "test.msi");
+
+                var result = WixRunner.Execute(
+                [
+                    "build",
+                    "-arch", "x64",
+                    Path.Combine(folder, "Directory", "AnonymousDirectories.wxs"),
+                    "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", wixlibPath
+                ]);
+
+                result.AssertSuccess();
+
+                var pdb = WindowsInstallerData.Load(Path.Combine(baseFolder, "bin", "test.wixpdb"));
+                var directoryRows = pdb.Tables["Directory"].Rows;
+
+                var dirs = directoryRows.Select(d => d.ToString()).OrderBy(s => s).ToArray();
+                WixAssert.CompareLineByLine(
+                [
+                    @"DesktopFolder/TARGETDIR/Desktop",
+                    @"dHKac23vLoBC5fFrqxqAIybMFOj0/DesktopFolder/a",
+                    @"dKom2ks4onBH9RsYLDhjge71s7s8/dlvpu0ovv8DWWVrdCVSYzDfsibzg/c",
+                    @"dlvpu0ovv8DWWVrdCVSYzDfsibzg/dHKac23vLoBC5fFrqxqAIybMFOj0/b",
+                    @"SomeFolder/TARGETDIR/fef2brvc|Some Folder",
+                    @"TARGETDIR//SourceDir"
+                ], dirs);
+            }
+        }
+
         [Fact]
         public void CanGetDefaultName()
         {
@@ -293,9 +332,9 @@ public void CanGetWithMultiNestedSubdirectory()
                 var directoryRows = data.Tables["Directory"].Rows;
                 WixAssert.CompareLineByLine(new[]
                 {
-                    "d4EceYatXTyy8HXPt5B6DT9Rj.wE:ProgramFilesFolder:u7-b4gch|Example Corporation",
-                    "dSJ1pgiASlW7kJTu0wqsGBklJsS0:d4EceYatXTyy8HXPt5B6DT9Rj.wE:vjj-gxay|Test Product",
-                    "BinFolder:dSJ1pgiASlW7kJTu0wqsGBklJsS0:bin",
+                    "dwGveZhe5wcMbbRyRAkRwm2sqnE4:ProgramFilesFolder:u7-b4gch|Example Corporation",
+                    "d8kPFuRMPxdOxfpYS0O8azlhLUpY:dwGveZhe5wcMbbRyRAkRwm2sqnE4:vjj-gxay|Test Product",
+                    "BinFolder:d8kPFuRMPxdOxfpYS0O8azlhLUpY:bin",
                     "ProgramFilesFolder:TARGETDIR:PFiles",
                     "TARGETDIR::SourceDir"
                 }, directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(1) + ":" + r.FieldAsString(2)).ToArray());
@@ -386,9 +425,9 @@ public void CanFindRedundantSubdirectoryInSecondSection()
                 var directoryRows = data.Tables["Directory"].Rows;
                 WixAssert.CompareLineByLine(new[]
                 {
-                    @"d1nVb5_zcCwRCz7i2YXNAofGRmfc:ProgramFilesFolder:a",
-                    @"dijlG.bNicFgvj1_DujiGg9EBGrQ:d1nVb5_zcCwRCz7i2YXNAofGRmfc:b",
-                    @"dKO7wPCF.XLmq6KnqybHHgcBBqtU:dijlG.bNicFgvj1_DujiGg9EBGrQ:c",
+                    @"dKO7wPCF.XLmq6KnqybHHgcBBqtU:dvFwapipzsdDBjHbM5DUi_2llN.k:c",
+                    @"dvFwapipzsdDBjHbM5DUi_2llN.k:dyJuT.V6E4sWuHVv3CZiBNvrX2Lo:b",
+                    @"dyJuT.V6E4sWuHVv3CZiBNvrX2Lo:ProgramFilesFolder:a",
                     "ProgramFilesFolder:TARGETDIR:PFiles",
                     "TARGETDIR::SourceDir"
                 }, directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(1) + ":" + r.FieldAsString(2)).OrderBy(s => s).ToArray());

src/wix/test/WixToolsetTest.CoreIntegration/LanguageFixture.cs

@@ -56,13 +56,13 @@ public void CanBuildWithDefaultProductLanguage()
 
                 var data = WindowsInstallerData.Load(Path.Combine(baseFolder, @"bin\test.wixpdb"));
                 var directoryRows = data.Tables["Directory"].Rows;
-                WixAssert.CompareLineByLine(new[]
-                {
-                    "d4EceYatXTyy8HXPt5B6DT9Rj.wE:u7-b4gch|Example Corporation",
-                    "INSTALLFOLDER:oekcr5lq|MsiPackage",
-                    "ProgramFilesFolder:PFiles",
-                    "TARGETDIR:SourceDir"
-                }, directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(2)).ToArray());
+                WixAssert.CompareLineByLine(
+                [
+                    "dwGveZhe5wcMbbRyRAkRwm2sqnE4:ProgramFilesFolder:u7-b4gch|Example Corporation",
+                    "INSTALLFOLDER:dwGveZhe5wcMbbRyRAkRwm2sqnE4:oekcr5lq|MsiPackage",
+                    "ProgramFilesFolder:TARGETDIR:PFiles",
+                    "TARGETDIR::SourceDir"
+                ], [.. directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(1) + ":" + r.FieldAsString(2))]);
             }
         }
 

src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/AnonymousDirectories.wxs

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Package Id="WixToolsetTest.TestPackage" Name="~RedundantSubdirectories" Version="1.0.0.0" Manufacturer="Example Corporation" Compressed="no">
+        <ComponentGroupRef Id="Files" />
+
+        <Directory Id="SomeFolder" Name="Some Folder" />
+    </Package>
+
+    <Fragment>
+        <ComponentGroup Id="Files">
+            <Component Directory="SomeFolder">
+                <File Source="test.txt" KeyPath="yes">
+                    <Shortcut Id="Shortcut1" Name="Test Shortcut" Directory="DesktopFolder" Subdirectory="a\b\c" Advertise="yes" />
+                </File>
+
+                <RemoveFolder Id="RemoveAFolder" Directory="DesktopFolder" Subdirectory="a" On="uninstall" />
+                <RemoveFolder Id="RemoveBFolder" Directory="DesktopFolder" Subdirectory="a\b" On="uninstall" />
+                <RemoveFolder Id="RemoveCFolder" Directory="DesktopFolder" Subdirectory="a\b\c" On="uninstall" />
+            </Component>
+        </ComponentGroup>
+    </Fragment>
+</Wix>