Skip to content

Commit

Permalink
Ensure virtual symbols are included when overridden but not referenced
Browse files Browse the repository at this point in the history
  • Loading branch information
robmen committed Mar 8, 2024
1 parent d8c4f61 commit 6a3a695
Show file tree
Hide file tree
Showing 13 changed files with 322 additions and 156 deletions.
12 changes: 6 additions & 6 deletions src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public void CanBuildUsingWixUIAdvancedARM64()
}, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray());
}

[Fact(Skip = "Linker problem")]
[Fact]
public void CanBuildUsingWixUIFeatureTree()
{
var folder = TestData.Get(@"TestData", "WixUI_FeatureTree");
Expand Down Expand Up @@ -161,7 +161,7 @@ public void CanBuildUsingWixUIFeatureTree()
}, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
}

[Fact(Skip = "Linker problem")]
[Fact]
public void CanBuildWithWixUIInstallDirWithCustomizedEula()
{
var folder = TestData.Get(@"TestData", "WixUI_InstallDir");
Expand Down Expand Up @@ -274,7 +274,7 @@ public void CanBuildUsingWixUIMinimalAndReadPdb()
}
}

[Fact(Skip = "Linker problem")]
[Fact]
public void CanBuildUsingWixUIMondo()
{
var folder = TestData.Get(@"TestData", "WixUI_Mondo");
Expand Down Expand Up @@ -307,7 +307,7 @@ public void CanBuildUsingWixUIMondo()
}, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray());
WixAssert.CompareLineByLine(new[]
{
"InstallUISequence:WelcomeDlg\tInstalled AND PATCH\t1295",
"InstallUISequence:WelcomeDlg\tNOT Installed OR PATCH\t1297",
}, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
}

Expand All @@ -325,7 +325,7 @@ public void CanBuildUsingWixUIMondoLocalized()
}, results.Where(s => s.StartsWith("Control:ErrorDlg\tY")).Select(s => s.Split('\t')[9]).ToArray());
}

[Fact(Skip = "Linker problem")]
[Fact]
public void CanBuildWithInstallDirAndRemovedDialog()
{
var folder = TestData.Get(@"TestData", "InstallDir_NoLicense");
Expand Down Expand Up @@ -366,7 +366,7 @@ public void CanBuildWithInstallDirAndRemovedDialog()
}, results.Where(r => r.StartsWith("InstallUISequence:AdvancedWelcome") || r.StartsWith("InstallUISequence:Welcome")).ToArray());
}

[Fact(Skip = "Linker problem")]
[Fact]
public void CanBuildWithInstallDirAndAddedDialog()
{
var folder = TestData.Get(@"TestData", "InstallDir_SpecialDlg");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,16 @@ public FindEntrySectionAndLoadSymbolsCommand(IMessaging messaging, IEnumerable<I
public ISet<IntermediateSymbol> IdenticalDirectorySymbols { get; private set; }

/// <summary>
/// Gets the collection of overridden symbols that should not be included
/// in the final output.
/// Gets the collection of symbols that are marked as overrides.
/// </summary>
public ISet<IntermediateSymbol> OverriddenSymbols { get; private set; }
public IReadOnlyCollection<SymbolWithSection> OverrideSymbols { get; private set; }

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

if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType))
{
Expand Down Expand Up @@ -92,34 +90,15 @@ public void Execute()

if (!symbolsByName.TryGetValue(fullName, out var existingSymbol))
{
if (symbolWithSection.Access == AccessModifier.Override)
{
overrideSymbols.Add(symbolWithSection);
}

symbolsByName.Add(fullName, symbolWithSection);
}
else // uh-oh, duplicate symbols.
else // duplicate symbol ids MAY be a problem, but not always (e.g. identical directories and virtual symbols that get overridden) so we do NOT report errors here.
{
if (AccessModifier.Virtual == existingSymbol.Access && AccessModifier.Override == symbolWithSection.Access)
{
symbolWithSection.OverrideVirtualSymbol(existingSymbol);
symbolsByName[fullName] = symbolWithSection; // replace the virtual symbol with the override symbol.

overrideSymbols.Add(symbolWithSection);
overriddenSymbols.Add(existingSymbol.Symbol);
}
else if (AccessModifier.Override == existingSymbol.Access && AccessModifier.Virtual == symbolWithSection.Access)
{
existingSymbol.OverrideVirtualSymbol(symbolWithSection);

overriddenSymbols.Add(symbolWithSection.Symbol);
}
// 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.
else if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access &&
SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol))
if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access &&
SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol))
{
// Ensure identical symbols are tracked to ensure that only one symbol will end up in linked intermediate.
identicalDirectorySymbols.Add(existingSymbol.Symbol);
Expand All @@ -129,25 +108,21 @@ public void Execute()
{
symbolWithSection.AddPossibleConflict(existingSymbol);
existingSymbol.AddPossibleConflict(symbolWithSection);
possibleConflicts.Add(symbolWithSection);
possibleConflicts.Add(existingSymbol);
}
}
}
}

// Ensure override symbols actually overrode a virtual symbol.
foreach (var symbolWithSection in overrideSymbols)
{
if (symbolWithSection.Overrides is null)
{
this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(symbolWithSection.Symbol));
if (symbolWithSection.Access == AccessModifier.Override)
{
overrideSymbols.Add(symbolWithSection);
}
}
}

