Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable configuration via IConfigurationStatics #3576

Merged
merged 7 commits into from Sep 5, 2023

Conversation

msftrubengu
Copy link
Contributor

@msftrubengu msftrubengu commented Aug 31, 2023

Winget configuration is not going to be enabled by default. The user needs to opt-in by calling winget configuration --enable. Callers that are interested in using the OOP Microsoft.Management.Configuration APIs need a way to ensure configuration is enabled and enable it if not. This PR address that.

New IConfigurationStatics methods

  • IsConfigurationEnabled - Checks if the configuration is enabled by checking if the current AppInstaller package is the stub or full package. Inproc implementation always returns true.
  • EnableConfigurationAsync - Asynchronously enables configuration. This changes the stub preference to full package and calls our own APIs to install AppIntaller. By design, this also updates AppInstaller to the newest version.

Caveats

Callers need to understand that this is effectively saying "winget please update yourself". Which means that once EnableConfigurationAsync is done, configuration isn't ready yet. At this point, deployment scheduled AppInstaller for an update. The server process that owns the configuration statics object is already gone (or is about to) and a new instance of ConfigurationStaticFunctions needs to be created in order validate configuration is enabled. It is possible that creating that object fails with an error that the upgrade is in progress. Yes, this is awkward.

Testing

It is complicated to write automated tests for this without faking most of the stack. I manually created an AppInstaller package with an older version to what we internally have to do these tests. Then I used the current OOP configuration tests to update it.

This is how I programmatically verified it

            {
                var statics1 = new ConfigurationStaticFunctions();

                Assert.False(statics1.IsConfigurationEnabled);

                var task = statics1.EnableConfigurationAsync();
                task.Progress = (asyncInfo, progress) =>
                {
                    // show progress
                };

                await task;
            }

            await System.Threading.Tasks.Task.Delay(120000);

            {
                var statics2 = new ConfigurationStaticFunctions(); // This failed with E_NOINTERFACE because the "newer" package in the store doesn't have my changes
                Assert.True(statics2.IsConfigurationEnabled);
            }
Microsoft Reviewers: Open in CodeFlow

@msftrubengu msftrubengu requested a review from a team as a code owner August 31, 2023 23:47
Copy link
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a static inside a function rather than a global, but I can live with the global for now.

@msftrubengu msftrubengu merged commit 8a15f53 into microsoft:master Sep 5, 2023
8 checks passed
@msftrubengu msftrubengu deleted the stubstatics branch November 14, 2023 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants