Fix handling of duplicate directories

7 files changed, 92 insertions, 15 deletions

diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/AddRequiredStandardDirectories.cs b/src/WixToolset.Core.WindowsInstaller/Bind/AddRequiredStandardDirectories.cs
index 4597639b..ee3bcc91 100644
--- a/src/WixToolset.Core.WindowsInstaller/Bind/AddRequiredStandardDirectories.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Bind/AddRequiredStandardDirectories.cs
@@ -27,7 +27,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
         public void Execute()
         {
             var directories = this.Section.Symbols.OfType<DirectorySymbol>().ToList();
-            var directoriesById = directories.ToDictionary(d => d.Id.Id);
+            var directoryIds = new SortedSet<string>(directories.Select(d => d.Id.Id));
 
             foreach (var directory in directories)
             {
@@ -42,20 +42,20 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                 }
                 else
                 {
-                    this.EnsureStandardDirectoryAdded(directoriesById, parentDirectoryId, directory.SourceLineNumbers);
+                    this.EnsureStandardDirectoryAdded(directoryIds, parentDirectoryId, directory.SourceLineNumbers);
                 }
             }
 
-            if (!directoriesById.ContainsKey("TARGETDIR") && WindowsInstallerStandard.TryGetStandardDirectory("TARGETDIR", out var targetDir))
+            if (!directoryIds.Contains("TARGETDIR") && WindowsInstallerStandard.TryGetStandardDirectory("TARGETDIR", out var targetDir))
             {
-                directoriesById.Add(targetDir.Id.Id, targetDir);
+                directoryIds.Add(targetDir.Id.Id);
                 this.Section.AddSymbol(targetDir);
             }
         }
 
-        private void EnsureStandardDirectoryAdded(Dictionary<string, DirectorySymbol> directoriesById, string directoryId, SourceLineNumber sourceLineNumbers)
+        private void EnsureStandardDirectoryAdded(ISet<string> directoryIds, string directoryId, SourceLineNumber sourceLineNumbers)
         {
-            if (!directoriesById.ContainsKey(directoryId) && WindowsInstallerStandard.TryGetStandardDirectory(directoryId, out var standardDirectory))
+            if (!directoryIds.Contains(directoryId) && WindowsInstallerStandard.TryGetStandardDirectory(directoryId, out var standardDirectory))
             {
                 var parentDirectoryId = this.GetStandardDirectoryParent(directoryId);
 
@@ -65,12 +65,12 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                     ParentDirectoryRef = parentDirectoryId,
                 };
 
-                directoriesById.Add(directory.Id.Id, directory);
+                directoryIds.Add(directory.Id.Id);
                 this.Section.AddSymbol(directory);
 
                 if (!String.IsNullOrEmpty(parentDirectoryId))
                 {
-                    this.EnsureStandardDirectoryAdded(directoriesById, parentDirectoryId, sourceLineNumbers);
+                    this.EnsureStandardDirectoryAdded(directoryIds, parentDirectoryId, sourceLineNumbers);
                 }
             }
         }
diff --git a/src/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
index 1c2ca8eb..a4b2bee3 100644
--- a/src/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
+++ b/src/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
@@ -38,10 +38,17 @@ namespace WixToolset.Core.Link
         /// </summary>
         public IEnumerable<SymbolWithSection> PossibleConflicts { get; private set; }
 
+        /// <summary>
+        /// Gets the collection of redundant symbols that should not be included
+        /// in the final output.
+        /// </summary>
+        public ISet<IntermediateSymbol> RedundantSymbols { get; private set; }
+
         public void Execute()
         {
             var symbolsByName = new Dictionary<string, SymbolWithSection>();
             var possibleConflicts = new HashSet<SymbolWithSection>();
+            var redundantSymbols = new HashSet<IntermediateSymbol>();
 
             if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType))
             {
@@ -91,13 +98,13 @@ namespace WixToolset.Core.Link
                             // Ensure identical symbol's symbol is marked redundant to ensure (should the symbol be
                             // referenced into the final output) it will not add duplicate primary keys during
                             // the .IDT importing.
-                            //symbol.Row.Redundant = true; - TODO: remove this
                             existingSymbol.AddRedundant(symbolWithSection);
+                            redundantSymbols.Add(symbolWithSection.Symbol);
                         }
                         else
                         {
                             existingSymbol.AddPossibleConflict(symbolWithSection);
-                            possibleConflicts.Add(existingSymbol);
+                            possibleConflicts.Add(symbolWithSection);
                         }
                     }
                 }
@@ -105,6 +112,7 @@ namespace WixToolset.Core.Link
 
             this.SymbolsByName = symbolsByName;
             this.PossibleConflicts = possibleConflicts;
+            this.RedundantSymbols = redundantSymbols;
         }
     }
 }