this.SymbolsByName = symbolsByName;
this.PossibleConflicts = possibleConflicts;
this.IdenticalDirectorySymbols = identicalDirectorySymbols;
this.OverriddenSymbols = overriddenSymbols;
this.OverrideSymbols = overrideSymbols;
}
}
}
159 changes: 159 additions & 0 deletions src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
// 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
{
using System.Collections.Generic;
using System.Linq;
using WixToolset.Data;
using WixToolset.Data.Symbols;
using WixToolset.Extensibility.Services;

internal class ProcessConflictingSymbolsCommand
{
public ProcessConflictingSymbolsCommand(IMessaging messaging, IReadOnlyCollection<SymbolWithSection> possibleConflicts, IReadOnlyCollection<SymbolWithSection> overrideSymbols, ISet<IntermediateSection> resolvedSections)
{
this.Messaging = messaging;
this.PossibleConflicts = possibleConflicts;
this.OverrideSymbols = overrideSymbols;
this.ResolvedSections = resolvedSections;
}

private IMessaging Messaging { get; }

private IReadOnlyCollection<SymbolWithSection> PossibleConflicts { get; }

private ISet<IntermediateSection> ResolvedSections { get; }

private IReadOnlyCollection<SymbolWithSection> OverrideSymbols { get; }

/// <summary>
/// Gets the collection of overridden symbols that should not be included
/// in the final output.
/// </summary>
public ISet<IntermediateSymbol> OverriddenSymbols { get; private set; }

public void Execute()
{
var overriddenSymbols = new HashSet<IntermediateSymbol>();

foreach (var symbolWithConflicts in this.PossibleConflicts)
{
var conflicts = YieldReferencedConflicts(symbolWithConflicts, this.ResolvedSections).ToList();

if (conflicts.Count > 1)
{
IEnumerable<SymbolWithSection> reportDuplicates;

var virtualConflicts = conflicts.Where(s => s.Access == AccessModifier.Virtual).ToList();

// No virtual symbols, just plain old duplicate errors. This is the easy case.
if (virtualConflicts.Count == 0)
{
var first = conflicts[0];
reportDuplicates = conflicts.Skip(1);

var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;

this.Messaging.Write(LinkerErrors.DuplicateSymbol(first.Symbol, referencingSourceLineNumber));
}
else // there are virtual symbols, which complicates conflict resolution and may not be an error at all.
{
var firstVirtualSymbol = virtualConflicts[0];
var overrideConflicts = conflicts.Where(s => s.Access == AccessModifier.Override).ToList();

// If there is a single virtual symbol, there may be a single override symbol to make this a success case.
// All other scenarios are errors.
if (virtualConflicts.Count == 1)
{
var otherConflicts = conflicts.Where(s => s.Access != AccessModifier.Virtual && s.Access != AccessModifier.Override).ToList();

if (otherConflicts.Count > 0)
{
var first = otherConflicts[0];
var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;

reportDuplicates = virtualConflicts;

switch (first.Symbol)
{
case WixActionSymbol action:
this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(action));
break;
default:
this.Messaging.Write(LinkerErrors.VirtualSymbolMustBeOverridden(first.Symbol, referencingSourceLineNumber));
break;
}
}
else if (overrideConflicts.Count > 1) // multiple overrides report as normal duplicates.
{
var first = overrideConflicts[0];
var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;

reportDuplicates = overrideConflicts.Skip(1);

this.Messaging.Write(LinkerErrors.DuplicateSymbol(first.Symbol, referencingSourceLineNumber));
}
else // the single virtual symbol is overridden by a single override symbol. This is a success case.
{
var overrideSymbol = overrideConflicts[0];

overriddenSymbols.Add(firstVirtualSymbol.Symbol);

reportDuplicates = Enumerable.Empty<SymbolWithSection>();
}
}
else // multiple symbols are virtual, use the duplicate virtual symbol message.
{
var first = virtualConflicts[0];
var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers;

reportDuplicates = virtualConflicts.Skip(1);

this.Messaging.Write(LinkerErrors.DuplicateVirtualSymbol(first.Symbol, referencingSourceLineNumber));
}

// Always point the override symbols at the first virtual symbol to prevent error being reported about missing overrides.
// There may have been errors reported above, but there was at least one virtual symbol to satisfy the overrides so we
// don't want extra errors in this case.
foreach (var overrideSymbol in overrideConflicts)
{
overrideSymbol.OverrideVirtualSymbol(firstVirtualSymbol);
}
}

foreach (var duplicate in reportDuplicates)
{
this.Messaging.Write(LinkerErrors.DuplicateSymbol2(duplicate.Symbol));
}
}
}

// Ensure referenced override symbols actually overrode a virtual symbol.
foreach (var referencedOverrideSymbol in this.OverrideSymbols.Where(s => this.ResolvedSections.Contains(s.Section)))
{
if (referencedOverrideSymbol.Overrides is null)
{
this.Messaging.Write(LinkerErrors.VirtualSymbolNotFoundForOverride(referencedOverrideSymbol.Symbol));
}
}

this.OverriddenSymbols = overriddenSymbols;
}

private static IEnumerable<SymbolWithSection> YieldReferencedConflicts(SymbolWithSection symbolWithConflicts, ISet<IntermediateSection> resolvedSections)
{
if (resolvedSections.Contains(symbolWithConflicts.Section))
{
yield return symbolWithConflicts;
}

foreach (var possibleConflict in symbolWithConflicts.PossiblyConflicts)
{
if (resolvedSections.Contains(possibleConflict.Section))
{
yield return possibleConflict;
}
}
}
}
}
Loading

0 comments on commit 6a3a695

Please sign in to comment.