From a1307cd4e76a89598c53cb68309358a7012db553 Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Fri, 9 Sep 2022 16:03:29 -0500 Subject: Move `Bundle/@CommandLineVariables` into Bal.wixext. Implements 6858 --- .../WixToolset.Mba.Core/IOverridableVariables.cs | 10 ++-- src/api/burn/WixToolset.Mba.Core/MbaCommand.cs | 5 +- .../WixToolset.Mba.Core/OverridableVariables.cs | 44 ++++++++++-------- src/api/burn/balutil/balinfo.cpp | 47 +++++++++---------- src/api/burn/balutil/inc/balinfo.h | 2 +- .../wix/WixToolset.Data/Symbols/WixBundleSymbol.cs | 14 ------ src/burn/engine/core.cpp | 12 +---- src/burn/engine/variable.cpp | 51 ++++++++++---------- src/burn/engine/variable.h | 11 ++--- src/burn/test/BurnUnitTest/ManifestTest.cpp | 1 - src/burn/test/BurnUnitTest/RegistrationTest.cpp | 2 - .../BasicFunctionality_BundleA_manifest.xml | 2 +- .../PlanTest/BundlePackage_Multiple_manifest.xml | 2 +- .../ExePackage_PerUserArpEntry_manifest.xml | 2 +- .../TestData/PlanTest/Failure_BundleD_manifest.xml | 2 +- .../PlanTest/MsiTransaction_BundleAv1_manifest.xml | 2 +- .../PlanTest/MsuPackageFixture_manifest.xml | 2 +- .../PlanTest/Slipstream_BundleA_manifest.xml | 2 +- .../Slipstream_BundleA_modified_manifest.xml | 2 +- src/burn/test/BurnUnitTest/VariableTest.cpp | 10 ++-- .../test/WixToolsetTest.Bal/BalExtensionFixture.cs | 9 +++- .../TestData/Overridable/Bundle.wxs | 2 +- .../TestData/Overridable/WrongCaseBundle.wxs | 3 +- src/ext/Bal/wixext/BalBurnBackendExtension.cs | 40 ++++++++++++++-- src/ext/Bal/wixext/BalCompiler.cs | 36 +++++++++++++++ src/ext/Bal/wixext/BalErrors.cs | 12 +++-- src/ext/Bal/wixext/Symbols/BalSymbolDefinitions.cs | 5 ++ .../wixext/Symbols/WixStdbaCommandLineSymbol.cs | 54 ++++++++++++++++++++++ .../burn/TestData/VariableTests/BundleA/Bundle.wxs | 40 ++++++++++++++++ .../TestData/VariableTests/BundleA/BundleA.wixproj | 3 -- .../burn/TestData/VariableTests/BundleB/Bundle.wxs | 40 ---------------- .../TestData/VariableTests/BundleB/BundleB.wixproj | 3 ++ .../TestData/VariableTests/BundleB/BundleB.wxs | 1 + .../burn/TestData/WixStdBaTests/BundleA/Bundle.wxs | 2 +- .../burn/WixToolsetTest.BurnE2E/VariableTests.cs | 10 ++-- ...CreateBootstrapperApplicationManifestCommand.cs | 19 -------- .../Bundles/CreateBurnManifestCommand.cs | 14 ------ src/wix/WixToolset.Core/Compiler_Bundle.cs | 17 ------- .../BundleFixture.cs | 6 --- 39 files changed, 305 insertions(+), 236 deletions(-) create mode 100644 src/ext/Bal/wixext/Symbols/WixStdbaCommandLineSymbol.cs create mode 100644 src/test/burn/TestData/VariableTests/BundleA/Bundle.wxs delete mode 100644 src/test/burn/TestData/VariableTests/BundleB/Bundle.wxs diff --git a/src/api/burn/WixToolset.Mba.Core/IOverridableVariables.cs b/src/api/burn/WixToolset.Mba.Core/IOverridableVariables.cs index 1ffe50e4..2b26d7b6 100644 --- a/src/api/burn/WixToolset.Mba.Core/IOverridableVariables.cs +++ b/src/api/burn/WixToolset.Mba.Core/IOverridableVariables.cs @@ -9,14 +9,14 @@ namespace WixToolset.Mba.Core /// public enum VariableCommandLineType { - /// - /// Similar to Windows Installer, all variable names specified on the command line are automatically converted to upper case. - /// - UpperCase, /// /// All variable names specified on the command line must match the case specified when building the bundle. /// CaseSensitive, + /// + /// Variable names specified on the command line do not have to match the case specified when building the bundle. + /// + CaseInsensitive, } /// @@ -34,4 +34,4 @@ namespace WixToolset.Mba.Core /// IDictionary Variables { get; } } -} \ No newline at end of file +} diff --git a/src/api/burn/WixToolset.Mba.Core/MbaCommand.cs b/src/api/burn/WixToolset.Mba.Core/MbaCommand.cs index 8917a334..bd93b7e7 100644 --- a/src/api/burn/WixToolset.Mba.Core/MbaCommand.cs +++ b/src/api/burn/WixToolset.Mba.Core/MbaCommand.cs @@ -2,6 +2,7 @@ namespace WixToolset.Mba.Core { + using System; using System.Collections.Generic; /// @@ -21,11 +22,11 @@ namespace WixToolset.Mba.Core { foreach (var kvp in this.Variables) { - var key = overridableVariables.CommandLineType == VariableCommandLineType.UpperCase ? kvp.Key.ToUpperInvariant() : kvp.Key; + var key = kvp.Key; if (!overridableVariables.Variables.TryGetValue(key, out var overridableVariable)) { - engine.Log(LogLevel.Error, string.Format("Ignoring attempt to set non-overridable variable: '{0}'.", key)); + engine.Log(LogLevel.Error, String.Format("Ignoring attempt to set non-overridable variable: '{0}'.", key)); } else { diff --git a/src/api/burn/WixToolset.Mba.Core/OverridableVariables.cs b/src/api/burn/WixToolset.Mba.Core/OverridableVariables.cs index 148acb34..121c6aee 100644 --- a/src/api/burn/WixToolset.Mba.Core/OverridableVariables.cs +++ b/src/api/burn/WixToolset.Mba.Core/OverridableVariables.cs @@ -29,37 +29,43 @@ namespace WixToolset.Mba.Core { XmlNamespaceManager namespaceManager = new XmlNamespaceManager(root.NameTable); namespaceManager.AddNamespace("p", BootstrapperApplicationData.XMLNamespace); - XPathNavigator commandLineNode = root.SelectSingleNode("/p:BootstrapperApplicationData/p:CommandLine", namespaceManager); + XPathNavigator commandLineNode = root.SelectSingleNode("/p:BootstrapperApplicationData/p:WixStdbaCommandLine", namespaceManager); XPathNodeIterator nodes = root.Select("/p:BootstrapperApplicationData/p:WixStdbaOverridableVariable", namespaceManager); var overridableVariables = new OverridableVariablesInfo(); + IEqualityComparer variableNameComparer; if (commandLineNode == null) - { - throw new Exception("Failed to select command line information."); - } - - string variablesValue = BootstrapperApplicationData.GetAttribute(commandLineNode, "Variables"); - - if (variablesValue == null) - { - throw new Exception("Failed to get command line variable type."); - } - - if (variablesValue.Equals("upperCase", StringComparison.InvariantCulture)) - { - overridableVariables.CommandLineType = VariableCommandLineType.UpperCase; - } - else if (variablesValue.Equals("caseSensitive", StringComparison.InvariantCulture)) { overridableVariables.CommandLineType = VariableCommandLineType.CaseSensitive; + variableNameComparer = StringComparer.InvariantCulture; } else { - throw new Exception(string.Format("Unknown command line variable type: '{0}'", variablesValue)); + string variablesValue = BootstrapperApplicationData.GetAttribute(commandLineNode, "Variables"); + + if (variablesValue == null) + { + throw new Exception("Failed to get command line variable type."); + } + + if (variablesValue.Equals("caseInsensitive", StringComparison.InvariantCulture)) + { + overridableVariables.CommandLineType = VariableCommandLineType.CaseInsensitive; + variableNameComparer = StringComparer.InvariantCultureIgnoreCase; + } + else if (variablesValue.Equals("caseSensitive", StringComparison.InvariantCulture)) + { + overridableVariables.CommandLineType = VariableCommandLineType.CaseSensitive; + variableNameComparer = StringComparer.InvariantCulture; + } + else + { + throw new Exception(String.Format("Unknown command line variable type: '{0}'", variablesValue)); + } } - overridableVariables.Variables = new Dictionary(); + overridableVariables.Variables = new Dictionary(variableNameComparer); foreach (XPathNavigator node in nodes) { diff --git a/src/api/burn/balutil/balinfo.cpp b/src/api/burn/balutil/balinfo.cpp index 52a7f911..751ba4f1 100644 --- a/src/api/burn/balutil/balinfo.cpp +++ b/src/api/burn/balutil/balinfo.cpp @@ -348,7 +348,6 @@ DAPI_(HRESULT) BalSetOverridableVariablesFromEngine( ) { HRESULT hr = S_OK; - LPWSTR sczKey = NULL; BAL_INFO_OVERRIDABLE_VARIABLE* pOverridableVariable = NULL; for (DWORD i = 0; i < pCommand->cVariables; ++i) @@ -356,14 +355,6 @@ DAPI_(HRESULT) BalSetOverridableVariablesFromEngine( LPCWSTR wzVariableName = pCommand->rgVariableNames[i]; LPCWSTR wzVariableValue = pCommand->rgVariableValues[i]; - if (BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE == pOverridableVariables->commandLineType) - { - hr = StrAllocStringToUpperInvariant(&sczKey, wzVariableName, 0); - ExitOnFailure(hr, "Failed to upper case variable name."); - - wzVariableName = sczKey; - } - hr = DictGetValue(pOverridableVariables->sdVariables, wzVariableName, reinterpret_cast(&pOverridableVariable)); if (E_NOTFOUND == hr) { @@ -378,8 +369,6 @@ DAPI_(HRESULT) BalSetOverridableVariablesFromEngine( } LExit: - ReleaseStr(sczKey); - return hr; } @@ -604,29 +593,37 @@ static HRESULT ParseOverridableVariablesFromXml( { HRESULT hr = S_OK; IXMLDOMNode* pCommandLineNode = NULL; + BOOL fXmlFound = FALSE; LPWSTR scz = NULL; IXMLDOMNode* pNode = NULL; IXMLDOMNodeList* pNodes = NULL; BAL_INFO_OVERRIDABLE_VARIABLE* pOverridableVariable = NULL; - hr = XmlSelectSingleNode(pixdManifest, L"/BootstrapperApplicationData/CommandLine", &pCommandLineNode); - ExitOnRequiredXmlQueryFailure(hr, "Failed to select command line information."); - - // @Variables - hr = XmlGetAttributeEx(pCommandLineNode, L"Variables", &scz); - ExitOnRequiredXmlQueryFailure(hr, "Failed to get command line variable type."); + hr = XmlSelectSingleNode(pixdManifest, L"/BootstrapperApplicationData/WixStdbaCommandLine", &pCommandLineNode); + ExitOnOptionalXmlQueryFailure(hr, fXmlFound, "Failed to select command line information."); - if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"upperCase", -1)) - { - pOverridableVariables->commandLineType = BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE; - } - else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"caseSensitive", -1)) + if (!fXmlFound) { pOverridableVariables->commandLineType = BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_SENSITIVE; } else { - ExitWithRootFailure(hr, E_INVALIDARG, "Invalid value for CommandLine/@Variables: %ls", scz); + // @Variables + hr = XmlGetAttributeEx(pCommandLineNode, L"VariableType", &scz); + ExitOnRequiredXmlQueryFailure(hr, "Failed to get command line variable type."); + + if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"caseInsensitive", -1)) + { + pOverridableVariables->commandLineType = BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_INSENSITIVE; + } + else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"caseSensitive", -1)) + { + pOverridableVariables->commandLineType = BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_SENSITIVE; + } + else + { + ExitWithRootFailure(hr, E_INVALIDARG, "Invalid value for CommandLine/@Variables: %ls", scz); + } } // Get the list of variables users can override on the command line. @@ -638,7 +635,9 @@ static HRESULT ParseOverridableVariablesFromXml( if (pOverridableVariables->cVariables) { - hr = DictCreateWithEmbeddedKey(&pOverridableVariables->sdVariables, pOverridableVariables->cVariables, reinterpret_cast(&pOverridableVariables->rgVariables), offsetof(BAL_INFO_OVERRIDABLE_VARIABLE, sczName), DICT_FLAG_NONE); + DICT_FLAG dfFlags = BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_INSENSITIVE == pOverridableVariables->commandLineType ? DICT_FLAG_CASEINSENSITIVE : DICT_FLAG_NONE; + + hr = DictCreateWithEmbeddedKey(&pOverridableVariables->sdVariables, pOverridableVariables->cVariables, reinterpret_cast(&pOverridableVariables->rgVariables), offsetof(BAL_INFO_OVERRIDABLE_VARIABLE, sczName), dfFlags); ExitOnFailure(hr, "Failed to create the overridable variables string dictionary."); hr = MemAllocArray(reinterpret_cast(&pOverridableVariables->rgVariables), sizeof(pOverridableVariable), pOverridableVariables->cVariables); diff --git a/src/api/burn/balutil/inc/balinfo.h b/src/api/burn/balutil/inc/balinfo.h index 818ff5ef..3ef3a61a 100644 --- a/src/api/burn/balutil/inc/balinfo.h +++ b/src/api/burn/balutil/inc/balinfo.h @@ -40,8 +40,8 @@ typedef enum _BAL_INFO_RESTART typedef enum _BAL_INFO_VARIABLE_COMMAND_LINE_TYPE { - BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE, BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_SENSITIVE, + BAL_INFO_VARIABLE_COMMAND_LINE_TYPE_CASE_INSENSITIVE, } BAL_INFO_VARIABLE_COMMAND_LINE_TYPE; diff --git a/src/api/wix/WixToolset.Data/Symbols/WixBundleSymbol.cs b/src/api/wix/WixToolset.Data/Symbols/WixBundleSymbol.cs index c4db0c21..ffeb5f3b 100644 --- a/src/api/wix/WixToolset.Data/Symbols/WixBundleSymbol.cs +++ b/src/api/wix/WixToolset.Data/Symbols/WixBundleSymbol.cs @@ -33,7 +33,6 @@ namespace WixToolset.Data new IntermediateFieldDefinition(nameof(WixBundleSymbolFields.BundleId), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(WixBundleSymbolFields.ProviderKey), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(WixBundleSymbolFields.InProgressName), IntermediateFieldType.String), - new IntermediateFieldDefinition(nameof(WixBundleSymbolFields.CommandLineVariables), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(WixBundleSymbolFields.DisableModify), IntermediateFieldType.String), }, typeof(WixBundleSymbol)); @@ -69,7 +68,6 @@ namespace WixToolset.Data.Symbols BundleId, ProviderKey, InProgressName, - CommandLineVariables, DisableModify, } @@ -81,12 +79,6 @@ namespace WixToolset.Data.Symbols PerMachine = 0x2, } - public enum WixBundleCommandLineVariables - { - UpperCase, - CaseSensitive, - } - public enum WixBundleModifyType { Allowed = 0, @@ -244,12 +236,6 @@ namespace WixToolset.Data.Symbols set => this.Set((int)WixBundleSymbolFields.InProgressName, value); } - public WixBundleCommandLineVariables CommandLineVariables - { - get => (WixBundleCommandLineVariables)this.Fields[(int)WixBundleSymbolFields.CommandLineVariables].AsNumber(); - set => this.Set((int)WixBundleSymbolFields.CommandLineVariables, (int)value); - } - public WixBundleModifyType DisableModify { get diff --git a/src/burn/engine/core.cpp b/src/burn/engine/core.cpp index 93b9c002..c443e0c8 100644 --- a/src/burn/engine/core.cpp +++ b/src/burn/engine/core.cpp @@ -2203,18 +2203,10 @@ static HRESULT GetSanitizedCommandLine( const wchar_t* pwc = wcschr(argv[i], L'='); if (pwc) { - if (BURN_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE == pVariables->commandLineType) - { - hr = StrAllocStringToUpperInvariant(&sczVariableName, argv[i], pwc - argv[i]); - } - else - { - hr = StrAllocString(&sczVariableName, argv[i], pwc - argv[i]); - } + hr = StrAllocString(&sczVariableName, argv[i], pwc - argv[i]); ExitOnFailure(hr, "Failed to copy variable name."); - hr = VariableIsHidden(pVariables, sczVariableName, &fHidden); - ExitOnFailure(hr, "Failed to determine whether variable is hidden."); + fHidden = VariableIsHiddenCommandLine(pVariables, sczVariableName); if (fHidden) { diff --git a/src/burn/engine/variable.cpp b/src/burn/engine/variable.cpp index 3947e29e..9b6fecaf 100644 --- a/src/burn/engine/variable.cpp +++ b/src/burn/engine/variable.cpp @@ -342,7 +342,6 @@ extern "C" HRESULT VariablesParseFromXml( { HRESULT hr = S_OK; BOOL fXmlFound = FALSE; - IXMLDOMNode* pixnCommandLine = NULL; IXMLDOMNodeList* pixnNodes = NULL; IXMLDOMNode* pixnNode = NULL; DWORD cNodes = 0; @@ -356,27 +355,6 @@ extern "C" HRESULT VariablesParseFromXml( ::EnterCriticalSection(&pVariables->csAccess); - // select command-line node - hr = XmlSelectSingleNode(pixnBundle, L"CommandLine", &pixnCommandLine); - ExitOnRequiredXmlQueryFailure(hr, "Failed to select CommandLine node."); - - // @Variables - hr = XmlGetAttributeEx(pixnCommandLine, L"Variables", &scz); - ExitOnRequiredXmlQueryFailure(hr, "Failed to get CommandLine/@Variables."); - - if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"upperCase", -1)) - { - pVariables->commandLineType = BURN_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE; - } - else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, scz, -1, L"caseSensitive", -1)) - { - pVariables->commandLineType = BURN_VARIABLE_COMMAND_LINE_TYPE_CASE_SENSITIVE; - } - else - { - ExitWithRootFailure(hr, E_INVALIDARG, "Invalid value for CommandLine/@Variables: %ls", scz); - } - // select variable nodes hr = XmlSelectNodes(pixnBundle, L"Variable", &pixnNodes); ExitOnFailure(hr, "Failed to select variable nodes."); @@ -507,7 +485,6 @@ extern "C" HRESULT VariablesParseFromXml( LExit: ::LeaveCriticalSection(&pVariables->csAccess); - ReleaseObject(pixnCommandLine); ReleaseObject(pixnNodes); ReleaseObject(pixnNode); ReleaseStr(scz); @@ -1150,6 +1127,32 @@ LExit: return hr; } +extern "C" BOOL VariableIsHiddenCommandLine( + __in BURN_VARIABLES* pVariables, + __in_z LPCWSTR wzVariable + ) +{ + BURN_VARIABLE* pVariable = NULL; + BOOL fHidden = FALSE; + + ::EnterCriticalSection(&pVariables->csAccess); + + for (DWORD i = 0; i < pVariables->cVariables; ++i) + { + pVariable = pVariables->rgVariables + i; + + if (pVariable->fHidden && CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, pVariable->sczName, -1, wzVariable, -1)) + { + fHidden = TRUE; + break; + } + } + + ::LeaveCriticalSection(&pVariables->csAccess); + + return fHidden; +} + // internal function definitions @@ -1587,7 +1590,7 @@ static HRESULT InsertUserVariable( if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, 0, wzVariable, 3, L"Wix", 3)) { - ExitWithRootFailure(hr, E_INVALIDARG, "Attempted to insert variable with reserved prefix: %ls", wzVariable); + ExitWithRootFailure(hr, E_INVALIDARG, "Attempted to insert variable with reserved prefix: %ls", wzVariable); } hr = InsertVariable(pVariables, wzVariable, iPosition); diff --git a/src/burn/engine/variable.h b/src/burn/engine/variable.h index f8d3e1e2..309d7ab9 100644 --- a/src/burn/engine/variable.h +++ b/src/burn/engine/variable.h @@ -26,12 +26,6 @@ typedef HRESULT (*PFN_INITIALIZEVARIABLE)( // constants -enum BURN_VARIABLE_COMMAND_LINE_TYPE -{ - BURN_VARIABLE_COMMAND_LINE_TYPE_UPPER_CASE, - BURN_VARIABLE_COMMAND_LINE_TYPE_CASE_SENSITIVE, -}; - enum BURN_VARIABLE_INTERNAL_TYPE { BURN_VARIABLE_INTERNAL_TYPE_NORMAL, // the BA can set this variable. @@ -61,7 +55,6 @@ typedef struct _BURN_VARIABLES DWORD dwMaxVariables; DWORD cVariables; BURN_VARIABLE* rgVariables; - BURN_VARIABLE_COMMAND_LINE_TYPE commandLineType; } BURN_VARIABLES; @@ -187,6 +180,10 @@ HRESULT VariableIsHidden( __in_z LPCWSTR wzVariable, __out BOOL* pfHidden ); +BOOL VariableIsHiddenCommandLine( + __in BURN_VARIABLES* pVariables, + __in_z LPCWSTR wzVariable + ); #if defined(__cplusplus) } diff --git a/src/burn/test/BurnUnitTest/ManifestTest.cpp b/src/burn/test/BurnUnitTest/ManifestTest.cpp index 5e2725bf..67e9c25f 100644 --- a/src/burn/test/BurnUnitTest/ManifestTest.cpp +++ b/src/burn/test/BurnUnitTest/ManifestTest.cpp @@ -43,7 +43,6 @@ namespace Bootstrapper " " "