From 86774dad361714fdd55a2df23379c1a1600d1f27 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Mon, 6 Nov 2023 21:46:30 -0800 Subject: Include duplicated inline directory symbols referenced in subsequent sections Due to the handling of redundant symbols, which are only used by inline directory syntax, the symbols were only defined in the first section encountered by the linker. Fix that so at most one duplicated inline directory symbol is included when referenced. Fixes 7840 --- .../Link/FindEntrySectionAndLoadSymbolsCommand.cs | 25 ++++++------ .../Link/ReportConflictingSymbolsCommand.cs | 4 +- .../Link/ResolveReferencesCommand.cs | 13 ++---- src/wix/WixToolset.Core/Link/SymbolWithSection.cs | 25 ++---------- src/wix/WixToolset.Core/Linker.cs | 20 ++++++--- .../DirectoryFixture.cs | 47 ++++++++++++++++++++++ .../RedundantSubdirectoryInSecondSection.wxs | 26 ++++++++++++ 7 files changed, 108 insertions(+), 52 deletions(-) create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs diff --git a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs index 908476db..496c6178 100644 --- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs @@ -42,13 +42,13 @@ namespace WixToolset.Core.Link /// Gets the collection of redundant symbols that should not be included /// in the final output. /// - 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 @@ namespace WixToolset.Core.Link } } - // 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 @@ namespace WixToolset.Core.Link 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 ace2e19d..3a07d366 100644 --- a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs @@ -40,7 +40,9 @@ namespace WixToolset.Core.Link 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 efb90bb8..d95d648f 100644 --- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs +++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs @@ -91,15 +91,16 @@ namespace WixToolset.Core.Link 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 @@ namespace WixToolset.Core.Link } } - 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 08e01077..dbaceb28 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 @@ namespace WixToolset.Core.Link { this.Symbol = symbol; this.Section = section; - this.Name = String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id); } /// @@ -33,12 +31,6 @@ namespace WixToolset.Core.Link /// 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 @@ namespace WixToolset.Core.Link /// 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 @@ namespace WixToolset.Core.Link } /// - /// 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 a3d99039..5c021ca0 100644 --- a/src/wix/WixToolset.Core/Linker.cs +++ b/src/wix/WixToolset.Core/Linker.cs @@ -177,14 +177,20 @@ namespace WixToolset.Core // 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 @@ namespace WixToolset.Core 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 cacf72bd..3f4108cb 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs @@ -273,5 +273,52 @@ namespace WixToolsetTest.CoreIntegration }, 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 00000000..fc73ee47 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + -- cgit v1.2.3-55-g6feb