DeploymentManager spec update for the new Repair API #2869

sachintaMSFT · 2022-08-18T00:31:06Z

Deployment Manager Spec update to add the new Repair API for 1.3 release. Hence, incrementing the DeploymentContract to version 4.

specs/Deployment/DeploymentAPI.md

…entRepairAPISpec

DrusTheAxe

DON'T CHECKIN until API Review

specs/Deployment/DeploymentAPI.md

…b.com/microsoft/WindowsAppSDK into user/sachinta/DeploymentRepairAPISpec

…ntract version.

specs/Deployment/DeploymentAPI.md

MikeHillberg · 2022-10-04T19:29:26Z

specs/Deployment/DeploymentAPI.md

+architecture needed are derived from the current WinAppSDK Framework package.
+
+Since both the Main _and_ the Singleton packages will be repaired, it is possible that Main package
+repair has failed and Initialize returns with PackageRepairFailed status (OR) it is possible that


What does "(OR)" mean here? "or"?

It is just "or". I should probably just use "or" to avoid such confusion.

MikeHillberg · 2022-10-04T19:30:43Z

specs/Deployment/DeploymentAPI.md

+repair has failed and Initialize returns with PackageRepairFailed status (OR) it is possible that
+Main package is repaired successfully but the Singleton package repair has failed and Initialize
+returns with repair has failed and Initialize returns with PackageRepairFailed status (OR) it is
+possible that status. In both cases, the returned WindowsAppRuntimeStatus will contain the error of


both cases

I think it just listed 3 cases

It's actually just 2 cases only.

I see three cases in the text (separated by "OR"):

it is possible that Main package repair has failed and Initialize returns with PackageRepairFailed status

it is possible that Main package is repaired successfully but the Singleton package repair has failed and Initialize returns with repair has failed and Initialize returns with PackageRepairFailed status

it is possible that status

Not sure what 3 is trying to say.

MikeHillberg · 2022-10-04T19:31:47Z

specs/Deployment/DeploymentAPI.md

+repair has failed and Initialize returns with PackageRepairFailed status (OR) it is possible that
+Main package is repaired successfully but the Singleton package repair has failed and Initialize
+returns with repair has failed and Initialize returns with PackageRepairFailed status (OR) it is
+possible that status. In both cases, the returned WindowsAppRuntimeStatus will contain the error of


possible that status

Looks like there was more sentence intended here

Yes, I need to rectify that. Not sure how I ended up with that incomplete sentence there. But I wanted to mean "Main's repair could fail" or "Main repair succeeded but Singleton repair could fail".

specs/Deployment/DeploymentAPI.md

MikeHillberg · 2022-10-04T19:40:58Z

specs/Deployment/DeploymentAPI.md

@@ -143,18 +161,16 @@ by using DeploymentInitializeOptions object and setting ForceDeployment option b
 this API. This option when set will shut down the application(s) associated with WinAppSDK packages,
 update the WinAppSDK packages and restart the application(s).


If I don't set ForceDeployment true, the deploy will fail? And then the app should interact with the to say "on next reboot" or some such?

If the packaged app doesn't call Initialize() or Repair() with ForceDeployment set and if the current install is actually an update because an older package version is already installed, then yes it does fail with an error that reads "Package in Use". There is no further story to it. If the intention of the developer is to use ForceDeployment eventually, then there is no reason to wait for the first call of Initialize to fail before calling it again with ForceDeployment set this time. The developer could as well have passed it in the first call of Initialize or Repair itself.

Explained what happens when ForceDeployment is set in your next feedback comment in detail.

MikeHillberg · 2022-10-04T19:42:03Z

specs/Deployment/DeploymentAPI.md

-framework package and all dependent packages that are currently in use will be re-installed, after
-they shut down, to refer to the updated framework package.
+Framework package and all dependent packages that are currently in use will be re-installed, after
+they shut down, to refer to the updated Framework package.


after they shut down

Does this mean it will force the apps currently using the package to shut down, or does this mean that the Initialize() call could block indefinitely?

