Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure virtual symbols are included when overridden but not referenced #504

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Ensure virtual symbols are included when overridden but not referenced #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs
View file
Open in desktop
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
47 changes: 11 additions & 36 deletions src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
View file
Open in desktop
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
View file
Open in desktop
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
Loading