Config code #2942
Config code #2942
Conversation
…nality; add progress to apply and test to enable direct feedback
…it processor infra and processor itself. Implement progress callbacks
This comment has been minimized.
This comment has been minimized.
| SemanticVersion(const std::string& version) : | ||
| SemanticVersion(std::string(version)) {} | ||
|
|
||
| void Assign(std::string version, std::string_view splitChars = DefaultSplitChars) override; |
There was a problem hiding this comment.
Not std::string&& or const std::string& ?
There was a problem hiding this comment.
This is the signature of the virtual Assign method. I do move version in the implementation.
There is an argument for the non-constructor based call to Assign to also allow a const& version to maximize efficiency, but it isn't really worth the effort to keep two versions of the same method that only differ in how they assign to m_version.
| // ----------------------------------------------------------------------------- | ||
| // <copyright file="UnitTestCollection.cs" company="Microsoft Corporation"> | ||
| // Copyright (c) Microsoft Corporation. Licensed under the MIT License. | ||
| // </copyright> | ||
| // ----------------------------------------------------------------------------- |
There was a problem hiding this comment.
| // ----------------------------------------------------------------------------- | |
| // <copyright file="UnitTestCollection.cs" company="Microsoft Corporation"> | |
| // Copyright (c) Microsoft Corporation. Licensed under the MIT License. | |
| // </copyright> | |
| // ----------------------------------------------------------------------------- | |
| // Copyright (c) Microsoft Corporation. | |
| // Licensed under the MIT License. |
There was a problem hiding this comment.
We use the C# standard header in our other C# files. Not sure if that is "the right thing to do", but it is the current convention.
| /// <summary> | ||
| /// Gets the message sink for the fixture. | ||
| /// </summary> | ||
| public IMessageSink MessageSink { get; private set; } |
There was a problem hiding this comment.
| public IMessageSink MessageSink { get; private set; } | |
| public IMessageSink MessageSink { get; init; } |
Not sure if we're on the right version for this, or if it does what I think it does...
There was a problem hiding this comment.
It does, but I still want private init because I want the constructor path to be used. public init would be a good way to provide optional construction-time parameters though.
| #include <winrt/Windows.Foundation.Collections.h> | ||
| #include <winrt/Windows.Storage.Streams.h> | ||
|
|
||
| // TODO: Is this needed? |
There was a problem hiding this comment.
The comment, no. What it was referring to, honestly I have no idea 😉
| Unknown, | ||
| Pending, | ||
| InProgress, | ||
| Completed, |
There was a problem hiding this comment.
Do we need/want to add comments to each value? I thought the docs were automagically generated from the IDL for English.
There was a problem hiding this comment.
I'm not aware of automagic documentation, but I can add comments.
| Windows.Foundation.Collections.ValueSet Directives{ get; }; | ||
|
|
||
| // Contains the values that are for use by the configuration unit itself. | ||
| Windows.Foundation.Collections.ValueSet Settings{ get; }; |
There was a problem hiding this comment.
How is this different from IConfigurationUnitSettingDetails?
There was a problem hiding this comment.
These are the settings values to use for the particular unit of configuration being described by the object. IConfigurationUnitSettingDetails is a description of the settings value available for the configuration unit.
Basically the difference between the actual values passed in to the command line for winget (this Settings) and the help output by winget for the command (IConfigurationUnitSettingDetails).
| // The state of the configuration set for this event (the ConfigurationSet can be used to get the current state, which may be different). | ||
| ConfigurationSetState SetState{ get; }; | ||
|
|
||
| // The state of the configuration unit for this event (the ConfigurationUnit can be used to get the current state, which may be different). | ||
| ConfigurationUnitState UnitState{ get; }; |
There was a problem hiding this comment.
We only have one of those at a time depending on the type?
There was a problem hiding this comment.
Yes, and it doesn't seem worthwhile to create three runtime classes and require a QI to get any useful data out.
| // The state that the set is in. | ||
| ConfigurationSetState State{ get; }; | ||
| // The time that this set was recorded with intent to apply. | ||
| Windows.Foundation.DateTime InitialIntent{ get; }; |
There was a problem hiding this comment.
The name of this property makes me think "what was the initial intent", not "when was the initial intent"
There was a problem hiding this comment.
True, I wrote this "intent" before I added the other "intent" property. Changed to FirstApply unless there are other suggestions.
| TestSettingsResult TestSettings(); | ||
|
|
||
| // Gets the current system state for the configuration unit. | ||
| GetSettingsResult GetSettings(); | ||
|
|
||
| // Applies the state described in the configuration unit. | ||
| ApplySettingsResult ApplySettings(); |
There was a problem hiding this comment.
Here we use Test/Get/Apply, but elsewhere it was Assert/Inform/Apply
There was a problem hiding this comment.
I wanted to separate the semantics, but maybe it is also confusing...
|
|
||
| // The result of applying the settings for a configuration unit. | ||
| [contract(Microsoft.Management.Configuration.Contract, 1)] | ||
| runtimeclass ApplyConfigurationUnitResult |
There was a problem hiding this comment.
What's the difference with ApplySettingsResult?
There was a problem hiding this comment.
ApplyConfigurationUnitResult is the interface for users of Microsoft.Management.Configuration. ApplySettingsResult is the interface for implementors of IConfigurationUnitProcessor.
…ing on it to come through the dependency
| <IntDir>$(PlatformTarget)\$(Configuration)\</IntDir> | ||
| <TargetName>$(RootNamespace)</TargetName> | ||
| <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors> | ||
| <RunCodeAnalysis>false</RunCodeAnalysis> |
There was a problem hiding this comment.
I think we should turn it on in debug builds to match other libs we build
src/AppInstallerCLI.sln
Outdated
| {CA460806-5E41-4E97-9A3D-1D74B433B663}.TestRelease|x86.Build.0 = Release|Win32 | ||
| {E8454BF1-2068-4513-A525-ABF55CC8742C}.Debug|Any CPU.ActiveCfg = Debug|x64 | ||
| {E8454BF1-2068-4513-A525-ABF55CC8742C}.Debug|ARM.ActiveCfg = Debug|Any CPU | ||
| {E8454BF1-2068-4513-A525-ABF55CC8742C}.Debug|ARM.Build.0 = Debug|Any CPU |
There was a problem hiding this comment.
ARM should point to ARM?
I saw Release and TestRelease below all point to Any Cpu by the way
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Debug|Any CPU.ActiveCfg = Debug|x64 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Debug|ARM.ActiveCfg = Debug|x64 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Debug|ARM.Build.0 = Debug|x64 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Debug|ARM64.ActiveCfg = Debug|arm64 |
There was a problem hiding this comment.
nit: Arm should point to Arm for consistency
src/AppInstallerCLI.sln
Outdated
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Debug|x86.Build.0 = Debug|x86 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Fuzzing|Any CPU.ActiveCfg = Debug|x64 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Fuzzing|Any CPU.Build.0 = Debug|x64 | ||
| {EE43C990-7789-4A60-B077-BF0ED3D093A1}.Fuzzing|ARM.ActiveCfg = Debug|x64 |
There was a problem hiding this comment.
Arm -> Arm
And all below instances
Microsoft.Management.Configuration.Process is a cswinrt component that provides implementation of `IConfigurationSetProcessor`, `IConfigurationUnitProcessor`, `IConfigurationUnitProcessorDetails`, `IConfigurationUnitSettingDetails` and `IConfigurationProcessorFactory` defined in #2942 as well as new runtime classes and an interface. `ConfigurationSetProcessorFactory` implements `IConfigurationProcessorFactory`, which takes the type of configuration and optional `IConfigurationProcessorFactoryProperties` with additional properties that need to be defined for the processor set. An implementation of `IConfigurationProcessorFactoryProperties` is also provided. Internally, it uses PowerShell Host to find and install DSC modules and apply them via InvokeDscResource. Requires [PSDesiredStateConfiguration 2.0.6](https://www.powershellgallery.com/packages/PSDesiredStateConfiguration/2.0.6)
This work is a small part of #2845
Change
Introduces a new component,
Microsoft.Management.Configuration. This is the API surface for controlling the configuration of the system for a user (with the intention that any user can run it, includingSYSTEM). There are extensive details on the API in this document, so I will not repeat them here.The API is not fully implemented by this change, only the baseline functionality for reading configuration files and acting on them immediately. None of the history functionality is present, nor any ability to serialize files. However, this should be sufficient to create an initial, golden path implementation of user interactive surfaces.
Speaking of the user interface, the plan is to add functionality to both
wingetand PowerShell. Thewingetimplementation will come first, if only because it is easier to integrate there in general. The PowerShell module is expected to ship simultaneously with a supported release.In addition to this portion, the back-end functionality is coming soon in a separate PR.
Validation
New unit tests are added specifically for this component.
Microsoft Reviewers: Open in CodeFlow
Microsoft Reviewers: Open in CodeFlow