It will only force the Main, Singleton and DDLM packages to shut down in order to update Main, Singleton and DDLM packages (ex: PushNotifications would be shutdown as it's part of Singleton package) and the shutdown packages are restarted automatically by the OS platform after their updates are complete (and in cases such as PushNotifictions which would need explicit reboot as it's a com server, we do reboot it after the update in Initialize()/Repair()).

It doesn't shutdown the WinAppSDK framework package dependent packaged apps and installs the higher version of the framework package side by side without uninstalling the older version. It does update the WinAppSDK framework package dependent packaged apps (in their package graph) to point to higher version of the framework package, once they are shutdown by the user. Once all references to older version of the framework package are updated to refer to the higher version of it, the older version is then completely uninstalled by the OS platform.

What if an app is currently using Main/Singleton/DDLM package (and I set Force)? What happens to the app?

MikeHillberg · 2022-10-04T19:44:02Z

specs/Deployment/DeploymentAPI.md

+}
+```
+
+Here's the previous example extended with the `Show UI on error` option:


This example doesn't match the above logic; the logic shows UI, the example doesn't. If the UI built in to the API and explains to the user what the problem is? Or if the logic is correct and the app needs to show UI, it's awkward because they can't use WinAppSDK to show the UI

Yes, the UI shown is built into the API explaining the problem to the user (currently via MessageBox())

MikeHillberg · 2022-10-04T19:46:14Z

specs/Deployment/DeploymentAPI.md

-        /// WindowsAppSDK main and singleton packages will be shut down forcibly if they are
-        /// currently in use, when registering the WinAppSDK packages.
+        /// WinAppSDK Main and Singleton packages will be shut down forcibly if they are
+        /// currently in use, when initializing the WindowsAppRuntime.


So when you start my app, you might see some other apps disappear? Is there any kind of interaction with the user first?

No. Please see one of the above comments for detailed explanation on ForceDeployment. In short, ForceDeployment only shuts down WinAppSDK packages that are dependent on it's framework packages (ex: Main, Singleton and DDLM). It doesn't shut down any non-WinAppSDK packages that declared dependency on WinAppSDK framework package.

FrankC-msft · 2022-10-04T19:41:21Z

specs/Deployment/DeploymentAPI.md

+they shut down, to refer to the updated Framework package.
+
+```C#
+    if (DeploymentManager.GetStatus().Status == DeploymentStatus.PackageInstallRequired)


This is in the DeploymentRepairOptions section, so this should be checking for == DeploymentStatus.PackageRepairRequired right?

Yes. Need to check for == DeploymentStatus.PackageRepairRequired. Thanks for pointing that out.

FrankC-msft · 2022-10-04T19:49:01Z

specs/Deployment/DeploymentAPI.md

   the system?
+3. How do I repair the already installed WinAppSDK Main and Singleton packages, if needed ?


Why do we need Repair to be a distinct action than Install/Initialize, instead of having developers [re]install the packages whenever they're "broken". This is something of a breaking change since we'd previously told developers to Initialize for any status other than .OK, right?

The manual method to repair by "reinstalling from msdn downloads page" is identical for install and repair. Why is the full-trust-app method different?

Intialize installs/re-installs WinAppSDK Main and Singleton packages only when needed (i.e. when one of the packages is not installed or if one of them is in identified bad state). Thus, avoiding the unnecessary perf overhead of the unconditional re-install in every launch of the WinAppSDK dependent Packaged app.

Repair, on the other hand, re-installs those packages unconditionally. This API is important because there are many bad states a package can be in but only a few of them could be identified by Initialize and fix. Hence, we need Developers to just able to call Repair in cases where the WinAppSDK features aren't working due to a bad state that Initialize couldn't identify and fix it.

This is not a breaking change. Because the current usage of Initialize as detailed in the code sample in this spec prior to the current PR would still work for Initialize API (i.e. up until 1.2 WinAppSDK release). But the changes to code samples in this PR show a better recommended method in calling Initialize and Repair API conditionally, that developers can adopt with 1.3+ WinAppSDK release.

