Add pwsh support for trust level and explicit #4750
Conversation
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
| @@ -262,6 +262,12 @@ namespace Microsoft.Management.Deployment | |||
| PackageCatalogOrigin Origin { get; }; | |||
| /// The trust level of the package catalog | |||
| PackageCatalogTrustLevel TrustLevel { get; }; | |||
|
|
|||
| [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 10)] | |||
There was a problem hiding this comment.
You can't add things to contract 10, it shipped with 1.7.
On a related note, when you add contract 11 at the top of the file, put a comment above it like // For version 1.9
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| if (string.IsNullOrEmpty(type)) | ||
| { | ||
| _ = this.Run("source", $"add --name {name} --arg {arg}", 300000); | ||
| parameters = $"add --name {name} --arg {arg}"; |
There was a problem hiding this comment.
Not related to this change, but this code is not resilient to spaces in the arguments.
| /// <summary> | ||
| /// Gets the trust level of the source. | ||
| /// </summary> | ||
| public string TrustLevel { get; private set; } |
There was a problem hiding this comment.
Why is this a string and not a PSSourceTrustLevel?
Maybe that is normal in PowerShell, it just seems odd to me.
There was a problem hiding this comment.
The main reason is not load any dependency when the CLR loads this binary as the ALC hasn't been being configured yet at this point. For cswinrt enums, there are PSSomething equivalents that get converted to string and then converted to the cswinrt representation in the engine module. Since this doesn't have it he had to go with strings,
There was a problem hiding this comment.
Is it that we can't have a dependency inversion with Engine taking a dependency on Cmdlets? And we can't have this kind of immediate dependency on Engine in types exposed on the interface of any cmdlet because we don't have the ALC yet to resolve it (thus preventing us from putting PSSourceTrustLevel in Engine)?
In any case, I don't have a problem with it being a string. Just wasn't clear why.
There was a problem hiding this comment.
Yeah, we don't want to have the dependency on Engine until the ALC is setup
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| switch (trustLevel) | ||
| { | ||
| case PSSourceTrustLevel.None: | ||
| trustLevelString = "None"; |
There was a problem hiding this comment.
You could do switch return with PSSourceTrusLevel.Default => string.Empty and _ being trustLevel.ToString()
There was a problem hiding this comment.
Changed it to your fancy switch pattern syntax.
| /// <summary> | ||
| /// Gets the trust level of the source. | ||
| /// </summary> | ||
| public string TrustLevel { get; private set; } |
There was a problem hiding this comment.
The main reason is not load any dependency when the CLR loads this binary as the ALC hasn't been being configured yet at this point. For cswinrt enums, there are PSSomething equivalents that get converted to string and then converted to the cswinrt representation in the engine module. Since this doesn't have it he had to go with strings,
Adds support for
-TrustLeveland-Explicit.The pwsh tests can now use the Add-winGetsource cmdlet to modify the trust level.
Updated Get-WinGetsource to report the TrustLevel and Explicit flag.
Updated tests and docs to reflect these changes.
Microsoft Reviewers: Open in CodeFlow