PowerShell cmdlets #1407
PowerShell cmdlets #1407
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
||
|
|
||
| ```PowerShell | ||
| Get-WinGetSource | convertto-json -compress |
There was a problem hiding this comment.
Since this would entirely rely on the output object structure this approach could break easily. It might make more sense to handle this via a -Raw parameter, e.g.
Get-WinGetSource -Raw
There was a problem hiding this comment.
The cmdlets should emit .NET Objects (for consumption by other PowerShell cmdlets) along with JSON only where necessary.
There was a problem hiding this comment.
NOte - if the intention is to handle cmdlets using Crescendo - then this can be met, provided the team do a good job at writing the necessary output handelers. And for the record, with Cresendo, this is NOT an easy task unless you are outstandingly good with regular expressions.
With that said, the cmdlets must emit objects - and some of those objects can contain json where json is relevant.
| ``` | ||
| **Parameters** | ||
|
|
||
| -Output |
There was a problem hiding this comment.
-Path and -LiteralPath might be better parameter names for -Output. See Export-Csv
There was a problem hiding this comment.
@megamorf what is the difference between the two?
There was a problem hiding this comment.
Per convention, -Path accepts and interprets wildcards and globs. LiteralPath does not. There's probably official docs for this as it's very standard
There was a problem hiding this comment.
Correct. The official documentation says the following:
Unlike Path, the value of the LiteralPath parameter is used exactly as it is typed. No characters are interpreted as wildcards. If the path includes escape characters, use single quotation marks. Single quotation marks tell PowerShell not to interpret any characters as escape sequences.
Why is that important? Let's consider that you have a folder with square brackets or an asterisk in its name. -Path will treat that as an expression rather than a true representantion of the path. Users have learned to use -LiteralPath when they literally want PowerShell to treat the user specified path as it is.
There was a problem hiding this comment.
OK, so rather than "-Output" I believe what I'm reading is we should support both "-Path" and "-LiteralPath". Is it safe to assume PowerShell handles the wildcards when "-Path" is used?
There was a problem hiding this comment.
So far as I can recall, -Path takes a wildcarded string and the cmdlet does the variable expansion.
There was a problem hiding this comment.
OK, so rather than "-Output" I believe what I'm reading is we should support both "-Path" and "-LiteralPath". Is it safe to assume PowerShell handles the wildcards when "-Path" is used?
AFAIK the -Path string is passed to the so called "provider" and the interpretation of wildcards is up to the implementation of that provider. PowerShells "path-related" cmdlets can interact with multiple providers, e.g. the Certificate Store, ActiveDirectory, the registry OR the filesystem. So the meaning of * may be different for an NTFS filesystem path / provider implementation than for the registry or for an ActiveDirectory LDAP path. It would be best to speak to the professionals, PowerShell team, about their preference here. Another existing cmdlet that creates a file and writes content to it is Set-Content - it too supports -Path and -LiteralPath but when trying to use it with a registry-path I got an error that the functionality is not implemented by the provider - similar to how winget wouldn't support exporting a packageset to the registry or certificate store I imagine:
❯ Set-Content -path "HKCU:\Software\XD" -Value 'kek'
Set-Content: Cannot use interface. The IContentCmdletProvider interface is not implemented by this provider.| >Note: The current behavior for `winget show vscode` would look like the example below. | ||
|
|
||
| ```PowerShell | ||
| Find-WinGetPackage -Moniker vscode | Get-WinGetPackageVersion -Detail | ||
| ``` | ||
|
|
||
| >Note: The current behavior for `winget show vscode --versions` would look like the example below. | ||
| ```PowerShell | ||
| Find-WinGetPackage -Moniker vscode | Get-WinGetPackageVersion -All | ||
| ``` |
There was a problem hiding this comment.
This is valid PowerShell code but needlessly complicated from a user experience perspective. Many users will expect these actions to be done with a single command. It'll be interesting to see what feedback you'll get here :-)
There was a problem hiding this comment.
Find-Package should, uhh, find packages and return all the information. C/F Find-Module. Please do not introduce needless complexity where it can be avoided.
There was a problem hiding this comment.
I disagree. AFAIK winget repositories use a cached index to speed up searches. It makes a ton of sense to return only the minimal, cached set of data first and then get all the information (which is an expensive operation) only for the package you're really interested in. Other existing cmdlets such as Get-ADUser and Get-WindowsDriver take a similar approach for performance and server-load reasons.
There was a problem hiding this comment.
If the approach is to return a minimal amount of properties, then two things:
- The Find-WinGetPackage should return enough properties, by default, to be useful. As you mantion, like Get-ADuser etc.
- If there are more properties not returned by default - then use a -Property parameter to specify which optional properties to return.
In this case, I'd argue that the version of a package is important and probably should be returned by default.
There are two issues therefore: what properties get returned by default and how to get more. I do not support adding cmdlets in the proposed manner.
Just checking, since it's not in this spec file, these cmdlets will be targeting Windows PowerShell (5.1), not PowerShell Core, right? We can't guarantee that PowerShell Core is installed since it's not shipped with Windows (and it isn't usually installed on systems that aren't used for development unless you work at a really hip company).
I sure hope not. It would be crazy, this long after PowerShell 7 has shipped to be so backwards as to ONLY target WIndows PowerShell.
There was a problem hiding this comment.
I sure hope not. It would be crazy, this long after PowerShell 7 has shipped to be so backwards as to ONLY target WIndows PowerShell.
I'd disagree. For all that Microsoft want PowerShell 7 to be the future, I don't know a single enterprise that's using it, precisely because it's not a backwards compatible, drop in replacement for PowerShell 5. In every current, supported version of Windows, PowerShell 7 is a side by side addon and the chance of MS actually ever releasing an in place upgrade from PS5 to PS7+ is slim to none, so PowerShell 5 is going to continue to be what is used by enterprise admins for years to come.
So yeah, IMHO this module absolutely needs to be PS 5.1 compatible.
There was a problem hiding this comment.
@JustSomeGuyNZ I agree with your point that there are companies that will never allow PS 7 to be deployed. I do not get the reason to be honest. I consult customers to deploy it widely just for the same of improved commandlets like test-connection etc.
It's an MSI package, easy to deploy with a small mst. What is their issues with PS 7? Do you have more information about it? I just know it is a controversy because of patchmanagement.org and also some ConfigMgr MEMCM pals.
There was a problem hiding this comment.
It's not a drop in replacement, and it breaks backwards compatibility. In my experience enterprise IT is pretty resistant to change that breaks backwards compatibility and unforced changes lol.
There was a problem hiding this comment.
I've been engaging the PowerShell team in the discussions with our design, and the crescendo team to aid in rapid prototyping. We will follow their guidance with respect to which versions of PowerShell behave differently so we can maintain the greatest compatibility level. I am certainly not a PowerShell expert, so I'm learning from the experts on that front. I wanted to get this draft out quickly so we could get community feedback. I'm building on the list of concerns brought up here so they can be clearly addressed in the document.
This is excellent news and much to be welcomed. Knowing what I do, Crescendo should provide a good way forward and helps to move this forward and makes the best out of the decision to do this all in C++. But we are where we are, and Crescendo does look promising as a way forward. It also gives the winget team to re-write certain cmdlets in C# where that makes business sense.
As I stated a while ago, creating the object model is very important. With Crescendo, as I understand it, you use regular expressions to create objects that the Crescendo-generated cmdlet emits. As I have said elsewhere, this is going to be a very difficult task to do well and completely - string to object translation is not an exact science. Thus my first concern is the objects emitted. It should be a goal for winget-ps to emit objects that are as rich and functional as those emitted by other modules and usable in rich cross-silo scenarios.
The second concern is over verb usage. As mentioned a long time ago, the subcommand of winget do not line up completely with PowerShell standard verb naming. For example, the show command in winget should probably be get. And hash should be something else. I would like to see the command to verb translation table - and please, please, please only uses approved verbs in all cmdlets.
I look forward to seeing how this progresses.
There was a problem hiding this comment.
I've been going through the process of learning the PowerShell verbs and continuing to expand the draft to others so we can get a sense of the best verbs and behaviors. First impressions are very valuable in this area. There is definitely some impedance to overcome between the .exe cli syntax and PowerShell. We're heads down on the v1.1 release, and then I'll have more time to dedicate to fleshing this out. We have been discussing the rich object model needed for a good PowerShell experience.
There was a problem hiding this comment.
The verb choice is a vital component of the sacred vow. IT Pros would expect to use certain verbs and certain parameter names names. To create a truly great Powershell module means using approved verbs. :-)
|
Looks like a solid first draft overall. I have noticed there is no mention of ParameterSets, but some cmdlets parameters should probably be mutually exclusive e.g. these three wherever they are used: I think a user should not be able to specify more than one of these for any given command. |
|
It looks like we're going to go with Get-WinGetPackage for "list" a.k.a. installed packages and Find-WinGetPackages for "search" a.k.a. packages available in sources. We're still debating some options for "show". We're not really displaying an entire manifest, but we're returning an object that is a bit of a subset of a manifest. We're still looking at parameters and verbs that might fit the bill better in this scenario. |
This combo would be my preferred config. 👍 |
|
How about "Find-WinGetInstaller" for |
You could do something like This would also open the door for cmdlets like EDIT - Additional thoughts: Frankly, I think if we have |
I like it, but wondering if Find-WinGetManifest might fit better? Or is that too controversial since you will only be returning a bit of the manifest? |
I would think that |
|
For the "medium detail level" cmdlet (aka Show-WinGetPackagePros:
Cons:
Find-WinGetPackage -DetailedPros:
Cons:
Find-WinGetPackageDetailPros:
Cons:
I wouldn't want to use the noun "WinGetManifest" for an object that isn't actually the entire package manifest - that'd just be confusing and make cmdlet names even weirder once we have cmdlets that do actually have to interact with the full manifest (like for validating or uploading a package manifest). |
I think it would be able to be? Doesn't powershell just cycle through available completion options?
|
|
@Trenly oh uh, yea - I forgot that it has that horrible completion behavior by default (it takes way too long to tab through stuff, and it's easy to tab past what you wanted). I have always adjusted my completion setting to be bash-like with: But yea, you are right. It wouldn't matter for the default completion behavior. Me and the other users who use the "Complete" setting though would still appreciate the different cmdlet names :P I updated my comment above |
|
We're going to have "Get-WinGetManifest" for operations against a REST source. |
Remember to not be limited by how the current Also, the source type should not ever be a consideration for API design at this level. They are all black box repositories of packages. Yes, there might be some special flags that only affect a subset of types, but that is acceptable so long as the underlying code is handling it, not this layer. |
|
There may have been a lack of clarity on my part. We're not designing the PowerShell API based on the constraints of the Windows Package Manager. We're designing the API to work in the future anticipating changes in the Client. The functionality we're providing in this early "alpha" stage will take advantage of the portions of the system available today, and will be extended as improvements in the COM interface are made. We're anticipating some performance hits in some areas due to the current implementation when we might need several round trip calls to populate the object model in some areas, and in some cases where we truncate strings in the table formatter, PowerShell pipelining will not work as expected in every case, so we're putting comments/documentation in place to call those out. We're also looking for possible approaches in PowerShell to at least be able to detect when this is happening so users can decide how to move forward util we have full fidelity. |
|
It seems to me that instead of emitting mangled strings, there should be an option at least for Crescendo, that send the output ion JSON, and enables Crescendo to do the conversion into objects. When will the team publish the object model? |
|
We still have to take what we get from the client (strings in most cases) and build objects. Crescendo ended up not being quite as useful as we had hoped due to the difference between the command line verbs/switches and PowerShell. One exception was the Since we ended up with four modules, we're going to create a new repository to get the PowerShell build pipelines set up, then we will move the "child" modules back to their respective repositories. You can see the work in progress on my fork. |
|
@denelon I see your repo and I find 2 big issues:
|
|
Regarding @leoniDEV's comment - it looks like the Write-Host is on an error path just before throwing an exception, so it would work in the normal case (well as I read it). There has to be a better way do throw that exception though. And as an aside: both as a tool for debugging and a tool for letting the IT pro have more useful info, consider populating the functions with Write-Verbose and expose operations inside. The user never has to see them so it's no big deal leaving them in. And when you are debugging, especially with Crescendo functions, verbose output would be useful from time to time. |
Already started making that shift in the Module 😊 returning informative objects for reference in a failure. |
|
The Microsoft.WinGet.Source PowerShell module was released at https://github.com/microsoft/winget-cli-restsource/releases as a preview version. I've created a discussion to gather feedback. |
That's great to hear. I'll check the code base and cmdlet behaviour this weekend and add my feedback 👍 |
|
We will be releasing the other modules containing cmdlets via another open source repository here at GitHub. They will be migrated to their respective repositories as they become a bit more complete. We will keep the other repository for the parent module designed to import all three of the modules we've defined: Microsoft.WinGet
We're currently dotting the "I"s and crossing the "T"s to release the project. Update: we've decided to keep the modules in the associated repository: Microsoft.WinGet winget-cli
|
|
@denelon why did you not merge the spec pr? |
#674
Microsoft Reviewers: Open in CodeFlow