Manual method of using WindowsAppRuntimeInstall.exe can now, starting with 1.2 WinAppSDK release, use repair option explicitly. Once again, this is due to the desire to avoid unnecessary re-installs in default install path (ex: as complained by PowerToys team).

FrankC-msft · 2022-10-04T19:55:11Z

specs/Deployment/DeploymentAPI.md

+current WinAppSDK Framework package.
+
+Since both the Main _and_ the Singleton packages may need to be installed, it is possible that Main
+package install has failed and Initialize returns with PackageInstallFailed state (OR) it is


What happens if I try to Repair() a package that's in PackageInstallFailed state? That seems like a natural inclination ("I did Initialize() and it's in some error state now, I guess that means it's broken so I should fix it. Fix -> repair. Call Repair()"). Are we providing DeploymentResult properties that tell the developer they called the wrong method?

Repair() essentially provides an unconditional core functionality of Initialize() - which is to install WindowsAppSDK Main and Singleton licenses and packages. So, if Repair() is tried when Initialize() fails, then it is effectively a retry of Initialize(). If the underlying issue for PackageInstallFailed is transient, then Repair() may succeed. Otherwise, it will continue to be PackageInstallFailed.

No, DeploymentResult doesn't have any property that calls out wrong method usage. Because calling Repair() when Initialize() fails is not a wrong method.

sachintaMSFT · 2022-10-06T01:26:10Z

specs/Deployment/DeploymentAPI.md

+architecture needed are derived from the current WinAppSDK Framework package.
+
+Since both the Main _and_ the Singleton packages will be repaired, it is possible that Main package
+repair has failed and Initialize returns with PackageRepairFailed status (OR) it is possible that


In this paragraph here, "Initialize returns with PackageRepairFailed status" should actually be "Repair returns with PackageRepairFailed status" at 2 instances.

FrankC-msft · 2022-10-06T22:27:26Z

specs/Deployment/DeploymentAPI.md

+            // for the Status.
+        }
+        else if (result.Status == DeploymentStatus.PackageRepairRequired ||
+                result.Status == DeploymentStatus.Unknown)


what if the status is PackageInstallFailed ? Shouldn't I want to Repair() it in that case? (Since repair is unconditional core functionality of Initialize, is PackageInstallFailed a special-case or subset of PackageRepairRequired?)

oldnewthing · 2022-10-13T19:14:20Z

specs/Deployment/DeploymentAPI.md

-        // WindowsAppRuntime. result.ExtendedError has the error code that has the reason
-        // for the Status.
+        // WindowsAppRuntime is either not installed or not in a good state (can't be both at same time).
+        if (result.Status == DeploymentStatus.PackageInstallRequired)


What happens if an app developer upgrades to WinAppSDK 1.3 but forgets to update this part of their code? In other words, what happens if the app just calls Initialize for all non-OK deployment statuses instead of calling Repair sometimes?

I hope the answer is "Everything still works, though maybe less efficiently than if you had used Repair."

oldnewthing · 2022-10-13T19:15:56Z

specs/Deployment/DeploymentAPI.md

+            // for the Status.
+        }
+        else if (result.Status == DeploymentStatus.PackageRepairRequired ||
+                result.Status == DeploymentStatus.Unknown)


Suppose in WinAppSDK version 1.9 we add a new DeploymentStatus. This code will ignore all new DeploymentStatus error codes. Should it treat all unknown DeploymentStatus values the same as Unknown?

As written, it means that adding a new DeploymentStatus is a breaking change.

oldnewthing · 2022-10-13T19:18:51Z

specs/Deployment/DeploymentAPI.md

-again.
+Once the Main and Singleton packages have been deployed, they generally do not need to be deployed
+again. If a user were to remove the Main or Singleton package, the API can be used to reinstall them
+again. If there is anything wrong with installed Main and Singleton packages, use Repair API to try


What does "anything wrong" mean? Does it mean "Any DeploymentStatus other than DeploymentStatus.Ok"?

Because that's not what the sample code does. It calls Repair only for selected DeploymentStatus values, not for "anything wrong".