diff --git a/src/WixToolset.Core/Link/IntermediateSymbolExtensions.cs b/src/WixToolset.Core/Link/IntermediateSymbolExtensions.cs
index db53f1ce..cbf48abe 100644
--- a/src/WixToolset.Core/Link/IntermediateSymbolExtensions.cs
+++ b/src/WixToolset.Core/Link/IntermediateSymbolExtensions.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
 
 namespace WixToolset.Core.Link
 {
@@ -9,10 +9,10 @@ namespace WixToolset.Core.Link
         public static bool IsIdentical(this IntermediateSymbol first, IntermediateSymbol second)
         {
             var identical = (first.Definition.Type == second.Definition.Type &&
-                             first.Definition.Name == second.Definition.Name &&
+                             (first.Definition.Type != SymbolDefinitionType.MustBeFromAnExtension || first.Definition.Name == second.Definition.Name) &&
                              first.Definition.FieldDefinitions.Length == second.Definition.FieldDefinitions.Length);
 
-            for (int i = 0; identical && i < first.Definition.FieldDefinitions.Length; ++i)
+            for (var i = 0; identical && i < first.Definition.FieldDefinitions.Length; ++i)
             {
                 var firstField = first[i];
                 var secondField = second[i];
diff --git a/src/WixToolset.Core/Link/ResolveReferencesCommand.cs b/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
index 90b61e8b..2bdd5646 100644
--- a/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
+++ b/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
@@ -170,9 +170,9 @@ namespace WixToolset.Core.Link
                 case AccessModifier.Public:
                     return true;
                 case AccessModifier.Internal:
-                    return symbolWithSection.Section.CompilationId.Equals(referencingSection.CompilationId) || (null != symbolWithSection.Section.LibraryId && symbolWithSection.Section.LibraryId.Equals(referencingSection.LibraryId));
+                    return symbolWithSection.Section.CompilationId == referencingSection.CompilationId || (null != symbolWithSection.Section.LibraryId && symbolWithSection.Section.LibraryId == referencingSection.LibraryId);
                 case AccessModifier.Protected:
-                    return symbolWithSection.Section.CompilationId.Equals(referencingSection.CompilationId);
+                    return symbolWithSection.Section.CompilationId == referencingSection.CompilationId;
                 case AccessModifier.Private:
                     return referencingSection == symbolWithSection.Section;
                 default:
diff --git a/src/WixToolset.Core/Linker.cs b/src/WixToolset.Core/Linker.cs
index e9f9554c..431ba4c7 100644
--- a/src/WixToolset.Core/Linker.cs
+++ b/src/WixToolset.Core/Linker.cs
@@ -226,6 +226,11 @@ namespace WixToolset.Core
 
                     foreach (var symbol in section.Symbols)
                     {
+                        if (find.RedundantSymbols.Contains(symbol))
+                        {
+                            continue;
+                        }
+
                         var copySymbol = true; // by default, copy symbols.
 
                         // handle special tables
diff --git a/src/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs b/src/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs
index 83f2f2bb..2d6e4802 100644
--- a/src/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs
@@ -85,5 +85,44 @@ namespace WixToolsetTest.CoreIntegration
                 }, dirSymbols.Select(d => d.Id.Id).ToArray());
             }
         }
+
+        [Fact]
+        public void CanGetDuplicateDir()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var msiPath = Path.Combine(baseFolder, @"bin\test.msi");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    "-arch", "x64",
+                    Path.Combine(folder, "DuplicateDir", "DuplicateDir.wxs"),
+                    Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
+                    "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", msiPath
+                });
+
+                result.AssertSuccess();
+
+                var intermediate = Intermediate.Load(Path.Combine(baseFolder, @"bin\test.wixpdb"));
+                var section = intermediate.Sections.Single();
+
+                var dirSymbols = section.Symbols.OfType<WixToolset.Data.Symbols.DirectorySymbol>().ToList();
+                Assert.Equal(new[]
+                {
+                    "dirZsSsu81KcG46xXTwc4mTSZO5Zx4",
+                    "INSTALLFOLDER",
+                    "ProgramFiles6432Folder",
+                    "ProgramFiles64Folder",
+                    "TARGETDIR"
+                }, dirSymbols.Select(d => d.Id.Id).ToArray());
+            }
+        }
     }
 }
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/DuplicateDir/DuplicateDir.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/DuplicateDir/DuplicateDir.wxs
new file mode 100644
index 00000000..ffee969d
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/DuplicateDir/DuplicateDir.wxs
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <ComponentGroup Id="ProductComponents">
+            <ComponentGroupRef Id="GroupA" />
+            <ComponentGroupRef Id="GroupB" />
+        </ComponentGroup>
+    </Fragment>
+
+    <Fragment>
+        <ComponentGroup Id="GroupA" Directory="INSTALLFOLDER:\dupe">
+            <Component>
+                <File Name="a.txt" Source="test.txt" />
+            </Component>
+        </ComponentGroup>
+    </Fragment>
+
+    <Fragment>
+        <ComponentGroup Id="GroupB" Directory="INSTALLFOLDER:\dupe">
+            <Component>
+                <File Name="b.txt" Source="test.txt" />
+            </Component>
+        </ComponentGroup>
+    </Fragment>
+</Wix>