From 33ec6f9f2dafd6d890c385c8828ddfee08e6fff7 Mon Sep 17 00:00:00 2001 From: Nir Bar Date: Mon, 20 Nov 2023 18:00:03 +0200 Subject: [PATCH] ArpEntry reads QuietUninstallString or UninstallString, and uses UninstallArguments for the uninstall command line --- .../Symbols/WixBundleExePackageSymbol.cs | 17 +++++++ src/burn/engine/exeengine.cpp | 26 ++++++++-- src/burn/engine/package.h | 1 + .../Bundles/CreateBurnManifestCommand.cs | 10 ++++ src/wix/WixToolset.Core/Compiler_Bundle.cs | 49 +++++++++---------- .../ExePackageFixture.cs | 13 +++-- 6 files changed, 81 insertions(+), 35 deletions(-) diff --git a/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs b/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs index 741467164..0fcc6dd1b 100644 --- a/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs +++ b/src/api/wix/WixToolset.Data/Symbols/WixBundleExePackageSymbol.cs @@ -57,6 +57,7 @@ public enum WixBundleExePackageAttributes None = 0, Bundle = 1, ArpWin64 = 2, + ArpUseUninstallString = 4, } public class WixBundleExePackageSymbol : IntermediateSymbol @@ -157,6 +158,22 @@ public bool ArpWin64 } } + public bool ArpUseUninstallString + { + get { return this.Attributes.HasFlag(WixBundleExePackageAttributes.ArpUseUninstallString); } + set + { + if (value) + { + this.Attributes |= WixBundleExePackageAttributes.ArpUseUninstallString; + } + else + { + this.Attributes &= ~WixBundleExePackageAttributes.ArpUseUninstallString; + } + } + } + public bool Repairable => this.RepairCommand != null; public bool Uninstallable => this.UninstallCommand != null; diff --git a/src/burn/engine/exeengine.cpp b/src/burn/engine/exeengine.cpp index 6d326a5a7..c0bab2545 100644 --- a/src/burn/engine/exeengine.cpp +++ b/src/burn/engine/exeengine.cpp @@ -81,6 +81,14 @@ extern "C" HRESULT ExeEngineParsePackageFromXml( hr = XmlGetYesNoAttribute(pixnExePackage, L"ArpWin64", &pPackage->Exe.fArpWin64); ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @ArpWin64."); + // @ArpUseUninstallString + hr = XmlGetYesNoAttribute(pixnExePackage, L"ArpUseUninstallString", &pPackage->Exe.fArpUseUninstallString); + ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @ArpWin64."); + + // @UninstallArguments + hr = XmlGetAttributeEx(pixnExePackage, L"UninstallArguments", &pPackage->Exe.sczUninstallArguments); + ExitOnOptionalXmlQueryFailure(hr, fFoundXml, "Failed to get @UninstallArguments."); + pPackage->Exe.fUninstallable = TRUE; } @@ -481,7 +489,7 @@ extern "C" HRESULT ExeEngineExecutePackage( } else if (BURN_EXE_DETECTION_TYPE_ARP == pPackage->Exe.detectionType && BOOTSTRAPPER_ACTION_STATE_UNINSTALL == pExecuteAction->exePackage.action) { - ExitOnNull(sczArpUninstallString, hr, E_INVALIDARG, "QuietUninstallString is null."); + ExitOnNull(sczArpUninstallString, hr, E_INVALIDARG, "%hs is null.", pPackage->Exe.fArpUseUninstallString ? "UninstallString" : "QuietUninstallString"); hr = AppParseCommandLine(sczArpUninstallString, &argcArp, &argvArp); ExitOnFailure(hr, "Failed to parse QuietUninstallString: %ls.", sczArpUninstallString); @@ -1122,8 +1130,20 @@ static HRESULT DetectArpEntry( if (psczQuietUninstallString) { - hr = RegReadString(hKey, L"QuietUninstallString", psczQuietUninstallString); - ExitOnPathFailure(hr, fExists, "Failed to read QuietUninstallString."); + LPCWSTR sczUninstallStringName = pPackage->Exe.fArpUseUninstallString ? L"UninstallString" : L"QuietUninstallString"; + + hr = RegReadString(hKey, sczUninstallStringName, psczQuietUninstallString); + ExitOnPathFailure(hr, fExists, "Failed to read %ls.", sczUninstallStringName); + + // If the uninstall string is an executable path then ensure it is enclosed in quotes + if (fExists && *psczQuietUninstallString && (L'\"' != **psczQuietUninstallString) && FileExistsEx(*psczQuietUninstallString, nullptr)) + { + hr = StrAllocPrefix(psczQuietUninstallString, L"\"", 0); + ExitOnFailure(hr, "Failed to prepend UninstallString with quote."); + + hr = StrAllocConcat(psczQuietUninstallString, L"\"", 0); + ExitOnFailure(hr, "Failed to append quote to UninstallString."); + } } LExit: diff --git a/src/burn/engine/package.h b/src/burn/engine/package.h index bdebd5b65..1ea169e69 100644 --- a/src/burn/engine/package.h +++ b/src/burn/engine/package.h @@ -362,6 +362,7 @@ typedef struct _BURN_PACKAGE BURN_EXE_DETECTION_TYPE detectionType; BOOL fArpWin64; + BOOL fArpUseUninstallString; LPWSTR sczArpKeyPath; VERUTIL_VERSION* pArpDisplayVersion; diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs index ec418bc1f..0a11ea3aa 100644 --- a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs +++ b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs @@ -436,6 +436,16 @@ public void Execute() { writer.WriteAttributeString("ArpWin64", "yes"); } + + if (exePackage.ArpUseUninstallString) + { + writer.WriteAttributeString("ArpUseUninstallString", "yes"); + } + + if (!String.IsNullOrEmpty(exePackage.UninstallCommand)) + { + writer.WriteAttributeString("UninstallArguments", exePackage.UninstallCommand); + } break; case WixBundleExePackageDetectionType.None: writer.WriteAttributeString("DetectionType", "none"); diff --git a/src/wix/WixToolset.Core/Compiler_Bundle.cs b/src/wix/WixToolset.Core/Compiler_Bundle.cs index 89826cf62..a0a6d850d 100644 --- a/src/wix/WixToolset.Core/Compiler_Bundle.cs +++ b/src/wix/WixToolset.Core/Compiler_Bundle.cs @@ -1997,10 +1997,11 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType var bundle = YesNoType.NotSet; var slipstream = YesNoType.NotSet; var hasPayloadInfo = false; - WixBundleExePackageDetectionType? exeDetectionType = WixBundleExePackageDetectionType.None; + WixBundleExePackageDetectionType exeDetectionType = WixBundleExePackageDetectionType.None; string arpId = null; string arpDisplayVersion = null; var arpWin64 = YesNoType.NotSet; + var arpUseUninstallString = YesNoType.NotSet; var expectedNetFx4Args = new string[] { "/q", "/norestart" }; @@ -2117,6 +2118,7 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType case "DetectCondition": detectCondition = this.Core.GetAttributeValue(sourceLineNumbers, attrib, EmptyRule.CanBeEmpty); allowed = (packageType == WixBundlePackageType.Exe || packageType == WixBundlePackageType.Msu); + exeDetectionType = WixBundleExePackageDetectionType.Condition; break; case "Protocol": protocol = this.Core.GetAttributeValue(sourceLineNumbers, attrib); @@ -2183,11 +2185,6 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType this.Core.ParseExtensionAttribute(node, attribute, contextValues); } - if (packageType == WixBundlePackageType.Exe && (detectCondition != null || uninstallArguments != null)) - { - exeDetectionType = WixBundleExePackageDetectionType.Condition; - } - foreach (var child in node.Elements()) { if (CompilerCore.WixNamespace == child.Name.Namespace) @@ -2199,25 +2196,17 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType allowed = packageType == WixBundlePackageType.Exe; if (allowed) { - if (exeDetectionType == WixBundleExePackageDetectionType.Arp) + if (exeDetectionType != WixBundleExePackageDetectionType.None) { - this.Core.Write(ErrorMessages.TooManyChildren(Preprocessor.GetSourceLineNumbers(child), node.Name.LocalName, child.Name.LocalName)); + this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, child.Name.LocalName, "DetectCondition")); } - else if (!exeDetectionType.HasValue || exeDetectionType.Value == WixBundleExePackageDetectionType.Condition) + if (null != uninstallArguments) { - exeDetectionType = null; - } - else - { - if (exeDetectionType.Value != WixBundleExePackageDetectionType.None) - { - throw new WixException($"Unexpected WixBundleExePackageDetectionType: {exeDetectionType}"); - } - - exeDetectionType = WixBundleExePackageDetectionType.Arp; + this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, child.Name.LocalName, "UninstallArguments")); } - this.ParseExePackageArpEntryElement(child, out arpId, out arpDisplayVersion, out arpWin64); + exeDetectionType = WixBundleExePackageDetectionType.Arp; + this.ParseExePackageArpEntryElement(child, out arpId, out arpDisplayVersion, out arpWin64, out uninstallArguments, out arpUseUninstallString); } break; case "SlipstreamMsp": @@ -2282,6 +2271,11 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType } } + if (packageType == WixBundlePackageType.Exe && exeDetectionType == WixBundleExePackageDetectionType.None && uninstallArguments != null) + { + exeDetectionType = WixBundleExePackageDetectionType.Condition; + } + if (id.Id == BurnConstants.BundleDefaultBoundaryId) { this.Messaging.Write(CompilerErrors.ReservedValue(sourceLineNumbers, node.Name.LocalName, "Id", id.Id)); @@ -2374,10 +2368,6 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType this.Core.Write(WarningMessages.ExePackageDetectInformationRecommended(sourceLineNumbers)); } } - else - { - this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, "ArpEntry", String.IsNullOrEmpty(detectCondition) ? "UninstallArguments" : "DetectCondition")); - } if (repairArguments == null && repairCondition != null) { @@ -2497,6 +2487,7 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType WixBundleExePackageAttributes exeAttributes = 0; exeAttributes |= (YesNoType.Yes == bundle) ? WixBundleExePackageAttributes.Bundle : 0; exeAttributes |= (YesNoType.Yes == arpWin64) ? WixBundleExePackageAttributes.ArpWin64 : 0; + exeAttributes |= (YesNoType.Yes == arpUseUninstallString) ? WixBundleExePackageAttributes.ArpUseUninstallString : 0; this.Core.AddSymbol(new WixBundleExePackageSymbol(sourceLineNumbers, id) { @@ -2971,12 +2962,14 @@ private void ParseRemoteRelatedBundleElement(XElement node, string payloadId) } } - private void ParseExePackageArpEntryElement(XElement node, out string id, out string version, out YesNoType win64) + private void ParseExePackageArpEntryElement(XElement node, out string id, out string version, out YesNoType win64, out string uninstallArguments, out YesNoType arpUseUninstallString) { var sourceLineNumbers = Preprocessor.GetSourceLineNumbers(node); id = null; version = null; win64 = YesNoType.NotSet; + arpUseUninstallString = YesNoType.NotSet; + uninstallArguments = null; foreach (var attrib in node.Attributes()) { @@ -2990,6 +2983,12 @@ private void ParseExePackageArpEntryElement(XElement node, out string id, out st case "Version": version = this.Core.GetAttributeValue(sourceLineNumbers, attrib); break; + case "AdditionalUninstallArguments": + uninstallArguments = this.Core.GetAttributeValue(sourceLineNumbers, attrib); + break; + case "UseUninstallString": + arpUseUninstallString = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib); + break; case "Win64": win64 = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib); break; diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/ExePackageFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/ExePackageFixture.cs index 7403f96dd..b8f1af4d3 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/ExePackageFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/ExePackageFixture.cs @@ -287,26 +287,25 @@ public void ErrorWhenArpEntryWithDetectCondition() } [Fact] - public void ErrorWhenArpEntryWithUninstallArguments() + public void NoErrorWhenArpEntryWithUninstallArguments() { var folder = TestData.Get(@"TestData", "ExePackage"); using (var fs = new DisposableFileSystem()) { var baseFolder = fs.GetFolder(); + var wixlibPath = Path.Combine(baseFolder, "test.wixlib"); var result = WixRunner.Execute(new[] { "build", Path.Combine(folder, "ArpEntryWithUninstallArguments.wxs"), - "-o", Path.Combine(baseFolder, "test.wixlib") + "-o", wixlibPath }); - WixAssert.CompareLineByLine(new[] - { - "The ExePackage element cannot have a child element 'ArpEntry' when attribute 'UninstallArguments' is set.", - }, result.Messages.Select(m => m.ToString()).ToArray()); - Assert.Equal(372, result.ExitCode); + result.AssertSuccess(); + + Assert.True(File.Exists(wixlibPath)); } }