oldnewthing · 2022-10-13T19:20:12Z

specs/Deployment/DeploymentAPI.md


 ```C#
-    if (DeploymentManager.GetStatus().Status != DeploymentStatus.Ok)
+    if (DeploymentManager.GetStatus().Status == DeploymentStatus.PackageInstallRequired)


This does not do what the previous text recommends. Previous text says, "If there is anything wrong with installed Main and Singleton packages, use Repair API to try and repair them." But in this code, we don't call Repair for "anything wrong". In fact, we're not calling Repair at all!

oldnewthing · 2022-10-13T19:21:53Z

specs/Deployment/DeploymentAPI.md

@@ -167,12 +183,13 @@ they shut down, to refer to the updated framework package.
        // These should be run on a separate thread so as not to hang your app while the
        // packages deploy.
        var initializeTask = Task.Run(() => DeploymentManager.Initialize(deploymentInitializeOptions));
+        //...do other work while the repair is running...


The comment says "repair", but the method we called was "Initialize".

Either the code should be "Repair", to match the comment; or the comment should say "initialize" to match the code.

oldnewthing · 2022-10-13T19:39:26Z

specs/Deployment/DeploymentAPI.md

+    if (DeploymentManager.GetStatus().Status == DeploymentStatus.PackageRepairRequired ||
+        DeploymentManager.GetStatus().Status == DeploymentStatus.Unknown)
+    {
+        // Repair will always attempt to repair WindowsAppRuntime regardless of it's state.


Suggested change

// Repair will always attempt to repair WindowsAppRuntime regardless of it's state.

// Repair will always attempt to repair WindowsAppRuntime regardless of its state.

oldnewthing · 2022-10-13T19:44:48Z

specs/Deployment/DeploymentAPI.md

+
+```C#
+    if (DeploymentManager.GetStatus().Status == DeploymentStatus.PackageRepairRequired ||
+        DeploymentManager.GetStatus().Status == DeploymentStatus.Unknown)


Is it right to call GetStatus() twice? Could the status change between the two calls?

oldnewthing · 2022-10-13T19:47:39Z

specs/Deployment/DeploymentAPI.md

+When the API is updating Framework package and ForceDeployment option is set, then all dependent
+packages that are NOT currently in use will be immediately re-installed to refer to the updated
+Framework package and all dependent packages that are currently in use will be re-installed, after
+they shut down, to refer to the updated Framework package.


Wait, I thought the first paragraph in this section said that if you force deployment, then we don't want for the users to exit the apps; we shut down the apps immediately. But here, we wait for the apps to shut down naturally.

Or is "after they shut down" saying "after we shut them down" (rather than the app shutting down on its own)? Could use some clarity about whether the shutdown is immediate or eventual.

oldnewthing · 2022-10-13T19:48:23Z

specs/Deployment/DeploymentAPI.md

+        var deploymentRepairOptions = new DeploymentRepairOptions();
+        deploymentRepairOptions.ForceDeployment = true;
+
+        // Repair will always attempt to repair WindowsAppRuntime regardless of it's state.


Suggested change

// Repair will always attempt to repair WindowsAppRuntime regardless of it's state.

// Repair will always attempt to repair WindowsAppRuntime regardless of its state.

oldnewthing · 2022-10-13T19:49:26Z

specs/Deployment/DeploymentAPI.md

