diff --git a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs index 908476db5..496c61783 100644 --- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs @@ -42,13 +42,13 @@ public FindEntrySectionAndLoadSymbolsCommand(IMessaging messaging, IEnumerable - public ISet RedundantSymbols { get; private set; } + public ISet IdenticalDirectorySymbols { get; private set; } public void Execute() { var symbolsByName = new Dictionary(); var possibleConflicts = new HashSet(); - var redundantSymbols = new HashSet(); + var identicalDirectorySymbols = new HashSet(); if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType)) { @@ -76,28 +76,27 @@ public void Execute() } } - // Load all the symbols from the section's tables that create symbols. + // Load all the symbols from the section's tables that can be referenced (i.e. have an Id). foreach (var symbol in section.Symbols.Where(t => t.Id != null)) { var symbolWithSection = new SymbolWithSection(section, symbol); + var fullName = symbolWithSection.GetFullName(); - if (!symbolsByName.TryGetValue(symbolWithSection.Name, out var existingSymbol)) + if (!symbolsByName.TryGetValue(fullName, out var existingSymbol)) { - symbolsByName.Add(symbolWithSection.Name, symbolWithSection); + symbolsByName.Add(fullName, symbolWithSection); } else // uh-oh, duplicate symbols. { // If the duplicate symbols are both private directories, there is a chance that they - // point to identical symbols. Identical directory symbols are redundant and will not cause - // conflicts. + // point to identical symbols. Identical directory symbols should be treated as redundant. + // and not cause conflicts. if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access && SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol)) { - // 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. - existingSymbol.AddRedundant(symbolWithSection); - redundantSymbols.Add(symbolWithSection.Symbol); + // Ensure identical symbols are tracked to ensure that only one symbol will end up in linked intermediate. + identicalDirectorySymbols.Add(existingSymbol.Symbol); + identicalDirectorySymbols.Add(symbolWithSection.Symbol); } else { @@ -111,7 +110,7 @@ public void Execute() this.SymbolsByName = symbolsByName; this.PossibleConflicts = possibleConflicts; - this.RedundantSymbols = redundantSymbols; + this.IdenticalDirectorySymbols = identicalDirectorySymbols; } } } diff --git a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs index ace2e19df..3a07d3662 100644 --- a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs @@ -40,7 +40,9 @@ public void Execute() if (actuallyReferencedDuplicates.Any()) { - this.Messaging.Write(ErrorMessages.DuplicateSymbol(referencedDuplicate.Symbol.SourceLineNumbers, referencedDuplicate.Name)); + var fullName = referencedDuplicate.GetFullName(); + + this.Messaging.Write(ErrorMessages.DuplicateSymbol(referencedDuplicate.Symbol.SourceLineNumbers, fullName)); foreach (var duplicate in actuallyReferencedDuplicates) { diff --git a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs index efb90bb85..d95d648f2 100644 --- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs +++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs @@ -91,15 +91,16 @@ private void RecursivelyResolveReferences(IntermediateSection section) else // display errors for the duplicate symbols. { var accessibleSymbol = accessible[0]; + var accessibleFullName = accessibleSymbol.GetFullName(); var referencingSourceLineNumber = wixSimpleReferenceRow.SourceLineNumbers?.ToString(); if (String.IsNullOrEmpty(referencingSourceLineNumber)) { - this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleSymbol.Name)); + this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName)); } else { - this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleSymbol.Name, referencingSourceLineNumber)); + this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName, referencingSourceLineNumber)); } foreach (var accessibleDuplicate in accessible.Skip(1)) @@ -146,14 +147,6 @@ private List DetermineAccessibleSymbols(IntermediateSection r } } - foreach (var dupe in symbolWithSection.Redundants) - { - if (this.AccessibleSymbol(referencingSection, dupe)) - { - accessibleSymbols.Add(dupe); - } - } - return accessibleSymbols; } diff --git a/src/wix/WixToolset.Core/Link/SymbolWithSection.cs b/src/wix/WixToolset.Core/Link/SymbolWithSection.cs index 08e01077a..dbaceb283 100644 --- a/src/wix/WixToolset.Core/Link/SymbolWithSection.cs +++ b/src/wix/WixToolset.Core/Link/SymbolWithSection.cs @@ -13,7 +13,6 @@ namespace WixToolset.Core.Link internal class SymbolWithSection { private HashSet possibleConflicts; - private HashSet redundants; /// /// Creates a symbol for a symbol. @@ -24,7 +23,6 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol) { this.Symbol = symbol; this.Section = section; - this.Name = String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id); } /// @@ -33,12 +31,6 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol) /// Accessbility of the symbol. public AccessModifier Access => this.Symbol.Id.Access; - /// - /// Gets the name of the symbol. - /// - /// Name of the symbol. - public string Name { get; } - /// /// Gets the symbol for this symbol. /// @@ -56,11 +48,6 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol) /// public IEnumerable PossiblyConflicts => this.possibleConflicts ?? Enumerable.Empty(); - /// - /// Gets any duplicates of this symbol with sections that are redundant. - /// - public IEnumerable Redundants => this.redundants ?? Enumerable.Empty(); - /// /// Adds a duplicate symbol with sections that is a possible conflict. /// @@ -76,17 +63,11 @@ public void AddPossibleConflict(SymbolWithSection symbolWithSection) } /// - /// Adds a duplicate symbol that is redundant. + /// Gets the full name of the symbol. /// - /// Symbol with section that is redundant of this symbol. - public void AddRedundant(SymbolWithSection symbolWithSection) + public string GetFullName() { - if (null == this.redundants) - { - this.redundants = new HashSet(); - } - - this.redundants.Add(symbolWithSection); + return String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id); } } } diff --git a/src/wix/WixToolset.Core/Linker.cs b/src/wix/WixToolset.Core/Linker.cs index a3d990392..5c021ca0f 100644 --- a/src/wix/WixToolset.Core/Linker.cs +++ b/src/wix/WixToolset.Core/Linker.cs @@ -177,14 +177,20 @@ public Intermediate Link(ILinkContext context) // metadata. var resolvedSection = new IntermediateSection(find.EntrySection.Id, find.EntrySection.Type); var references = new List(); + var identicalDirectoryIds = new HashSet(StringComparer.Ordinal); foreach (var section in sections) { foreach (var symbol in section.Symbols) { - if (find.RedundantSymbols.Contains(symbol)) + // If this symbol is an identical directory, ensure we only visit + // one (and skip the other identicals with the same id). + if (find.IdenticalDirectorySymbols.Contains(symbol)) { - continue; + if (!identicalDirectoryIds.Add(symbol.Id.Id)) + { + continue; + } } var copySymbol = true; // by default, copy symbols. @@ -351,22 +357,24 @@ private void LoadStandardSymbols(IDictionary symbolsB foreach (var actionSymbol in WindowsInstallerStandard.StandardActions()) { var symbolWithSection = new SymbolWithSection(null, actionSymbol); + var fullName = symbolWithSection.GetFullName(); // If the action's symbol has not already been defined (i.e. overriden by the user), add it now. - if (!symbolsByName.ContainsKey(symbolWithSection.Name)) + if (!symbolsByName.ContainsKey(fullName)) { - symbolsByName.Add(symbolWithSection.Name, symbolWithSection); + symbolsByName.Add(fullName, symbolWithSection); } } foreach (var directorySymbol in WindowsInstallerStandard.StandardDirectories()) { var symbolWithSection = new SymbolWithSection(null, directorySymbol); + var fullName = symbolWithSection.GetFullName(); // If the directory's symbol has not already been defined (i.e. overriden by the user), add it now. - if (!symbolsByName.ContainsKey(symbolWithSection.Name)) + if (!symbolsByName.ContainsKey(fullName)) { - symbolsByName.Add(symbolWithSection.Name, symbolWithSection); + symbolsByName.Add(fullName, symbolWithSection); } } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs index cacf72bdc..3f4108cbb 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs @@ -273,5 +273,52 @@ public void CanGetDuplicateTargetSourceName() }, directoryRows.Select(r => String.Join('\t', r.FieldAsString(0), r.FieldAsString(1), r.FieldAsString(2))).ToArray()); } } + + [Fact] + public void CanFindRedundantSubdirectoryInSecondSection() + { + 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 wixpdbPath = Path.ChangeExtension(msiPath, ".wixpdb"); + + var result = WixRunner.Execute(new[] + { + "build", + Path.Combine(folder, "Directory", "RedundantSubdirectoryInSecondSection.wxs"), + "-bindpath", Path.Combine(folder, "SingleFile", "data"), + "-intermediateFolder", intermediateFolder, + "-o", msiPath + }); + + result.AssertSuccess(); + + var intermediate = Intermediate.Load(wixpdbPath); + var section = intermediate.Sections.Single(); + + var dirSymbols = section.Symbols.OfType().ToList(); + WixAssert.CompareLineByLine(new[] + { + @"dKO7wPCF.XLmq6KnqybHHgcBBqtU:ProgramFilesFolder:a\b\c", + @"ProgramFilesFolder:TARGETDIR:PFiles", + @"TARGETDIR::SourceDir" + }, dirSymbols.OrderBy(d => d.Id.Id).Select(d => d.Id.Id + ":" + d.ParentDirectoryRef + ":" + d.Name).OrderBy(s => s).ToArray()); + + var data = WindowsInstallerData.Load(wixpdbPath); + 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", + "ProgramFilesFolder:TARGETDIR:PFiles", + "TARGETDIR::SourceDir" + }, directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(1) + ":" + r.FieldAsString(2)).OrderBy(s => s).ToArray()); + } + } } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs new file mode 100644 index 000000000..fc73ee474 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + +