Skip to content

Commit

Permalink
Include duplicated inline directory symbols referenced in subsequent …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@robmen
robmen committed Nov 7, 2023
1 parent 4b9bbae commit a532341
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 52 deletions.
25 changes: 12 additions & 13 deletions src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
View file
Expand Up @@ -42,13 +42,13 @@ public FindEntrySectionAndLoadSymbolsCommand(IMessaging messaging, IEnumerable<I
/// Gets the collection of redundant symbols that should not be included
/// in the final output.
/// </summary>
public ISet<IntermediateSymbol> RedundantSymbols { get; private set; }
public ISet<IntermediateSymbol> IdenticalDirectorySymbols { get; private set; }

public void Execute()
{
var symbolsByName = new Dictionary<string, SymbolWithSection>();
var possibleConflicts = new HashSet<SymbolWithSection>();
var redundantSymbols = new HashSet<IntermediateSymbol>();
var identicalDirectorySymbols = new HashSet<IntermediateSymbol>();

if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType))
{
Expand Down Expand Up @@ -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
{
Expand All @@ -111,7 +110,7 @@ public void Execute()

this.SymbolsByName = symbolsByName;
this.PossibleConflicts = possibleConflicts;
this.RedundantSymbols = redundantSymbols;
this.IdenticalDirectorySymbols = identicalDirectorySymbols;
}
}
}
4 changes: 3 additions & 1 deletion src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs
View file
Expand Up @@ -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)
{
Expand Down
13 changes: 3 additions & 10 deletions src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs
View file
Expand Up @@ -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))
Expand Down Expand Up @@ -146,14 +147,6 @@ private List<SymbolWithSection> DetermineAccessibleSymbols(IntermediateSection r
}
}

foreach (var dupe in symbolWithSection.Redundants)
{
if (this.AccessibleSymbol(referencingSection, dupe))
{
accessibleSymbols.Add(dupe);
}
}

return accessibleSymbols;
}

Expand Down
25 changes: 3 additions & 22 deletions src/wix/WixToolset.Core/Link/SymbolWithSection.cs
View file
Expand Up @@ -13,7 +13,6 @@ namespace WixToolset.Core.Link
internal class SymbolWithSection
{
private HashSet<SymbolWithSection> possibleConflicts;
private HashSet<SymbolWithSection> redundants;

/// <summary>
/// Creates a symbol for a symbol.
Expand All @@ -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);
}

/// <summary>
Expand All @@ -33,12 +31,6 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol)
/// <value>Accessbility of the symbol.</value>
public AccessModifier Access => this.Symbol.Id.Access;

/// <summary>
/// Gets the name of the symbol.
/// </summary>
/// <value>Name of the symbol.</value>
public string Name { get; }

/// <summary>
/// Gets the symbol for this symbol.
/// </summary>
Expand All @@ -56,11 +48,6 @@ public SymbolWithSection(IntermediateSection section, IntermediateSymbol symbol)
/// </summary>
public IEnumerable<SymbolWithSection> PossiblyConflicts => this.possibleConflicts ?? Enumerable.Empty<SymbolWithSection>();

/// <summary>
/// Gets any duplicates of this symbol with sections that are redundant.
/// </summary>
public IEnumerable<SymbolWithSection> Redundants => this.redundants ?? Enumerable.Empty<SymbolWithSection>();

/// <summary>
/// Adds a duplicate symbol with sections that is a possible conflict.
/// </summary>
Expand All @@ -76,17 +63,11 @@ public void AddPossibleConflict(SymbolWithSection symbolWithSection)
}

/// <summary>
/// Adds a duplicate symbol that is redundant.
/// Gets the full name of the symbol.
/// </summary>
/// <param name="symbolWithSection">Symbol with section that is redundant of this symbol.</param>
public void AddRedundant(SymbolWithSection symbolWithSection)
public string GetFullName()
{
if (null == this.redundants)
{
this.redundants = new HashSet<SymbolWithSection>();
}

this.redundants.Add(symbolWithSection);
return String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id);
}
}
}
20 changes: 14 additions & 6 deletions src/wix/WixToolset.Core/Linker.cs
View file
Expand Up @@ -177,14 +177,20 @@ public Intermediate Link(ILinkContext context)
// metadata.
var resolvedSection = new IntermediateSection(find.EntrySection.Id, find.EntrySection.Type);
var references = new List<WixSimpleReferenceSymbol>();
var identicalDirectoryIds = new HashSet<string>(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.
Expand Down Expand Up @@ -351,22 +357,24 @@ private void LoadStandardSymbols(IDictionary<string, SymbolWithSection> 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);
}
}
}
Expand Down
47 changes: 47 additions & 0 deletions src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs
View file
Expand Up @@ -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<WixToolset.Data.Symbols.DirectorySymbol>().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());
}
}
}
}
26 changes: 26 additions & 0 deletions ...ixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs
View file
@@ -0,0 +1,26 @@
<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
<Package Name="~RedundantSubdirectories" Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="12E4699F-E774-4D05-8A01-5BDD41BBA127" Compressed="no">
<MajorUpgrade DowngradeErrorMessage="A newer version of [ProductName] is already installed." />

<Feature Id="ProductFeature">
<!-- NotIncludeButFirst will define the subdirectory and IncludedButSecond needs the same symbols, this tests linking order -->
<ComponentGroupRef Id="IncludedButSecond" />
</Feature>
</Package>

<Fragment Id="NotIncludedButFirst">
<ComponentGroup Id="NotIncludedButFirst">
<Component Directory="ProgramFilesFolder" Subdirectory="a\b\c">
<File Name="notincluded.txt" Source="test.txt" />
</Component>
</ComponentGroup>
</Fragment>

<Fragment Id="IncludedButSecond">
<ComponentGroup Id="IncludedButSecond">
<Component Directory="ProgramFilesFolder" Subdirectory="a\b\c">
<File Name="included.txt" Source="test.txt" />
</Component>
</ComponentGroup>
</Fragment>
</Wix>

0 comments on commit a532341

Please sign in to comment.