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
Rob Mensching committed
Nov 6, 2023 at 21:46 UTC
a532341085b0c8d9065deb1557b8ae0c0297eea8
7 files changed
+108
-52
src/wix/WixToolset.Core/Link/FindEntrySectionAndLoadSymbolsCommand.cs
+12
-13
@@ -42,13 +42,13 @@ namespace WixToolset.Core.Link
42
/// Gets the collection of redundant symbols that should not be included
43
/// in the final output.
44
/// </summary>
45
- public ISet<IntermediateSymbol> RedundantSymbols { get; private set; }
45
+ public ISet<IntermediateSymbol> IdenticalDirectorySymbols { get; private set; }
46
47
public void Execute()
48
{
49
var symbolsByName = new Dictionary<string, SymbolWithSection>();
50
var possibleConflicts = new HashSet<SymbolWithSection>();
51
- var redundantSymbols = new HashSet<IntermediateSymbol>();
51
+ var identicalDirectorySymbols = new HashSet<IntermediateSymbol>();
52
53
if (!Enum.TryParse(this.ExpectedOutputType.ToString(), out SectionType expectedEntrySectionType))
54
{
@@ -76,28 +76,27 @@ namespace WixToolset.Core.Link
76
}
77
}
78
79
- // Load all the symbols from the section's tables that create symbols.
79
+ // Load all the symbols from the section's tables that can be referenced (i.e. have an Id).
80
foreach (var symbol in section.Symbols.Where(t => t.Id != null))
81
{
82
var symbolWithSection = new SymbolWithSection(section, symbol);
83
+ var fullName = symbolWithSection.GetFullName();
84
84
- if (!symbolsByName.TryGetValue(symbolWithSection.Name, out var existingSymbol))
85
+ if (!symbolsByName.TryGetValue(fullName, out var existingSymbol))
86
{
86
- symbolsByName.Add(symbolWithSection.Name, symbolWithSection);
87
+ symbolsByName.Add(fullName, symbolWithSection);
88
}
89
else // uh-oh, duplicate symbols.
90
{
91
// If the duplicate symbols are both private directories, there is a chance that they
91
- // point to identical symbols. Identical directory symbols are redundant and will not cause
92
- // conflicts.
92
+ // point to identical symbols. Identical directory symbols should be treated as redundant.
93
+ // and not cause conflicts.
94
if (AccessModifier.Section == existingSymbol.Access && AccessModifier.Section == symbolWithSection.Access &&
95
SymbolDefinitionType.Directory == existingSymbol.Symbol.Definition.Type && existingSymbol.Symbol.IsIdentical(symbolWithSection.Symbol))
96
{
96
- // Ensure identical symbol's symbol is marked redundant to ensure (should the symbol be
97
- // referenced into the final output) it will not add duplicate primary keys during
98
- // the .IDT importing.
99
- existingSymbol.AddRedundant(symbolWithSection);
100
- redundantSymbols.Add(symbolWithSection.Symbol);
97
+ // Ensure identical symbols are tracked to ensure that only one symbol will end up in linked intermediate.
98
+ identicalDirectorySymbols.Add(existingSymbol.Symbol);
99
+ identicalDirectorySymbols.Add(symbolWithSection.Symbol);
100
}
101
else
102
{
@@ -111,7 +110,7 @@ namespace WixToolset.Core.Link
110
111
this.SymbolsByName = symbolsByName;
112
this.PossibleConflicts = possibleConflicts;
114
- this.RedundantSymbols = redundantSymbols;
113
+ this.IdenticalDirectorySymbols = identicalDirectorySymbols;
114
}
115
}
116
}
src/wix/WixToolset.Core/Link/ReportConflictingSymbolsCommand.cs
+3
-1
@@ -40,7 +40,9 @@ namespace WixToolset.Core.Link
40
41
if (actuallyReferencedDuplicates.Any())
42
{
43
- this.Messaging.Write(ErrorMessages.DuplicateSymbol(referencedDuplicate.Symbol.SourceLineNumbers, referencedDuplicate.Name));
43
+ var fullName = referencedDuplicate.GetFullName();
44
+
45
+ this.Messaging.Write(ErrorMessages.DuplicateSymbol(referencedDuplicate.Symbol.SourceLineNumbers, fullName));
46
47
foreach (var duplicate in actuallyReferencedDuplicates)
48
{
src/wix/WixToolset.Core/Link/ResolveReferencesCommand.cs
+3
-10
@@ -91,15 +91,16 @@ namespace WixToolset.Core.Link
91
else // display errors for the duplicate symbols.
92
{
93
var accessibleSymbol = accessible[0];
94
+ var accessibleFullName = accessibleSymbol.GetFullName();
95
var referencingSourceLineNumber = wixSimpleReferenceRow.SourceLineNumbers?.ToString();
96
97
if (String.IsNullOrEmpty(referencingSourceLineNumber))
98
{
98
- this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleSymbol.Name));
99
+ this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName));
100
}
101
else
102
{
102
- this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleSymbol.Name, referencingSourceLineNumber));
103
+ this.Messaging.Write(ErrorMessages.DuplicateSymbol(accessibleSymbol.Symbol.SourceLineNumbers, accessibleFullName, referencingSourceLineNumber));
104
}
105
106
foreach (var accessibleDuplicate in accessible.Skip(1))
@@ -146,14 +147,6 @@ namespace WixToolset.Core.Link
147
}
148
}
149
149
- foreach (var dupe in symbolWithSection.Redundants)
150
- {
151
- if (this.AccessibleSymbol(referencingSection, dupe))
152
- {
153
- accessibleSymbols.Add(dupe);
154
- }
155
- }
156
-
150
return accessibleSymbols;
151
}
152
src/wix/WixToolset.Core/Link/SymbolWithSection.cs
+3
-22
@@ -13,7 +13,6 @@ namespace WixToolset.Core.Link
13
internal class SymbolWithSection
14
{
15
private HashSet<SymbolWithSection> possibleConflicts;
16
- private HashSet<SymbolWithSection> redundants;
16
17
/// <summary>
18
/// Creates a symbol for a symbol.
@@ -24,7 +23,6 @@ namespace WixToolset.Core.Link
23
{
24
this.Symbol = symbol;
25
this.Section = section;
27
- this.Name = String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id);
26
}
27
28
/// <summary>
@@ -33,12 +31,6 @@ namespace WixToolset.Core.Link
31
/// <value>Accessbility of the symbol.</value>
32
public AccessModifier Access => this.Symbol.Id.Access;
33
36
- /// <summary>
37
- /// Gets the name of the symbol.
38
- /// </summary>
39
- /// <value>Name of the symbol.</value>
40
- public string Name { get; }
41
-
34
/// <summary>
35
/// Gets the symbol for this symbol.
36
/// </summary>
@@ -56,11 +48,6 @@ namespace WixToolset.Core.Link
48
/// </summary>
49
public IEnumerable<SymbolWithSection> PossiblyConflicts => this.possibleConflicts ?? Enumerable.Empty<SymbolWithSection>();
50
59
- /// <summary>
60
- /// Gets any duplicates of this symbol with sections that are redundant.
61
- /// </summary>
62
- public IEnumerable<SymbolWithSection> Redundants => this.redundants ?? Enumerable.Empty<SymbolWithSection>();
63
-
51
/// <summary>
52
/// Adds a duplicate symbol with sections that is a possible conflict.
53
/// </summary>
@@ -76,17 +63,11 @@ namespace WixToolset.Core.Link
63
}
64
65
/// <summary>
79
- /// Adds a duplicate symbol that is redundant.
66
+ /// Gets the full name of the symbol.
67
/// </summary>
81
- /// <param name="symbolWithSection">Symbol with section that is redundant of this symbol.</param>
82
- public void AddRedundant(SymbolWithSection symbolWithSection)
68
+ public string GetFullName()
69
{
84
- if (null == this.redundants)
85
- {
86
- this.redundants = new HashSet<SymbolWithSection>();
87
- }
88
-
89
- this.redundants.Add(symbolWithSection);
70
+ return String.Concat(this.Symbol.Definition.Name, ":", this.Symbol.Id.Id);
71
}
72
}
73
}
src/wix/WixToolset.Core/Linker.cs
+14
-6
@@ -177,14 +177,20 @@ namespace WixToolset.Core
177
// metadata.
178
var resolvedSection = new IntermediateSection(find.EntrySection.Id, find.EntrySection.Type);
179
var references = new List<WixSimpleReferenceSymbol>();
180
+ var identicalDirectoryIds = new HashSet<string>(StringComparer.Ordinal);
181
182
foreach (var section in sections)
183
{
184
foreach (var symbol in section.Symbols)
185
{
185
- if (find.RedundantSymbols.Contains(symbol))
186
+ // If this symbol is an identical directory, ensure we only visit
187
+ // one (and skip the other identicals with the same id).
188
+ if (find.IdenticalDirectorySymbols.Contains(symbol))
189
{
187
- continue;
190
+ if (!identicalDirectoryIds.Add(symbol.Id.Id))
191
+ {
192
+ continue;
193
+ }
194
}
195
196
var copySymbol = true; // by default, copy symbols.
@@ -351,22 +357,24 @@ namespace WixToolset.Core
357
foreach (var actionSymbol in WindowsInstallerStandard.StandardActions())
358
{
359
var symbolWithSection = new SymbolWithSection(null, actionSymbol);
360
+ var fullName = symbolWithSection.GetFullName();
361
362
// If the action's symbol has not already been defined (i.e. overriden by the user), add it now.
356
- if (!symbolsByName.ContainsKey(symbolWithSection.Name))
363
+ if (!symbolsByName.ContainsKey(fullName))
364
{
358
- symbolsByName.Add(symbolWithSection.Name, symbolWithSection);
365
+ symbolsByName.Add(fullName, symbolWithSection);
366
}
367
}
368
369
foreach (var directorySymbol in WindowsInstallerStandard.StandardDirectories())
370
{
371
var symbolWithSection = new SymbolWithSection(null, directorySymbol);
372
+ var fullName = symbolWithSection.GetFullName();
373
374
// If the directory's symbol has not already been defined (i.e. overriden by the user), add it now.
367
- if (!symbolsByName.ContainsKey(symbolWithSection.Name))
375
+ if (!symbolsByName.ContainsKey(fullName))
376
{
369
- symbolsByName.Add(symbolWithSection.Name, symbolWithSection);
377
+ symbolsByName.Add(fullName, symbolWithSection);
378
}
379
}
380
}
src/wix/test/WixToolsetTest.CoreIntegration/DirectoryFixture.cs
+47
@@ -273,5 +273,52 @@ namespace WixToolsetTest.CoreIntegration
273
}, directoryRows.Select(r => String.Join('\t', r.FieldAsString(0), r.FieldAsString(1), r.FieldAsString(2))).ToArray());
274
}
275
}
276
+
277
+ [Fact]
278
+ public void CanFindRedundantSubdirectoryInSecondSection()
279
+ {
280
+ var folder = TestData.Get(@"TestData");
281
+
282
+ using (var fs = new DisposableFileSystem())
283
+ {
284
+ var baseFolder = fs.GetFolder();
285
+ var intermediateFolder = Path.Combine(baseFolder, "obj");
286
+ var msiPath = Path.Combine(baseFolder, "bin", "test.msi");
287
+ var wixpdbPath = Path.ChangeExtension(msiPath, ".wixpdb");
288
+
289
+ var result = WixRunner.Execute(new[]
290
+ {
291
+ "build",
292
+ Path.Combine(folder, "Directory", "RedundantSubdirectoryInSecondSection.wxs"),
293
+ "-bindpath", Path.Combine(folder, "SingleFile", "data"),
294
+ "-intermediateFolder", intermediateFolder,
295
+ "-o", msiPath
296
+ });
297
+
298
+ result.AssertSuccess();
299
+
300
+ var intermediate = Intermediate.Load(wixpdbPath);
301
+ var section = intermediate.Sections.Single();
302
+
303
+ var dirSymbols = section.Symbols.OfType<WixToolset.Data.Symbols.DirectorySymbol>().ToList();
304
+ WixAssert.CompareLineByLine(new[]
305
+ {
306
+ @"dKO7wPCF.XLmq6KnqybHHgcBBqtU:ProgramFilesFolder:a\b\c",
307
+ @"ProgramFilesFolder:TARGETDIR:PFiles",
308
+ @"TARGETDIR::SourceDir"
309
+ }, dirSymbols.OrderBy(d => d.Id.Id).Select(d => d.Id.Id + ":" + d.ParentDirectoryRef + ":" + d.Name).OrderBy(s => s).ToArray());
310
+
311
+ var data = WindowsInstallerData.Load(wixpdbPath);
312
+ var directoryRows = data.Tables["Directory"].Rows;
313
+ WixAssert.CompareLineByLine(new[]
314
+ {
315
+ @"d1nVb5_zcCwRCz7i2YXNAofGRmfc:ProgramFilesFolder:a",
316
+ @"dijlG.bNicFgvj1_DujiGg9EBGrQ:d1nVb5_zcCwRCz7i2YXNAofGRmfc:b",
317
+ @"dKO7wPCF.XLmq6KnqybHHgcBBqtU:dijlG.bNicFgvj1_DujiGg9EBGrQ:c",
318
+ "ProgramFilesFolder:TARGETDIR:PFiles",
319
+ "TARGETDIR::SourceDir"
320
+ }, directoryRows.Select(r => r.FieldAsString(0) + ":" + r.FieldAsString(1) + ":" + r.FieldAsString(2)).OrderBy(s => s).ToArray());
321
+ }
322
+ }
323
}
324
}
src/wix/test/WixToolsetTest.CoreIntegration/TestData/Directory/RedundantSubdirectoryInSecondSection.wxs
new
+26
@@ -0,0 +1,26 @@
1
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
2
+ <Package Name="~RedundantSubdirectories" Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="12E4699F-E774-4D05-8A01-5BDD41BBA127" Compressed="no">
3
+ <MajorUpgrade DowngradeErrorMessage="A newer version of [ProductName] is already installed." />
4
+
5
+ <Feature Id="ProductFeature">
6
+ <!-- NotIncludeButFirst will define the subdirectory and IncludedButSecond needs the same symbols, this tests linking order -->
7
+ <ComponentGroupRef Id="IncludedButSecond" />
8
+ </Feature>
9
+ </Package>
10
+
11
+ <Fragment Id="NotIncludedButFirst">
12
+ <ComponentGroup Id="NotIncludedButFirst">
13
+ <Component Directory="ProgramFilesFolder" Subdirectory="a\b\c">
14
+ <File Name="notincluded.txt" Source="test.txt" />
15
+ </Component>
16
+ </ComponentGroup>
17
+ </Fragment>
18
+
19
+ <Fragment Id="IncludedButSecond">
20
+ <ComponentGroup Id="IncludedButSecond">
21
+ <Component Directory="ProgramFilesFolder" Subdirectory="a\b\c">
22
+ <File Name="included.txt" Source="test.txt" />
23
+ </Component>
24
+ </ComponentGroup>
25
+ </Fragment>
26
+</Wix>