From d9c37992dfabfce5d6031fb565473f45652733f8 Mon Sep 17 00:00:00 2001 From: Bob Arnson Date: Sat, 3 Aug 2024 16:37:31 -0400 Subject: [PATCH] Ensure naked files take Subdirectory into account. Naked files generated their ids using the attributes that are common to both naked and clothed files. But naked files also support @Subdirectory to magic up a subdirectory in a specified directory (@Directory) or the default INSTALLFOLDER. That subdirectory needs to factor in to the generated file id (which is then used as the component id too). Without it, generated ids for files with the same name but from different @Subdirectory values would be duplicated. (Authored file ids must also continue to be supported.) Naked files now generate different file and component ids. :( Fixes https://github.com/wixtoolset/issues/issues/8674 --- src/wix/WixToolset.Core/Compiler.cs | 44 +++++++++++++++---- src/wix/WixToolset.Core/Linker.cs | 2 +- .../HarvestFilesFixture.cs | 35 +++++++++++++++ .../NakedFileFixture.cs | 7 +++ .../TestData/HarvestFiles/DuplicateNames.wxs | 5 +++ .../TestData/HarvestFiles/dupes/a/file.x | 0 .../TestData/HarvestFiles/dupes/b/file.x | 0 .../TestData/HarvestFiles/dupes/c/file.x | 0 .../TestData/HarvestFiles/dupes/d/file.x | 0 .../TestData/NakedFile/DuplicateNames.wxs | 9 ++++ .../TestData/NakedFile/Package.wxs | 10 ++--- .../PackageWithDefaultInstallFolder.wxs | 10 ++--- 12 files changed, 100 insertions(+), 22 deletions(-) create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/DuplicateNames.wxs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/a/file.x create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/b/file.x create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/c/file.x create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/d/file.x create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/DuplicateNames.wxs diff --git a/src/wix/WixToolset.Core/Compiler.cs b/src/wix/WixToolset.Core/Compiler.cs index d13f1a682..dd79ab832 100644 --- a/src/wix/WixToolset.Core/Compiler.cs +++ b/src/wix/WixToolset.Core/Compiler.cs @@ -5348,7 +5348,7 @@ private YesNoType ParseFileElementAttributes(XElement node, string componentId, } } - if (null == id) + if (null == id && !isNakedFile) { id = this.Core.CreateIdentifier("fil", directoryId, name); } @@ -5610,8 +5610,6 @@ private void ParseNakedFileElement(XElement node, ComplexReferenceParentType par string condition = null; string subdirectory = null; - var keyPath = this.ParseFileElementAttributes(node, "@WixTemporaryComponentId", directoryId, diskId: CompilerConstants.IntegerNotSet, sourcePath, out var _, componentGuid: "*", isNakedFile: true, out var fileSymbol, out var assemblySymbol); - if (!this.Core.EncounteredError) { // Naked files have additional attributes to handle common component attributes. @@ -5661,13 +5659,44 @@ private void ParseNakedFileElement(XElement node, ComplexReferenceParentType par directoryId = this.HandleSubdirectory(sourceLineNumbers, node, directoryId, subdirectory, "Directory", "Subdirectory"); - this.Core.AddSymbol(new ComponentSymbol(sourceLineNumbers, fileSymbol.Id) + var keyPath = this.ParseFileElementAttributes(node, "@WixTemporaryComponentId", directoryId, diskId: CompilerConstants.IntegerNotSet, sourcePath, out var _, componentGuid: "*", isNakedFile: true, out var fileSymbol, out var assemblySymbol); + + // Now that we have all the data we need to generate a good id, do + // so and create a file and component symbol with the right data. + var id = fileSymbol.Id ?? this.Core.CreateIdentifier("nkf", directoryId, fileSymbol.Name, condition, win64.ToString()); + + this.Core.AddSymbol(new FileSymbol(sourceLineNumbers, id) + { + ComponentRef = id.Id, + Name = fileSymbol.Name, + ShortName = fileSymbol.ShortName, + FileSize = fileSymbol.FileSize, + Version = fileSymbol.Version, + Language = fileSymbol.Language, + Attributes = fileSymbol.Attributes, + DirectoryRef = fileSymbol.DirectoryRef, + DiskId = fileSymbol.DiskId, + Source = fileSymbol.Source, + FontTitle = fileSymbol.FontTitle, + SelfRegCost = fileSymbol.SelfRegCost, + BindPath = fileSymbol.BindPath, + PatchGroup = fileSymbol.PatchGroup, + PatchAttributes = fileSymbol.PatchAttributes, + RetainLengths = fileSymbol.RetainLengths, + IgnoreOffsets = fileSymbol.IgnoreOffsets, + IgnoreLengths = fileSymbol.IgnoreLengths, + RetainOffsets = fileSymbol.RetainOffsets, + SymbolPaths = fileSymbol.SymbolPaths, + + }); + + this.Core.AddSymbol(new ComponentSymbol(sourceLineNumbers, id) { ComponentId = "*", DirectoryRef = directoryId, Location = ComponentLocation.LocalOnly, Condition = condition, - KeyPath = fileSymbol.Id.Id, + KeyPath = id.Id, KeyPathType = ComponentKeyPathType.File, DisableRegistryReflection = false, NeverOverwrite = false, @@ -5679,9 +5708,6 @@ private void ParseNakedFileElement(XElement node, ComplexReferenceParentType par Win64 = win64, }); - fileSymbol.ComponentRef = fileSymbol.Id.Id; - this.Core.AddSymbol(fileSymbol); - if (assemblySymbol != null) { this.Core.AddSymbol(assemblySymbol); @@ -5692,7 +5718,7 @@ private void ParseNakedFileElement(XElement node, ComplexReferenceParentType par if (ComplexReferenceParentType.Unknown != parentType && null != parentId) // if parent was provided, add a complex reference to that. { // If the naked file's component is defined directly under a feature, then mark the complex reference primary. - this.Core.CreateComplexReference(sourceLineNumbers, parentType, parentId, null, ComplexReferenceChildType.Component, fileSymbol.Id.Id, ComplexReferenceParentType.Feature == parentType); + this.Core.CreateComplexReference(sourceLineNumbers, parentType, parentId, null, ComplexReferenceChildType.Component, id.Id, ComplexReferenceParentType.Feature == parentType); } } } diff --git a/src/wix/WixToolset.Core/Linker.cs b/src/wix/WixToolset.Core/Linker.cs index 43a41eac9..b10a12d84 100644 --- a/src/wix/WixToolset.Core/Linker.cs +++ b/src/wix/WixToolset.Core/Linker.cs @@ -716,7 +716,7 @@ private void FlattenGroup(string parentTypeAndId, Stack loopDetector, Di var childTypeAndId = this.CombineTypeAndId(wixComplexReferenceRow.ChildType, wixComplexReferenceRow.Child); if (loopDetector.Contains(childTypeAndId)) { - // Create a comma delimited list of the references that participate in the + // Create an arrow-delimited list of the references that participate in the // loop for the error message. Start at the bottom of the stack and work the // way up to present the loop as a directed graph. var loop = String.Join(" -> ", loopDetector); diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/HarvestFilesFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/HarvestFilesFixture.cs index 821660ffe..e1615a018 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/HarvestFilesFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/HarvestFilesFixture.cs @@ -291,6 +291,41 @@ public void CanHarvestFilesInFiveLines() Build("PackageFiveLiner.wxs", (msiPath, _) => AssertFileIdsAndTargetPaths(msiPath, expected)); } + [Fact] + public void CanHarvestFilesWithDuplicateNames() + { + var expectedFiles = new[] + { + "File:fls47G631z.20kTPzBwIPjGuWifsVo\tfls47G631z.20kTPzBwIPjGuWifsVo\tfile.x\t0\t\t\t512\t2", + "File:flshOXYGoD0VHcQhrkIILRPNOVbYuM\tflshOXYGoD0VHcQhrkIILRPNOVbYuM\tfile.x\t0\t\t\t512\t1", + "File:flsp2QdFvBeSwll1i5tN0jM72w3Hu4\tflsp2QdFvBeSwll1i5tN0jM72w3Hu4\tfile.x\t0\t\t\t512\t3", + "File:flsqopW5ihQpwCTF7t_51GkHd4Hf6s\tflsqopW5ihQpwCTF7t_51GkHd4Hf6s\tfile.x\t0\t\t\t512\t4", + }; + + var expectedFilesAndDirectories = new[] + { + @"fls47G631z.20kTPzBwIPjGuWifsVo=PFiles\Example Corporation MsiPackage\b\file.x", + @"flshOXYGoD0VHcQhrkIILRPNOVbYuM=PFiles\Example Corporation MsiPackage\a\file.x", + @"flsp2QdFvBeSwll1i5tN0jM72w3Hu4=PFiles\Example Corporation MsiPackage\c\file.x", + @"flsqopW5ihQpwCTF7t_51GkHd4Hf6s=PFiles\Example Corporation MsiPackage\d\file.x", + }; + + Build("DuplicateNames.wxs", (msiPath, result) => AssertFileAndComponentIdsAndTargetPaths(msiPath, result, expectedFiles, expectedFilesAndDirectories)); + } + + private static void AssertFileAndComponentIdsAndTargetPaths(string msiPath, WixRunnerResult result, string[] expectedFiles, string[] expectedFilesAndDirectories) + { + result.AssertSuccess(); + + var files = Query.QueryDatabase(msiPath, new[] { "File" }) + .OrderBy(s => s) + .ToArray(); + + Assert.Equal(expectedFiles, files); + + AssertFileIdsAndTargetPaths(msiPath, expectedFilesAndDirectories); + } + private static void AssertFileIdsAndTargetPaths(string msiPath, string[] expected) { var pkg = new WixToolset.Dtf.WindowsInstaller.Package.InstallPackage(msiPath, diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/NakedFileFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/NakedFileFixture.cs index 6d874ff05..375fee754 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/NakedFileFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/NakedFileFixture.cs @@ -105,6 +105,13 @@ public void CanBuildNakedFilesUnderPackage() AssertFileComponentIds(4, rows); } + [Fact] + public void CanBuildNakedFilesUnderPackageWithDuplicateNames() + { + var rows = BuildAndQueryComponentAndFileTables("DuplicateNames.wxs"); + AssertFileComponentIds(5, rows); + } + [Fact] public void CanBuildNakedFilesUnderPackageWithDefaultInstallFolder() { diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/DuplicateNames.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/DuplicateNames.wxs new file mode 100644 index 000000000..c3b08b69e --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/DuplicateNames.wxs @@ -0,0 +1,5 @@ + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/a/file.x b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/a/file.x new file mode 100644 index 000000000..e69de29bb diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/b/file.x b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/b/file.x new file mode 100644 index 000000000..e69de29bb diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/c/file.x b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/c/file.x new file mode 100644 index 000000000..e69de29bb diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/d/file.x b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/HarvestFiles/dupes/d/file.x new file mode 100644 index 000000000..e69de29bb diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/DuplicateNames.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/DuplicateNames.wxs new file mode 100644 index 000000000..edccc5044 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/DuplicateNames.wxs @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/Package.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/Package.wxs index e5dd94e02..9ddd4787b 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/Package.wxs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/Package.wxs @@ -1,10 +1,8 @@ - - - - - - + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/PackageWithDefaultInstallFolder.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/PackageWithDefaultInstallFolder.wxs index 824f35014..01ca77350 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/PackageWithDefaultInstallFolder.wxs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/NakedFile/PackageWithDefaultInstallFolder.wxs @@ -1,10 +1,8 @@ - - - - - - + + + +