See wixtoolset/issues#7877: ArpEntry reads QuietUninstallString or Un…

…installString, and uses UninstallArguments for the uninstall command line
nirbar · Nov 20, 2023 · 72f97c8 · 72f97c8
1 parent ee3747f
commit 72f97c8
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 34 deletions.
diff --git a/README.md b/README.md
@@ -17,3 +17,4 @@ This repository contains the PanelSwWix4: A custom WiX Toolset codebase
 - [6298](https://github.com/wixtoolset/issues/issues/6298): Extract detached containers with "wix burn extract"
 - [6252](https://github.com/wixtoolset/issues/issues/6252): Automatically add -norestart flag for any burn ExePackage, BundlePackage, and related bundles
 - [7552](https://github.com/wixtoolset/issues/issues/7552): Add burn command line argument to log to console: /clog or /conlog
+- [7877](https://github.com/wixtoolset/issues/issues/7877): ArpEntry reads QuietUninstallString or UninstallString, and uses UninstallArguments for the uninstall command line 
diff --git a/src/burn/engine/exeengine.cpp b/src/burn/engine/exeengine.cpp
@@ -81,6 +81,10 @@ extern "C" HRESULT ExeEngineParsePackageFromXml(
         hr = XmlGetYesNoAttribute(pixnExePackage, L"ArpWin64", &pPackage->Exe.fArpWin64);
         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 +485,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, "QuietUninstallString and UninstallString are null.");
 
         hr = AppParseCommandLine(sczArpUninstallString, &argcArp, &argvArp);
         ExitOnFailure(hr, "Failed to parse QuietUninstallString: %ls.", sczArpUninstallString);
@@ -1129,7 +1133,19 @@ static HRESULT DetectArpEntry(
     if (psczQuietUninstallString)
     {
         hr = RegReadString(hKey, L"QuietUninstallString", psczQuietUninstallString);
-        ExitOnPathFailure(hr, fExists, "Failed to read QuietUninstallString.");
+        if ((E_PATHNOTFOUND == hr || E_FILENOTFOUND == hr))
+        {
+            hr = RegReadString(hKey, L"UninstallString", psczQuietUninstallString);
+            if (SUCCEEDED(hr) && *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.");
+            }
+        }
+        ExitOnPathFailure(hr, fExists, "Failed to read QuietUninstallString and UninstallString.");
     }
 
 LExit:

diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
@@ -451,6 +451,11 @@ public void Execute()
                                 {
                                     writer.WriteAttributeString("ArpWin64", "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
@@ -2321,11 +2321,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)
@@ -2337,24 +2332,12 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType
                             allowed = packageType == WixBundlePackageType.Exe;
                             if (allowed)
                             {
-                                if (exeDetectionType == WixBundleExePackageDetectionType.Arp)
+                                if (exeDetectionType.Value != WixBundleExePackageDetectionType.None)
                                 {
-                                    this.Core.Write(ErrorMessages.TooManyChildren(Preprocessor.GetSourceLineNumbers(child), node.Name.LocalName, child.Name.LocalName));
-                                }
-                                else if (!exeDetectionType.HasValue || exeDetectionType.Value == WixBundleExePackageDetectionType.Condition)
-                                {
-                                    exeDetectionType = null;
-                                }
-                                else
-                                {
-                                    if (exeDetectionType.Value != WixBundleExePackageDetectionType.None)
-                                    {
-                                        throw new WixException($"Unexpected WixBundleExePackageDetectionType: {exeDetectionType}");
-                                    }
-
-                                    exeDetectionType = WixBundleExePackageDetectionType.Arp;
+                                    throw new WixException($"Unexpected WixBundleExePackageDetectionType: {exeDetectionType}");
                                 }
 
+                                exeDetectionType = WixBundleExePackageDetectionType.Arp;
                                 this.ParseExePackageArpEntryElement(child, out arpId, out arpDisplayVersion, out arpWin64);
                             }
                             break;
@@ -2420,6 +2403,11 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType
                 }
             }
 
+            if (packageType == WixBundlePackageType.Exe && exeDetectionType.Value == WixBundleExePackageDetectionType.None && (detectCondition != null || uninstallArguments != null))
+            {
+                exeDetectionType = WixBundleExePackageDetectionType.Condition;
+            }
+
             if (id.Id == BurnConstants.BundleDefaultBoundaryId)
             {
                 this.Messaging.Write(CompilerErrors.ReservedValue(sourceLineNumbers, node.Name.LocalName, "Id", id.Id));
@@ -2452,7 +2440,10 @@ private string ParseChainPackage(XElement node, WixBundlePackageType packageType
 
                 if (exeDetectionType == WixBundleExePackageDetectionType.Arp)
                 {
-                    // Missing attributes are reported when parsing the element.
+                    if (!String.IsNullOrEmpty(detectCondition))
+                    {
+                        this.Core.Write(ErrorMessages.UnexpectedElementWithAttribute(sourceLineNumbers, node.Name.LocalName, "ArpEntry", "DetectCondition"));
+                    }
                 }
                 else if (exeDetectionType == WixBundleExePackageDetectionType.Condition)
                 {
@@ -2512,10 +2503,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)
                 {

diff --git 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));
             }
         }