+DeploymentManager::Repair(...)
+{
+    // Only needed supported in packaged processes
+    IF IsPackagedProcess()


Test seems backwards.

…oymentRepairAPISpec

…b.com/microsoft/WindowsAppSDK into user/sachinta/DeploymentRepairAPISpec

DeploymentManager spec update for the new Repair API

8598fb2

sachintaMSFT requested a review from DrusTheAxe August 18, 2022 00:31

ghost added the needs-triage label Aug 18, 2022

DrusTheAxe reviewed Aug 18, 2022

View reviewed changes

Incorporate PR feedback-1

ac61f62

sachintaMSFT requested a review from cwruss August 19, 2022 19:23

sachintaMSFT added area-Deployment Issues related to packaging, installation, runtime (e.g., SelfContained, Unpackaged) feature and removed needs-triage labels Aug 19, 2022

sachintaMSFT force-pushed the user/sachinta/DeploymentRepairAPISpec branch 2 times, most recently from 65cfd5a to ac61f62 Compare September 20, 2022 21:50

Santosh Chintalapati added 3 commits September 23, 2022 11:33

Merge remote-tracking branch 'origin/main' into user/sachinta/Deploym…

4cf032f

…entRepairAPISpec

Merge remote-tracking branch 'origin/main' into user/sachinta/Deploym…

88853b1

…entRepairAPISpec

Add DeploymentRepairOptions

bf570c5

sachintaMSFT requested a review from DrusTheAxe September 25, 2022 14:34

DrusTheAxe approved these changes Sep 26, 2022

View reviewed changes

DrusTheAxe requested changes Sep 26, 2022

View reviewed changes

sachintaMSFT commented Sep 28, 2022

View reviewed changes

specs/Deployment/DeploymentAPI.md Outdated Show resolved Hide resolved

This was referenced Sep 29, 2022

Add DeploymentRepairOptions #3001

Open

Cherry-Pick "Deployment Manager Repair API (#2954)" #2999

Closed

Santosh Chintalapati added 2 commits September 29, 2022 13:35

Merge branch 'user/sachinta/DeploymentRepairAPISpec' of https://githu…

8b09dec

…b.com/microsoft/WindowsAppSDK into user/sachinta/DeploymentRepairAPISpec

As Repair API is now targeted for 1.3 release, increment DeploymentCo…

c208a25

…ntract version.

MikeHillberg reviewed Oct 4, 2022

View reviewed changes

FrankC-msft reviewed Oct 4, 2022

View reviewed changes

sachintaMSFT commented Oct 6, 2022

View reviewed changes

FrankC-msft reviewed Oct 6, 2022

View reviewed changes

oldnewthing reviewed Oct 13, 2022

View reviewed changes

sachintaMSFT removed the request for review from cwruss January 26, 2023 14:00

sachintaMSFT changed the base branch from main to develop January 26, 2023 14:00

Merge remote-tracking branch 'origin/develop' into user/sachinta/Depl…

8e8f423

…oymentRepairAPISpec

Merge branch 'user/sachinta/DeploymentRepairAPISpec' of https://githu…

c69475a

…b.com/microsoft/WindowsAppSDK into user/sachinta/DeploymentRepairAPISpec

bpulliam added the api-design Updates to Project Reunion API surfaces label Mar 27, 2023

bpulliam added the status-no recent activity label May 8, 2023

codendone added feature proposal and removed feature labels May 23, 2024

		@@ -143,18 +161,16 @@ by using DeploymentInitializeOptions object and setting ForceDeployment option b
		this API. This option when set will shut down the application(s) associated with WinAppSDK packages,
		update the WinAppSDK packages and restart the application(s).

		the system?
		3. How do I repair the already installed WinAppSDK Main and Singleton packages, if needed ?

	// Repair will always attempt to repair WindowsAppRuntime regardless of it's state.
	// Repair will always attempt to repair WindowsAppRuntime regardless of its state.

DeploymentManager spec update for the new Repair API #2869

Are you sure you want to change the base?

DeploymentManager spec update for the new Repair API #2869

Conversation

sachintaMSFT commented Aug 18, 2022 • edited

DrusTheAxe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrusTheAxe Oct 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachintaMSFT Oct 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachintaMSFT Oct 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachintaMSFT Oct 6, 2022 • edited

Choose a reason for hiding this comment

sachintaMSFT Oct 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachintaMSFT commented Aug 18, 2022 •

edited

DrusTheAxe Oct 4, 2022 •

edited

sachintaMSFT Oct 4, 2022 •

edited

sachintaMSFT Oct 6, 2022 •

edited

sachintaMSFT Oct 6, 2022 •

edited

sachintaMSFT Oct 6, 2022 •

edited