winget-cli

commit 7016c56b5df7da8e42fe08d0ee5d0d72f7b9fb13
parent 92daa5fe3b52c5886e590023b2472c00ea7099ef
Author: Kaleb Luedtke <trenlymc@gmail.com>
Date:   Tue, 25 Mar 2025 15:19:24 -0500

Apply Registry Entries Last During Update Flow (#5214)

Resolves #5190

This PR makes it so that the registry entries for a portable application
are only applied if the DesiredState was applied successfully
Diffstat:
M
src/AppInstallerCLICore/PortableInstaller.cpp
 | 
18
++++++++++++++++--
M
src/AppInstallerCLICore/PortableInstaller.h
 | 
3
++-
M
src/AppInstallerCLICore/Workflows/PortableFlow.cpp
 | 
4
+++-
M
src/AppInstallerCLITests/PortableInstaller.cpp
 | 
9
+++++----

4 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp
@@ -10,6 +10,7 @@
 #include <winget/PortableIndex.h>
 #include <AppInstallerErrors.h>
 #include <AppInstallerRuntime.h>
+#include <Workflows/WorkflowBase.h>
 
 using namespace AppInstaller::Utility;
 using namespace AppInstaller::Registry;
@@ -19,6 +20,7 @@ using namespace AppInstaller::Repository;
 using namespace AppInstaller::SQLite;
 using namespace AppInstaller::Repository::Microsoft;
 using namespace AppInstaller::Repository::Microsoft::Schema;
+using namespace AppInstaller::CLI::Workflow;
 
 namespace AppInstaller::CLI::Portable
 {
@@ -279,9 +281,14 @@ namespace AppInstaller::CLI::Portable
         return true;
     }
 
-    void PortableInstaller::Install()
+    void PortableInstaller::Install( Workflow::OperationType operation = Workflow::OperationType::Install)
     {
-        RegisterARPEntry();
+        // If the operation is an install, the ARP entry should be created first so that a catastrophic failure
+        // leaves the system in a state where an uninstall may be possible
+        if (operation == Workflow::OperationType::Install)
+        {
+            RegisterARPEntry();
+        }
 
         CreateTargetInstallDirectory();
 
@@ -291,6 +298,13 @@ namespace AppInstaller::CLI::Portable
         {
             AddToPathVariable(GetPortableLinksLocation(GetScope()));
         }
+
+        // If the operation is an upgrade, the ARP entry should be created last so that a catastrophic failure
+        // leaves the system in a state where an upgrade can be re-attempted
+        if (operation == Workflow::OperationType::Upgrade)
+        {
+            RegisterARPEntry();
+        }
     }
 
     void PortableInstaller::Uninstall()
diff --git a/src/AppInstallerCLICore/PortableInstaller.h b/src/AppInstallerCLICore/PortableInstaller.h
@@ -3,6 +3,7 @@
 #pragma once
 #include "winget/PortableARPEntry.h"
 #include "winget/PortableFileEntry.h"
+#include <Workflows/WorkflowBase.h>
 #include <filesystem>
 
 using namespace AppInstaller::Registry::Portable;
@@ -58,7 +59,7 @@ namespace AppInstaller::CLI::Portable
             m_desiredEntries = {};
         }
 
-        void Install();
+        void Install( AppInstaller::CLI::Workflow::OperationType operation );
 
         void Uninstall();
 
diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp
@@ -244,6 +244,8 @@ namespace AppInstaller::CLI::Workflow
 
     void PortableInstallImpl(Execution::Context& context)
     {
+        OperationType installType = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate) ? OperationType::Upgrade : OperationType::Install;
+        
         PortableInstaller& portableInstaller = context.Get<Execution::Data::PortableInstaller>();
         try
         {
@@ -266,7 +268,7 @@ namespace AppInstaller::CLI::Workflow
                 }
             }
 
-            portableInstaller.Install();
+            portableInstaller.Install(installType);
             context.Add<Execution::Data::OperationReturnCode>(ERROR_SUCCESS);
             context.Reporter.Warn() << portableInstaller.GetOutputMessage();
         }
diff --git a/src/AppInstallerCLITests/PortableInstaller.cpp b/src/AppInstallerCLITests/PortableInstaller.cpp
@@ -21,6 +21,7 @@ using namespace AppInstaller::Repository::Microsoft;
 using namespace AppInstaller::SQLite;
 using namespace AppInstaller::Repository::Microsoft::Schema;
 using namespace AppInstaller::Utility;
+using namespace AppInstaller::CLI::Workflow;
 using namespace TestCommon;
 
 TEST_CASE("PortableInstaller_InstallToRegistry", "[PortableInstaller]")
@@ -44,7 +45,7 @@ TEST_CASE("PortableInstaller_InstallToRegistry", "[PortableInstaller]")
     portableInstaller.SetDesiredState(desiredTestState);
     REQUIRE(portableInstaller.VerifyExpectedState());
 
-    portableInstaller.Install();
+    portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install);
 
     PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode");
     REQUIRE(portableInstaller2.ARPEntryExists());
@@ -92,7 +93,7 @@ TEST_CASE("PortableInstaller_InstallToIndex_CreateInstallRoot", "[PortableInstal
     portableInstaller.SetDesiredState(desiredTestState);
     REQUIRE(portableInstaller.VerifyExpectedState());
 
-    portableInstaller.Install();
+    portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install);
 
     REQUIRE(std::filesystem::exists(installRootPath / portableInstaller.GetPortableIndexFileName()));
     REQUIRE(std::filesystem::exists(targetPath));
@@ -150,7 +151,7 @@ TEST_CASE("PortableInstaller_InstallToIndex_ExistingInstallRoot", "[PortableInst
     portableInstaller.SetDesiredState(desiredTestState);
     REQUIRE(portableInstaller.VerifyExpectedState());
 
-    portableInstaller.Install();
+    portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install);
 
     REQUIRE(std::filesystem::exists(installRootPath / portableInstaller.GetPortableIndexFileName()));
     REQUIRE(std::filesystem::exists(targetPath));
@@ -197,7 +198,7 @@ TEST_CASE("PortableInstaller_UnicodeSymlinkPath", "[PortableInstaller]")
     portableInstaller.SetDesiredState(desiredTestState);
     REQUIRE(portableInstaller.VerifyExpectedState());
 
-    portableInstaller.Install();
+    portableInstaller.Install(AppInstaller::CLI::Workflow::OperationType::Install);
 
     PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode");
     REQUIRE(portableInstaller2.ARPEntryExists());