From 6a3a695b5ac3758fbf4a7836f65d7f85fdd121e0 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Fri, 8 Mar 2024 13:58:11 -0800 Subject: [PATCH] Ensure virtual symbols are included when overridden but not referenced --- .../WixToolsetTest.UI/UIExtensionFixture.cs | 12 +- .../FindEntrySectionAndLoadSymbolsCommand.cs | 47 ++---- .../Link/ProcessConflictingSymbolsCommand.cs | 159 ++++++++++++++++++ .../Link/ReportConflictingSymbolsCommand.cs | 92 ---------- .../Link/ResolveReferencesCommand.cs | 63 ++++++- src/wix/WixToolset.Core/Linker.cs | 9 +- .../AccessModifierFixture.cs | 25 ++- .../PreprocessorFixture.cs | 15 +- .../RegistryFixture.cs | 7 +- .../DuplicateCrossFragmentReference.wxs | 4 + .../OverrideVirtualSymbolWithFragments.wxs | 13 ++ .../VirtualSymbolThatDoesNotGetOverridden.wxs | 30 ++++ .../TestData/Variables/Package.wxs | 2 +- 13 files changed, 322 insertions(+), 156 deletions(-) create mode 100644 src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs delete mode 100644 src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs diff --git a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs index 610fd5ed3..bcd7654f0 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs +++ b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs @@ -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"); @@ -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"); @@ -274,7 +274,7 @@ public void CanBuildUsingWixUIMinimalAndReadPdb() } } - [Fact(Skip = "Linker problem")] + [Fact] public void CanBuildUsingWixUIMondo() { var folder = TestData.Get(@"TestData", "WixUI_Mondo"); @@ -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()); } @@ -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"); @@ -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"); diff --git a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs index f9d6ab598..8eaac27db 100644 --- a/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs +++ b/src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs @@ -45,10 +45,9 @@ public FindEntrySectionAndLoadSymbolsCommand(IMessaging messaging, IEnumerable IdenticalDirectorySymbols { get; private set; } /// - /// 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. /// - public ISet OverriddenSymbols { get; private set; } + public IReadOnlyCollection OverrideSymbols { get; private set; } public void Execute() { @@ -56,7 +55,6 @@ public void Execute() var possibleConflicts = new HashSet(); var identicalDirectorySymbols = new HashSet(); var overrideSymbols = new List(); - var overriddenSymbols = new HashSet(); if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType)) { @@ -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); @@ -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; } } } diff --git a/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs new file mode 100644 index 000000000..556a24d74 --- /dev/null +++ b/src/wix/WixToolset.Core/Link/ProcessConflictingSymbolsCommand.cs @@ -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 possibleConflicts, IReadOnlyCollection overrideSymbols, ISet resolvedSections) + { + this.Messaging = messaging; + this.PossibleConflicts = possibleConflicts; + this.OverrideSymbols = overrideSymbols; + this.ResolvedSections = resolvedSections; + } + + private IMessaging Messaging { get; } + + private IReadOnlyCollection PossibleConflicts { get; } + + private ISet ResolvedSections { get; } + + private IReadOnlyCollection OverrideSymbols { get; } + + /// + /// Gets the collection of overridden symbols that should not be included + /// in the final output. + /// + public ISet OverriddenSymbols { get; private set; } + + public void Execute() + { + var overriddenSymbols = new HashSet(); + + foreach (var symbolWithConflicts in this.PossibleConflicts) + { + var conflicts = YieldReferencedConflicts(symbolWithConflicts, this.ResolvedSections).ToList(); + + if (conflicts.Count > 1) + { + IEnumerable 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(); + } + } + 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 YieldReferencedConflicts(SymbolWithSection symbolWithConflicts, ISet resolvedSections) + { + if (resolvedSections.Contains(symbolWithConflicts.Section)) + { + yield return symbolWithConflicts; + } + + foreach (var possibleConflict in symbolWithConflicts.PossiblyConflicts) + { + if (resolvedSections.Contains(possibleConflict.Section)) + { + yield return possibleConflict; + } + } + } + } +} diff --git a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs b/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs deleted file mode 100644 index b1db46a2d..000000000 --- a/src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs +++ /dev/null @@ -1,92 +0,0 @@ -// 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 ReportConflictingSymbolsCommand - { - public ReportConflictingSymbolsCommand(IMessaging messaging, IReadOnlyCollection possibleConflicts, ISet resolvedSections) - { - this.Messaging = messaging; - this.PossibleConflicts = possibleConflicts; - this.ResolvedSections = resolvedSections; - } - - private IMessaging Messaging { get; } - - private IReadOnlyCollection PossibleConflicts { get; } - - private ISet ResolvedSections { get; } - - public void Execute() - { - // Do a quick check if there are any possibly conflicting symbols. Hopefully the symbols with possible conflicts - // list is a very short list (empty should be the most common). - // - // If we have conflicts then we'll do a more costly check to see if the possible conflicting - // symbols are in sections we actually referenced. From the resulting set, show an error for each duplicate - // (aka: conflicting) symbol. - if (0 < this.PossibleConflicts.Count) - { - foreach (var referencedDuplicate in this.PossibleConflicts.Where(s => this.ResolvedSections.Contains(s.Section))) - { - var actuallyReferencedDuplicates = referencedDuplicate.PossiblyConflicts.Where(s => this.ResolvedSections.Contains(s.Section)).ToList(); - - if (actuallyReferencedDuplicates.Count > 0) - { - var conflicts = actuallyReferencedDuplicates.Append(referencedDuplicate).ToList(); - var virtualConflicts = conflicts.Where(s => s.Access == AccessModifier.Virtual).ToList(); - var overrideConflicts = conflicts.Where(s => s.Access == AccessModifier.Override).ToList(); - var otherConflicts = conflicts.Where(s => s.Access != AccessModifier.Virtual && s.Access != AccessModifier.Override).ToList(); - - IEnumerable reportDuplicates = actuallyReferencedDuplicates; - - // If multiple symbols are virtual, use the duplicate virtual symbol message. - if (virtualConflicts.Count > 1) - { - var first = virtualConflicts[0]; - var referencingSourceLineNumber = first.DirectReferences.FirstOrDefault()?.SourceLineNumbers; - - reportDuplicates = virtualConflicts.Skip(1); - - this.Messaging.Write(LinkerErrors.DuplicateVirtualSymbol(first.Symbol, referencingSourceLineNumber)); - } - else if (virtualConflicts.Count == 1 && 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 - { - var referencingSourceLineNumber = referencedDuplicate.DirectReferences.FirstOrDefault()?.SourceLineNumbers; - - this.Messaging.Write(LinkerErrors.DuplicateSymbol(referencedDuplicate.Symbol, referencingSourceLineNumber)); - } - - foreach (var duplicate in reportDuplicates) - { - this.Messaging.Write(LinkerErrors.DuplicateSymbol2(duplicate.Symbol)); - } - } - } - } - } - } -} diff --git a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs index 0e7a7a52f..7a3c502af 100644 --- a/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs +++ b/src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs @@ -76,6 +76,7 @@ private void RecursivelyResolveReferences(IntermediateSection section) if (this.symbolsWithSections.TryGetValue(reference.SymbolicName, out var symbolWithSection)) { var accessible = this.DetermineAccessibleSymbols(section, symbolWithSection); + if (accessible.Count == 1) { var accessibleSymbol = accessible[0]; @@ -91,7 +92,7 @@ private void RecursivelyResolveReferences(IntermediateSection section) { this.Messaging.Write(ErrorMessages.UnresolvedReference(reference.SourceLineNumbers, reference.SymbolicName, symbolWithSection.Access)); } - else // multiple symbols referenced creates conflicting symbols. + else // multiple accessible symbols referenced creates conflicting symbols. { // Remember the direct reference to the symbol for the error reporting later, // but do NOT continue resolving references found in these conflicting symbols. @@ -122,7 +123,7 @@ private void RecursivelyResolveReferences(IntermediateSection section) } /// - /// Determine if the symbol and any of its duplicates are accessbile by referencing section. + /// Determine if the symbol and any of its duplicates are accessbile by the referencing section. /// /// Section referencing the symbol. /// Symbol being referenced. @@ -130,23 +131,77 @@ private void RecursivelyResolveReferences(IntermediateSection section) private List DetermineAccessibleSymbols(IntermediateSection referencingSection, SymbolWithSection symbolWithSection) { var accessibleSymbols = new List(); + List virtualSymbols = null; if (this.AccessibleSymbol(referencingSection, symbolWithSection)) { - accessibleSymbols.Add(symbolWithSection); + AddSymbolToCorrectCollection(symbolWithSection, accessibleSymbols, ref virtualSymbols); } foreach (var dupe in symbolWithSection.PossiblyConflicts) { if (this.AccessibleSymbol(referencingSection, dupe)) { - accessibleSymbols.Add(dupe); + AddSymbolToCorrectCollection(dupe, accessibleSymbols, ref virtualSymbols); + } + } + + // If a virtual symbol is accessible we have some extra work to do. + if (virtualSymbols != null) + { + // If there are multiple virtual symbols accessible that's an error case so add them all and + // they'll be reported as conflicts later. + if (virtualSymbols.Count > 1) + { + accessibleSymbols.AddRange(virtualSymbols); + } + else + { + var overrode = false; + + // If there are any override symbols, skip adding the virtual symbols because they are "overridden". + foreach (var overrideSymbol in accessibleSymbols.Where(symbol => symbol.Access == AccessModifier.Override)) + { + overrideSymbol.OverrideVirtualSymbol(virtualSymbols[0]); + + overrode = true; + } + + // If there are no overriding symbols, add the virtual symbol (there is only one but we'll add the collection just in case) + // so it will be reported as a conflict later. + if (!overrode) + { + accessibleSymbols.AddRange(virtualSymbols); + } } } return accessibleSymbols; } + /// + /// Add a symbol to the correct collection based on its access. + /// + /// + /// Collection of non-virtual symbol that was accessible. + /// Collection of virtual symbol that was accessible + private static void AddSymbolToCorrectCollection(SymbolWithSection symbolWithSection, List accessibleSymbols, ref List virtualSymbols) + { + if (symbolWithSection.Access == AccessModifier.Virtual) + { + if (virtualSymbols == null) + { + virtualSymbols = new List(); + } + + virtualSymbols.Add(symbolWithSection); + } + else + { + accessibleSymbols.Add(symbolWithSection); + } + } + /// /// Determine if a single symbol is accessible by the referencing section. /// diff --git a/src/wix/WixToolset.Core/Linker.cs b/src/wix/WixToolset.Core/Linker.cs index a536c049b..43a41eac9 100644 --- a/src/wix/WixToolset.Core/Linker.cs +++ b/src/wix/WixToolset.Core/Linker.cs @@ -190,10 +190,13 @@ public Intermediate Link(ILinkContext context) } } - // Report duplicates that would ultimately end up being primary key collisions. + // Process conflicts that may be overridden virtual symbols (that's okay) or end up as primary key collisions (those need to be reported as errors). + ISet overriddenSymbols; { - var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections); + var reportDupes = new ProcessConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, find.OverrideSymbols, resolve.ResolvedSections); reportDupes.Execute(); + + overriddenSymbols = reportDupes.OverriddenSymbols; } if (this.Messaging.EncounteredError) @@ -222,7 +225,7 @@ public Intermediate Link(ILinkContext context) continue; } } - else if (find.OverriddenSymbols.Contains(symbol)) + else if (overriddenSymbols.Contains(symbol)) { // Skip the symbols that were overridden. continue; diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs index 5e40114fa..13eb15beb 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/AccessModifierFixture.cs @@ -62,6 +62,18 @@ public void CanCompileVirtualSymbolWithFragments() }, dirSymbols.OrderBy(d => d.Id.Id).Select(d => d.Id.Access.AsString() + ":" + d.Id.Id + ":" + d.ParentDirectoryRef + ":" + d.Name).ToArray()); } + [Fact] + public void CanCompileVirtualSymbolThatDoesNotGetOverridden() + { + var dirSymbols = BuildToGetDirectorySymbols("TestData", "AccessModifier", "VirtualSymbolThatDoesNotGetOverridden.wxs"); + WixAssert.CompareLineByLine(new[] + { + "virtual:ProgramFilesFolder:TARGETDIR:PFiles", + "virtual:TARGETDIR::SourceDir", + "virtual:TestFolder:ProgramFilesFolder:Used", + }, dirSymbols.OrderBy(d => d.Id.Id).Select(d => d.Id.Access.AsString() + ":" + d.Id.Id + ":" + d.ParentDirectoryRef + ":" + d.Name).ToArray()); + } + [Fact] public void CannotCompileInvalidCrossFragmentReference() { @@ -78,8 +90,9 @@ public void CannotCompileDuplicateCrossFragmentReference() var errors = BuildForFailure("TestData", "AccessModifier", "DuplicateCrossFragmentReference.wxs"); WixAssert.CompareLineByLine(new[] { - "ln 12: Duplicate Directory with identifier 'TestFolder' referenced by \\DuplicateCrossFragmentReference.wxs(4). Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.", - "ln 8: Location of symbol related to previous error." + "ln 8: Duplicate Directory with identifier 'TestFolder' referenced by \\DuplicateCrossFragmentReference.wxs(4). Ensure all your identifiers of a given type (Directory, File, etc.) are unique or use an access modifier to scope the identfier.", + "ln 12: Location of symbol related to previous error.", + "ln 16: Location of symbol related to previous error." }, errors); } @@ -99,8 +112,8 @@ public void CannotCompileDuplicatedOverride() var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatedOverrideVirtualSymbol.wxs"); WixAssert.CompareLineByLine(new[] { - "ln 14: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", - "ln 6: Location of symbol related to previous error." + "ln 6: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", + "ln 14: Location of symbol related to previous error." }, errors); } @@ -132,8 +145,8 @@ public void CannotCompilePublicAndOverrideWithVirtualSymbol() var errors = BuildForFailure("TestData", "AccessModifier", "DuplicatePublicOverrideVirtualSymbol.wxs"); WixAssert.CompareLineByLine(new[] { - "ln 14: Duplicate Directory with identifier 'TestFolder' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", - "ln 6: Location of symbol related to previous error." + "ln 14: The Directory symbol 'TestFolder' conflicts with a virtual symbol. Use the 'override' access modifier to override the virtual symbol or use a different Id to avoid the conflict.", + "ln 10: Location of symbol related to previous error." }, errors); } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs index f735b53a2..17c8db956 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/PreprocessorFixture.cs @@ -4,9 +4,9 @@ namespace WixToolsetTest.CoreIntegration { using System.IO; using System.Linq; + using WixInternal.Core.TestPackage; using WixInternal.TestSupport; using WixToolset.Core; - using WixInternal.Core.TestPackage; using WixToolset.Data; using WixToolset.Data.Symbols; using WixToolset.Extensibility.Data; @@ -101,7 +101,7 @@ public void SupportParensInEnvironmentVariables() [Fact] public void VariableRedefinitionIsAWarning() { - var folder = TestData.Get(@"TestData\Variables"); + var folder = TestData.Get(@"TestData", "Variables"); using (var fs = new DisposableFileSystem()) { @@ -121,8 +121,15 @@ public void VariableRedefinitionIsAWarning() result.AssertSuccess(); - var warning = result.Messages.Where(message => message.Id == (int)WarningMessages.Ids.VariableDeclarationCollision); - Assert.Single(warning); + var warning = result.Messages.Where(m => m.Level == MessageLevel.Warning || m.Level == MessageLevel.Error) + .Select(m => $"ln {m.SourceLineNumbers.LineNumber}: {m.Level.ToString().ToLowerInvariant()}: {m}".Replace(baseFolder, "")) + .ToArray(); + WixAssert.CompareLineByLine(new[] + { + "ln 5: warning: The variable 'Bar' with value 'Baz' was previously declared with value 'Bar'.", + "ln 24: warning: It is no longer necessary to define the standard directory 'TARGETDIR'. Use the StandardDirectory element instead.", + "ln 25: warning: It is no longer necessary to define the standard directory 'ProgramFilesFolder'. Use the StandardDirectory element instead." + }, warning); } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs index 9931a45ae..5b3579945 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/RegistryFixture.cs @@ -102,10 +102,9 @@ public void DuplicateRegistryValueIdsAreDetectedSmoothly() .ToArray(); WixAssert.CompareLineByLine(new[] { - "ln 8: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", - "ln 7: Location of symbol related to previous error.", - "ln 9: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", - "ln 7: Location of symbol related to previous error." + "ln 7: Duplicate Registry with identifier 'regJnkjRU9YGaMJhQOqKmivWKf_VdY' found. Access modifiers (global, library, file, section) cannot prevent these conflicts. Ensure all your identifiers of a given type (Directory, File, etc.) are unique.", + "ln 8: Location of symbol related to previous error.", + "ln 9: Location of symbol related to previous error." }, errors); } } diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs index f987fe132..73678cb6c 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/DuplicateCrossFragmentReference.wxs @@ -11,4 +11,8 @@ + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs index 13f030409..9afec0031 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/OverrideVirtualSymbolWithFragments.wxs @@ -2,8 +2,21 @@ + + + + + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs new file mode 100644 index 000000000..123b856d7 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/AccessModifier/VirtualSymbolThatDoesNotGetOverridden.wxs @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs index bcbdb00f9..d22676d4b 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/Variables/Package.wxs @@ -22,7 +22,7 @@ - +