Fixes for updating winget from winget (microsoft#5972)

Fixes microsoft#4283

## Change
The primary issue here is that the package update monitoring is too
aggressive at shutting down, and I made it listen to signal in more
cases. It really isn't a good signal for us to listen to, and was only
added for the elevated Store case. The fix is to move it entirely to the
Store flow, as if we are installing via MSIX we can just let the
deferred registration work and if someone else is updating us then we
can just listen to the proper termination signals.

I additionally noticed that the dependencies were being installed
despite the fact that I definitely already had them up to date. There
were a few issues here that I also fixed:

- An MSIX nested inside an archive that was expecting to inherit the
PackageFamilyName from the root node of the manifest was not doing so if
the installer type was also inherited. Inheriting the installer type
info before the PFN resolved this (for `Microsoft.VCLibs.14`).
- An exe installer that installs an MSIX that was expecting to inherit
the PackageFamilyName from the root node of the manifest was not doing
so. I allowed the PFN to be inherited when the `AppsAndFeaturesEntries`
has an MSIX type (for `Microsoft.WindowsAppRuntime.*`).
- MSIX packages were not being allowed to map display versions, so I
enabled them.
- There was a bug with display version sorting that would not map the
sort key of the "first" version when multiple versions are installed.
This could lead to the first version in the list not being the latest
version per the mapping.

Author: JohnMcPMS

src/AppInstallerCLICore/Commands/TestCommand.cpp

@@ -31,7 +31,7 @@ namespace AppInstaller::CLI
         HRESULT WaitForShutdown(Execution::Context& context)
         {
             LogAndReport(context, "Waiting for app shutdown event");
-            if (!ShutdownMonitoring::TerminationSignalHandler::Instance()->WaitForAppShutdownEvent())
+            if (!ShutdownMonitoring::ServerShutdownSynchronization::WaitForShutdown(300000))
             {
                 LogAndReport(context, "Failed getting app shutdown event");
                 return APPINSTALLER_CLI_ERROR_INTERNAL_ERROR;
@@ -82,6 +82,21 @@ namespace AppInstaller::CLI
             return hr;
         }
 
+        void AppShutdownTestSystemBlockNewWork(CancelReason reason)
+        {
+            AICLI_LOG(CLI, Info, << "AppShutdownTestSystemBlockNewWork :: " << reason);
+        }
+
+        void AppShutdownTestSystemBeginShutdown(CancelReason reason)
+        {
+            AICLI_LOG(CLI, Info, << "AppShutdownTestSystemBeginShutdown :: " << reason);
+        }
+
+        void AppShutdownTestSystemWait()
+        {
+            AICLI_LOG(CLI, Info, << "AppShutdownTestSystemWait");
+        }
+
         void EnsureDSCv3Processor(Execution::Context& context)
         {
             auto& configurationSet = context.Get<Execution::Data::ConfigurationContext>().Set();
@@ -354,6 +369,13 @@ namespace AppInstaller::CLI
     {
         HRESULT hr = E_FAIL;
 
+        ShutdownMonitoring::ServerShutdownSynchronization::ComponentSystem appShutdownTestSystem{};
+        appShutdownTestSystem.BlockNewWork = AppShutdownTestSystemBlockNewWork;
+        appShutdownTestSystem.BeginShutdown = AppShutdownTestSystemBeginShutdown;
+        appShutdownTestSystem.Wait = AppShutdownTestSystemWait;
+
+        ShutdownMonitoring::ServerShutdownSynchronization::AddComponent(appShutdownTestSystem);
+
         // Only package context and admin won't create the window message.
         if (!Runtime::IsRunningInPackagedContext() || !Runtime::IsRunningAsAdmin())
         {

src/AppInstallerCLICore/ExecutionContext.h

@@ -80,12 +80,6 @@ namespace AppInstaller::CLI::Execution
 
     DEFINE_ENUM_FLAG_OPERATORS(ContextFlag);
 
-#ifndef AICLI_DISABLE_TEST_HOOKS
-    HWND GetWindowHandle();
-
-    bool WaitForAppShutdownEvent();
-#endif
-
     // Callback to log data actions.
     void ContextEnumBasedVariantMapActionCallback(const void* map, Data data, EnumBasedVariantMapAction action);
 

src/AppInstallerCLICore/Public/ShutdownMonitoring.h

@@ -3,10 +3,10 @@
 #pragma once
 #include <Windows.h>
 #include <AppInstallerProgress.h>
-#include <winrt/Windows.ApplicationModel.h>
 #include <wil/resource.h>
 #include <memory>
 #include <mutex>
+#include <optional>
 
 namespace AppInstaller::ShutdownMonitoring
 {
@@ -39,9 +39,6 @@ namespace AppInstaller::ShutdownMonitoring
 #ifndef AICLI_DISABLE_TEST_HOOKS
         // Gets the window handle for the message window.
         HWND GetWindowHandle() const;
-
-        // Waits for the shutdown event.
-        bool WaitForAppShutdownEvent() const;
 #endif
 
     private:
@@ -59,17 +56,11 @@ namespace AppInstaller::ShutdownMonitoring
 
         void CreateWindowAndStartMessageLoop();
 
-#ifndef AICLI_DISABLE_TEST_HOOKS
-        wil::unique_event m_appShutdownEvent;
-#endif
-
         std::mutex m_listenersLock;
         std::vector<ICancellable*> m_listeners;
         wil::unique_event m_messageQueueReady;
         wil::unique_hwnd m_windowHandle;
         std::thread m_windowThread;
-        winrt::Windows::ApplicationModel::PackageCatalog m_catalog = nullptr;
-        decltype(winrt::Windows::ApplicationModel::PackageCatalog{ nullptr }.PackageUpdating(winrt::auto_revoke, nullptr)) m_updatingEvent;
     };
 
     // Coordinates shutdown across server components
@@ -98,7 +89,7 @@ namespace AppInstaller::ShutdownMonitoring
         static void AddComponent(const ComponentSystem& component);
 
         // Waits for the shutdown to complete.
-        static void WaitForShutdown();
+        static bool WaitForShutdown(std::optional<DWORD> timeout = std::nullopt);
 
     private:
         ServerShutdownSynchronization() = default;

src/AppInstallerCLICore/ShutdownMonitoring.cpp

@@ -75,36 +75,16 @@ namespace AppInstaller::ShutdownMonitoring
     {
         return m_windowHandle.get();
     }
-
-    bool TerminationSignalHandler::WaitForAppShutdownEvent() const
-    {
-        return m_appShutdownEvent.wait(60000);
-    }
 #endif
 
     TerminationSignalHandler::TerminationSignalHandler()
     {
-#ifndef AICLI_DISABLE_TEST_HOOKS
-        m_appShutdownEvent.create();
-#endif
-
         if (!s_TerminationSignalHandlerEnabled)
         {
             AICLI_LOG(CLI, Info, << "TerminationSignalHandler is disabled, skipping creation of signal listeners");
             return;
         }
 
-        if (Runtime::IsRunningInPackagedContext())
-        {
-            // Create package update listener
-            m_catalog = winrt::Windows::ApplicationModel::PackageCatalog::OpenForCurrentPackage();
-            m_updatingEvent = m_catalog.PackageUpdating(
-                winrt::auto_revoke, [this](winrt::Windows::ApplicationModel::PackageCatalog, winrt::Windows::ApplicationModel::PackageUpdatingEventArgs)
-                {
-                    this->StartAppShutdown();
-                });
-        }
-
         // Create message only window.
         m_messageQueueReady.create();
         m_windowThread = std::thread(&TerminationSignalHandler::CreateWindowAndStartMessageLoop, this);
@@ -138,10 +118,6 @@ namespace AppInstaller::ShutdownMonitoring
     {
         AICLI_LOG(CLI, Info, << "Initiating shutdown procedure");
 
-#ifndef AICLI_DISABLE_TEST_HOOKS
-        m_appShutdownEvent.SetEvent();
-#endif
-
         // Lifetime manager sends CTRL-C after the WM_QUERYENDSESSION is processed.
         // If we disable the CTRL-C handler, the default handler will kill us.
         InformListeners(CancelReason::AppShutdown, true);
@@ -334,20 +310,27 @@ namespace AppInstaller::ShutdownMonitoring
         instance.m_components.push_back(component);
     }
 
-    void ServerShutdownSynchronization::WaitForShutdown()
+    bool ServerShutdownSynchronization::WaitForShutdown(std::optional<DWORD> timeout)
     {
         ServerShutdownSynchronization& instance = Instance();
 
+        if (timeout)
+        {
+            return instance.m_shutdownComplete.wait(timeout.value());
+        }
+        else
         {
-            std::lock_guard<std::mutex> lock{ instance.m_threadLock };
-            if (!instance.m_shutdownThread.joinable())
             {
-                AICLI_LOG(Core, Warning, << "Attempt to wait for shutdown when shutdown has not been initiated.");
-                return;
+                std::lock_guard<std::mutex> lock{ instance.m_threadLock };
+                if (!instance.m_shutdownThread.joinable())
+                {
+                    AICLI_LOG(Core, Warning, << "Attempt to wait for shutdown when shutdown has not been initiated.");
+                    return false;
+                }
             }
-        }
 
-        instance.m_shutdownComplete.wait();
+            return instance.m_shutdownComplete.wait();
+        }
     }
 
     void ServerShutdownSynchronization::Signal(CancelReason reason)
@@ -393,6 +376,7 @@ namespace AppInstaller::ShutdownMonitoring
             components = m_components;
         }
 
+        AICLI_LOG(CLI, Verbose, << "ServerShutdownSynchronization :: BlockNewWork");
         for (const auto& component : components)
         {
             if (component.BlockNewWork)
@@ -401,6 +385,7 @@ namespace AppInstaller::ShutdownMonitoring
             }
         }
 
+        AICLI_LOG(CLI, Verbose, << "ServerShutdownSynchronization :: BeginShutdown");
         for (const auto& component : components)
         {
             if (component.BeginShutdown)
@@ -409,6 +394,7 @@ namespace AppInstaller::ShutdownMonitoring
             }
         }
 
+        AICLI_LOG(CLI, Verbose, << "ServerShutdownSynchronization :: Wait");
         for (const auto& component : components)
         {
             if (component.Wait)
@@ -417,6 +403,7 @@ namespace AppInstaller::ShutdownMonitoring
             }
         }
 
+        AICLI_LOG(CLI, Verbose, << "ServerShutdownSynchronization :: ShutdownCompleteCallback");
         ShutdownCompleteCallback callback = m_callback;
         if (callback)
         {

src/AppInstallerCLIE2ETests/AppShutdownTests.cs

@@ -24,6 +24,7 @@ public class AppShutdownTests
         /// Runs winget test appshutdown and register the application to force a WM_QUERYENDSESSION message.
         /// </summary>
         [Test]
+        [Ignore("This test relied on a signal to terminate that was determined to be problematic. We may need OS fixes to test it when elevated.")]
         public void RegisterApplicationTest()
         {
             if (!TestSetup.Parameters.PackagedContext)
@@ -95,9 +96,6 @@ public void RegisterApplicationTest()
 
             Task.WaitAll(new Task[] { testCmdTask, registerTask }, 360000);
 
-            // Assert.True(registerTask.Result);
-            TestContext.Out.Write(testCmdTask.Result.StdOut);
-
             // The ctrl-c command terminates the batch file before the exit code file gets created.
             // Look for the output.
             Assert.True(testCmdTask.Result.StdOut.Contains("Succeeded waiting for app shutdown event"));

src/AppInstallerCLIE2ETests/InprocTestbedTests.cs

@@ -90,7 +90,7 @@ public void OneTimeSetup()
         [Test]
         public void DefaultTest()
         {
-            this.RunInprocTestbed(new TestbedParameters());
+            this.RunInprocTestbed(new TestbedParameters() { WorkTestSleepInterval = 1000 });
         }
 
         /// <summary>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -1049,6 +1049,12 @@
     <CopyFileToFolders Include="TestData\TestFont.ttf">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-MSIX-in-AppsAndFeatures.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-MSIX-in-Archive.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj">
@@ -1080,5 +1086,4 @@
     <Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.250303.1\build\native\Microsoft.Windows.CppWinRT.props')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.250303.1\build\native\Microsoft.Windows.CppWinRT.props'))" />
     <Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.250303.1\build\native\Microsoft.Windows.CppWinRT.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.250303.1\build\native\Microsoft.Windows.CppWinRT.targets'))" />
   </Target>
-</Project>
-
+</Project>