From 923d3edd21da412b0fd6a09dfd3391913fe18c56 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Sat, 9 Jan 2021 01:37:43 -0800 Subject: Modularize XmlConfig/@ElementId and add tests for XmlConfig Fixes wixtoolset/issues#4698 Closes wixtoolset/issues#5447 --- src/ca/XmlConfig.cpp | 101 +++++++++++++-------- .../TestData/XmlConfig/Package.en-us.wxl | 11 +++ .../TestData/XmlConfig/Package.wxs | 27 ++++++ .../TestData/XmlConfigModule/Module.wxs | 34 +++++++ .../TestData/XmlConfigModule/my.xml | 1 + .../WixToolsetTest.Util/UtilExtensionFixture.cs | 27 ++++++ .../WixToolsetTest.Util/WixToolsetTest.Util.csproj | 36 +------- src/wixext/Symbols/XmlConfigSymbol.cs | 8 ++ src/wixext/UtilCompiler.cs | 4 +- src/wixext/UtilTableDefinitions.cs | 3 +- 10 files changed, 175 insertions(+), 77 deletions(-) create mode 100644 src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.en-us.wxl create mode 100644 src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.wxs create mode 100644 src/test/WixToolsetTest.Util/TestData/XmlConfigModule/Module.wxs create mode 100644 src/test/WixToolsetTest.Util/TestData/XmlConfigModule/my.xml diff --git a/src/ca/XmlConfig.cpp b/src/ca/XmlConfig.cpp index afd3dafb..0c2fae37 100644 --- a/src/ca/XmlConfig.cpp +++ b/src/ca/XmlConfig.cpp @@ -30,10 +30,10 @@ enum eXmlPreserveDate }; LPCWSTR vcsXmlConfigQuery = - L"SELECT `Wix4XmlConfig`.`Wix4XmlConfig`, `Wix4XmlConfig`.`File`, `Wix4XmlConfig`.`ElementPath`, `Wix4XmlConfig`.`VerifyPath`, `Wix4XmlConfig`.`Name`, " + L"SELECT `Wix4XmlConfig`.`Wix4XmlConfig`, `Wix4XmlConfig`.`File`, `Wix4XmlConfig`.`ElementId`, `Wix4XmlConfig`.`ElementPath`, `Wix4XmlConfig`.`VerifyPath`, `Wix4XmlConfig`.`Name`, " L"`Wix4XmlConfig`.`Value`, `Wix4XmlConfig`.`Flags`, `Wix4XmlConfig`.`Component_`, `Component`.`Attributes` " L"FROM `Wix4XmlConfig`,`Component` WHERE `Wix4XmlConfig`.`Component_`=`Component`.`Component` ORDER BY `File`, `Sequence`"; -enum eXmlConfigQuery { xfqXmlConfig = 1, xfqFile, xfqElementPath, xfqVerifyPath, xfqName, xfqValue, xfqXmlFlags, xfqComponent, xfqCompAttributes }; +enum eXmlConfigQuery { xfqXmlConfig = 1, xfqFile, xfqElementId, xfqElementPath, xfqVerifyPath, xfqName, xfqValue, xfqXmlFlags, xfqComponent, xfqCompAttributes }; struct XML_CONFIG_CHANGE { @@ -44,6 +44,7 @@ struct XML_CONFIG_CHANGE INSTALLSTATE isAction; WCHAR wzFile[MAX_PATH]; + LPWSTR pwzElementId; LPWSTR pwzElementPath; LPWSTR pwzVerifyPath; WCHAR wzName[MAX_DARWIN_COLUMN]; @@ -72,26 +73,32 @@ static HRESULT FreeXmlConfigChangeList( pxfcDelete = pxfcList; pxfcList = pxfcList->pxfcNext; + if (pxfcDelete->pwzElementId) + { + hr = MemFree(pxfcDelete->pwzElementId); + ExitOnFailure(hr, "failed to free xml config element id in change list item"); + } + if (pxfcDelete->pwzElementPath) { hr = MemFree(pxfcDelete->pwzElementPath); - ExitOnFailure(hr, "failed to free xml file element path in change list item"); + ExitOnFailure(hr, "failed to free xml config element path in change list item"); } if (pxfcDelete->pwzVerifyPath) { hr = MemFree(pxfcDelete->pwzVerifyPath); - ExitOnFailure(hr, "failed to free xml file verify path in change list item"); + ExitOnFailure(hr, "failed to free xml config verify path in change list item"); } if (pxfcDelete->pwzValue) { hr = MemFree(pxfcDelete->pwzValue); - ExitOnFailure(hr, "failed to free xml file value in change list item"); + ExitOnFailure(hr, "failed to free xml config value in change list item"); } hr = MemFree(pxfcDelete); - ExitOnFailure(hr, "failed to free xml file change list item"); + ExitOnFailure(hr, "failed to free xml config change list item"); } LExit: @@ -191,6 +198,10 @@ static HRESULT ReadXmlConfigTable( hr = WcaGetRecordInteger(hRec, xfqXmlFlags, &(*ppxfcTail)->iXmlFlags); ExitOnFailure(hr, "failed to get Wix4XmlConfig flags for Wix4XmlConfig: %ls", (*ppxfcTail)->wzId); + // Get the Element Id + hr = WcaGetRecordFormattedString(hRec, xfqElementId, &(*ppxfcTail)->pwzElementId); + ExitOnFailure(hr, "failed to get Element Id for Wix4XmlConfig: %ls", (*ppxfcTail)->wzId); + // Get the Element Path hr = WcaGetRecordFormattedString(hRec, xfqElementPath, &(*ppxfcTail)->pwzElementPath); ExitOnFailure(hr, "failed to get Element Path for Wix4XmlConfig: %ls", (*ppxfcTail)->wzId); @@ -259,45 +270,55 @@ static HRESULT ProcessChanges( pxfcCheck = *ppxfcHead; while (pxfcCheck) { - if (0 == lstrcmpW(pxfc->pwzElementPath, pxfcCheck->wzId) && 0 == pxfc->iXmlFlags - && XMLCONFIG_CREATE & pxfcCheck->iXmlFlags && XMLCONFIG_ELEMENT & pxfcCheck->iXmlFlags) + if (pxfc->pwzElementId) { - // We found a match. First, take it out of the current list - if (pxfc->pxfcPrev) + if (0 == lstrcmpW(pxfc->pwzElementId, pxfcCheck->wzId) + && 0 == pxfc->iXmlFlags + && XMLCONFIG_CREATE & pxfcCheck->iXmlFlags + && XMLCONFIG_ELEMENT & pxfcCheck->iXmlFlags) { - pxfc->pxfcPrev->pxfcNext = pxfc->pxfcNext; - } - else // it was the head. Update the head - { - *ppxfcHead = pxfc->pxfcNext; - } + // We found a match. First, take it out of the current list + if (pxfc->pxfcPrev) + { + pxfc->pxfcPrev->pxfcNext = pxfc->pxfcNext; + } + else // it was the head. Update the head + { + *ppxfcHead = pxfc->pxfcNext; + } - if (pxfc->pxfcNext) - { - pxfc->pxfcNext->pxfcPrev = pxfc->pxfcPrev; - } + if (pxfc->pxfcNext) + { + pxfc->pxfcNext->pxfcPrev = pxfc->pxfcPrev; + } - pxfc->pxfcNext = NULL; - pxfc->pxfcPrev = NULL; + pxfc->pxfcNext = NULL; + pxfc->pxfcPrev = NULL; - // Now, add this node to the end of the matched node's additional changes list - if (!pxfcCheck->pxfcAdditionalChanges) - { - pxfcCheck->pxfcAdditionalChanges = pxfc; - pxfcCheck->cAdditionalChanges = 1; + // Now, add this node to the end of the matched node's additional changes list + if (!pxfcCheck->pxfcAdditionalChanges) + { + pxfcCheck->pxfcAdditionalChanges = pxfc; + pxfcCheck->cAdditionalChanges = 1; + } + else + { + pxfcLast = pxfcCheck->pxfcAdditionalChanges; + cAdditionalChanges = 1; + while (pxfcLast->pxfcNext) + { + pxfcLast = pxfcLast->pxfcNext; + ++cAdditionalChanges; + } + pxfcLast->pxfcNext = pxfc; + pxfc->pxfcPrev = pxfcLast; + pxfcCheck->cAdditionalChanges = ++cAdditionalChanges; + } } else { - pxfcLast = pxfcCheck->pxfcAdditionalChanges; - cAdditionalChanges = 1; - while (pxfcLast->pxfcNext) - { - pxfcLast = pxfcLast->pxfcNext; - ++cAdditionalChanges; - } - pxfcLast->pxfcNext = pxfc; - pxfc->pxfcPrev = pxfcLast; - pxfcCheck->cAdditionalChanges = ++cAdditionalChanges; + hr = E_NOTFOUND; + ExitOnRootFailure(hr, "failed to find matching ElementId: %ls", pxfc->pwzElementId); } } @@ -381,9 +402,10 @@ static HRESULT WriteChangeData( HRESULT hr = S_OK; XML_CONFIG_CHANGE* pxfcAdditionalChanges = NULL; + LPCWSTR wzElementPath = pxfc->pwzElementId ? pxfc->pwzElementId : pxfc->pwzElementPath; - hr = WcaWriteStringToCaData(pxfc->pwzElementPath, ppwzCustomActionData); - ExitOnFailure(hr, "failed to write ElementPath to custom action data: %ls", pxfc->pwzElementPath); + hr = WcaWriteStringToCaData(wzElementPath, ppwzCustomActionData); + ExitOnFailure(hr, "failed to write ElementPath to custom action data: %ls", wzElementPath); hr = WcaWriteStringToCaData(pxfc->pwzVerifyPath, ppwzCustomActionData); ExitOnFailure(hr, "failed to write VerifyPath to custom action data: %ls", pxfc->pwzVerifyPath); @@ -1111,4 +1133,3 @@ LExit: } return WcaFinalize(er); } - diff --git a/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.en-us.wxl b/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.en-us.wxl new file mode 100644 index 00000000..38c12ac1 --- /dev/null +++ b/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.en-us.wxl @@ -0,0 +1,11 @@ + + + + + + A newer version of [ProductName] is already installed. + MsiPackage + + diff --git a/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.wxs b/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.wxs new file mode 100644 index 00000000..21f75f8f --- /dev/null +++ b/src/test/WixToolsetTest.Util/TestData/XmlConfig/Package.wxs @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + diff --git a/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/Module.wxs b/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/Module.wxs new file mode 100644 index 00000000..93f5eeb1 --- /dev/null +++ b/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/Module.wxs @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/my.xml b/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/my.xml new file mode 100644 index 00000000..bad25217 --- /dev/null +++ b/src/test/WixToolsetTest.Util/TestData/XmlConfigModule/my.xml @@ -0,0 +1 @@ +This is my.xml file. diff --git a/src/test/WixToolsetTest.Util/UtilExtensionFixture.cs b/src/test/WixToolsetTest.Util/UtilExtensionFixture.cs index a32a7d62..d9440614 100644 --- a/src/test/WixToolsetTest.Util/UtilExtensionFixture.cs +++ b/src/test/WixToolsetTest.Util/UtilExtensionFixture.cs @@ -172,6 +172,33 @@ namespace WixToolsetTest.Util }, results.OrderBy(s => s).ToArray()); } + [Fact] + public void CanBuildWithXmlConfig() + { + var folder = TestData.Get(@"TestData", "XmlConfig"); + var build = new Builder(folder, typeof(UtilExtensionFactory), new[] { folder }); + + var results = build.BuildAndQuery(BuildX64, "Wix4XmlConfig"); + WixAssert.CompareLineByLine(new[] + { + "Wix4XmlConfig:DelElement\t[INSTALLFOLDER]my.xml\t\t//root/sub\txxx\t\t\t289\tDel\t1", + }, results.OrderBy(s => s).ToArray()); + } + + [Fact] + public void CanBuildModuleWithXmlConfig() + { + var folder = TestData.Get(@"TestData", "XmlConfigModule"); + var build = new Builder(folder, typeof(UtilExtensionFactory), new[] { folder }); + + var results = build.BuildAndQuery(BuildX64, "Wix4XmlConfig"); + WixAssert.CompareLineByLine(new[] + { + "Wix4XmlConfig:AddElement.047730A5_30FE_4A62_A520_DA9381B8226A\t[my.xml.047730A5_30FE_4A62_A520_DA9381B8226A]\t\t//root/sub\txxx\t\t\t273\tParent.047730A5_30FE_4A62_A520_DA9381B8226A\t1", + "Wix4XmlConfig:ChildElement.047730A5_30FE_4A62_A520_DA9381B8226A\t[my.xml.047730A5_30FE_4A62_A520_DA9381B8226A]\tAddElement.047730A5_30FE_4A62_A520_DA9381B8226A\t\txxx\t\t\t0\tChild.047730A5_30FE_4A62_A520_DA9381B8226A\t1", + }, results.OrderBy(s => s).ToArray()); + } + [Fact] public void CanBuildBundleWithSearches() { diff --git a/src/test/WixToolsetTest.Util/WixToolsetTest.Util.csproj b/src/test/WixToolsetTest.Util/WixToolsetTest.Util.csproj index 50776c2c..e77ecbed 100644 --- a/src/test/WixToolsetTest.Util/WixToolsetTest.Util.csproj +++ b/src/test/WixToolsetTest.Util/WixToolsetTest.Util.csproj @@ -12,41 +12,7 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + diff --git a/src/wixext/Symbols/XmlConfigSymbol.cs b/src/wixext/Symbols/XmlConfigSymbol.cs index ca1cf047..6503a586 100644 --- a/src/wixext/Symbols/XmlConfigSymbol.cs +++ b/src/wixext/Symbols/XmlConfigSymbol.cs @@ -12,6 +12,7 @@ namespace WixToolset.Util new[] { new IntermediateFieldDefinition(nameof(XmlConfigSymbolFields.File), IntermediateFieldType.String), + new IntermediateFieldDefinition(nameof(XmlConfigSymbolFields.ElementId), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(XmlConfigSymbolFields.ElementPath), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(XmlConfigSymbolFields.VerifyPath), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(XmlConfigSymbolFields.Name), IntermediateFieldType.String), @@ -31,6 +32,7 @@ namespace WixToolset.Util.Symbols public enum XmlConfigSymbolFields { File, + ElementId, ElementPath, VerifyPath, Name, @@ -58,6 +60,12 @@ namespace WixToolset.Util.Symbols set => this.Set((int)XmlConfigSymbolFields.File, value); } + public string ElementId + { + get => this.Fields[(int)XmlConfigSymbolFields.ElementId].AsString(); + set => this.Set((int)XmlConfigSymbolFields.ElementId, value); + } + public string ElementPath { get => this.Fields[(int)XmlConfigSymbolFields.ElementPath].AsString(); diff --git a/src/wixext/UtilCompiler.cs b/src/wixext/UtilCompiler.cs index 68c7a234..7314570e 100644 --- a/src/wixext/UtilCompiler.cs +++ b/src/wixext/UtilCompiler.cs @@ -3706,13 +3706,15 @@ namespace WixToolset.Util var symbol = section.AddSymbol(new XmlConfigSymbol(sourceLineNumbers, id) { File = file, - ElementPath = elementId ?? elementPath, + ElementId = elementId, + ElementPath = elementPath, VerifyPath = verifyPath, Name = name, Value = value, Flags = flags, ComponentRef = componentId, }); + if (CompilerConstants.IntegerNotSet != sequence) { symbol.Sequence = sequence; diff --git a/src/wixext/UtilTableDefinitions.cs b/src/wixext/UtilTableDefinitions.cs index e5d0850f..1908915c 100644 --- a/src/wixext/UtilTableDefinitions.cs +++ b/src/wixext/UtilTableDefinitions.cs @@ -253,7 +253,8 @@ namespace WixToolset.Util { new ColumnDefinition("Wix4XmlConfig", ColumnType.String, 72, primaryKey: true, nullable: false, ColumnCategory.Identifier, description: "Primary key, non-localized token.", modularizeType: ColumnModularizeType.Column), new ColumnDefinition("File", ColumnType.Localized, 255, primaryKey: false, nullable: false, ColumnCategory.Formatted, description: "The .XML file in which to write the information", modularizeType: ColumnModularizeType.Property), - new ColumnDefinition("ElementPath", ColumnType.Localized, 0, primaryKey: false, nullable: false, ColumnCategory.Formatted, description: "The XPATH query for an element to modify or add children to. Can also be a foreign key reference to another Wix4XmlConfig row if no attributes are set and the row referenced is a create element row.", modularizeType: ColumnModularizeType.Property), + new ColumnDefinition("ElementId", ColumnType.String, 0, primaryKey: false, nullable: true, ColumnCategory.Identifier, keyTable: "Wix4XmlConfig", keyColumn: 1, description: "A foreign key reference to another Wix4XmlConfig row if no attributes are set and the row referenced is a create element row.", modularizeType: ColumnModularizeType.Column), + new ColumnDefinition("ElementPath", ColumnType.Localized, 0, primaryKey: false, nullable: true, ColumnCategory.Formatted, description: "The XPATH query for an element to modify or add children to. Must be null if ElementId is provided", modularizeType: ColumnModularizeType.Property), new ColumnDefinition("VerifyPath", ColumnType.Localized, 0, primaryKey: false, nullable: true, ColumnCategory.Formatted, description: "The XPATH query run from ElementPath to verify whether a repair is necessary. Also used to uninstall.", modularizeType: ColumnModularizeType.Property), new ColumnDefinition("Name", ColumnType.Localized, 255, primaryKey: false, nullable: true, ColumnCategory.Formatted, description: "The .XML file node to set/add in the element.", modularizeType: ColumnModularizeType.Property), new ColumnDefinition("Value", ColumnType.Localized, 0, primaryKey: false, nullable: true, ColumnCategory.Formatted, description: "The value to be written.", modularizeType: ColumnModularizeType.Property), -- cgit v1.2.3-55-g6feb