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
PSNativeCmdlet class - wrapper for native applications #13428
Comments
Don't replicate ALL discussions to the issue - please continue discussions in specific issues. |
I'm not really fully clear what this all means, but inferring what I can, it sounds like the idea is to build a whole new command processor to handle this -- among several other related tie-ins to existing subsystems. If I'm honest, I think this is probably massively complicated to approach in this manner. It may well be a more complete approach to resolving the problems we're seeing, but I think the necessary time to put this kind of change together and review it, and then figure out how to test it... would be pretty significant. I'm not sure the slated benefit is worth the level of complexity being proposed here. 🤔 |
You see a complicity only because all features from different issues we want implement are enumerated at once. |
To be clear - the surface features I think are desirable. But I don't think the proposed implementation is the way to go, there seem to be too many pieces to fix what really boil down to 2-3 much simpler requests:
In my own opinion, it's not worth duplicating significant portions of the engine code. These issues are, effectively, either bugs or feature requests that can be resolved in a much less expensive fashion -- if we're willing to tackle the issue of them introducing breaking changes here and there. It's my opinion that this approach is significantly more expensive in all respects than simply taking the necessary cost of a couple of breaking changes. |
Where do you see "duplications" in Engine? The proposal says about (1) step-by-step (2) injection in some Engine subsystems. Why do you say about complicity the work?
|
I can't speak to the complexity of the implementation of a PSNativeCmdlet type but the issues around using native exes have been around for 15 years. I don't want to see another "bandaid" (ahem --%) applied. So, whatever gets done, I'd love to see that adequate time & thought is given to the implementation the team comes up with. But please, let's not add yet another bandaid that will require additional fixes down the road and leave users with more bandaids to determine if they should use or not. I realize you can't anticipate everything. I suspect the team working on v1 never anticipated running on macOS and Linux. But given everything we do know, let's make sure the solution is well thought out and implemented even if it winds up being complex. Honestly, I'd rather wait a bit longer to really nail these native exe issues. In the meantime, modules like @mklement0's |
That would require command providers to bundle manifests that PowerShell can read. |
@rkeithhill With the proposal end users will run native applications as is without any additional --% and cmdlets. They only have to load the module with wrappers. |
@iSazonov, command-individual wrappers are not the right solution to this problem. We don't need to wrappers for external executables, we just need argument passing to work, fundamentally, with any executable. To put it differently, quoting the first part of the linked blog series:
It's precisely because that is not true that we need a fundamental fix - and that is the focus of #1995. #1995 (comment) now lays out the fix in detail:
The only question is: how do we make this fix available, if a direct fix is - at least for now - not an option? To be clear: a direct fix is clearly the most desirable option, given that it is a severe handicap for a shell to not properly pass arguments to external executables. Shipping an
As for the other areas of functionality mentioned in this proposal, as summarized by @vexx32 above:
You don't need a new, cmdlet-derived class to get tab completion with external executables; for instance, @bergmeister's
PowerShell/PowerShell-RFC#88 should address the fundamental lack of integration of external executables with PowerShell's error handling. #13361 just fixed the problem with #4332 would give us the ability to capture stderr output in a variable. Overall, I'm not convinced we need engine-level support for wrapping the functionality of executables, such as parsing their text output into objects. |
@mklement0 All you say about #1995 right in thoughts but... PowerShell lives with this over 15 years. And you know MSFT team said a day before - "we postpone this until 7.2". You can read this "We will think about it next year". It's a polite form to say NO. So you have two alternatives - (1) stop posting about this until 7.2 (and then wait 7.3, 7.4...) (2) find an compromise solution. This issue is also about in-depth refactoring. Today @daxian-dbw and MSFT team are thinking/working on SMA refactoring to loadable subsystems. As part of the work they could thing about refactoring these subsystems because this code is old and overgrown with extensions over the years. I suppose its partial refactoring is inevitable when creating subsystems. PSNativeCmdlets proposal is a way to make internal and external design simpler and clearer without breaking changes and the implementation faster and more compact. I believe this proposal has even more possibilities than announced. |
Indeed, and any day longer is unacceptable.
I don't think there's any reason to wait; if this really won't happen until 7.2, then we have more time for discussing and honing proposals, though I think the above proposal - which took a while to work out in detail - is stable now. In general, I believe in describing what (debatably) should be the right solution, even if the powers that be decide against it.
What I'm proposing is a compromise solution - one that's easy to implement in the short term.
I don't see any connection between this proposal and #1995. |
This is the status #15143 (comment) They added It works correctly for most of the applications now (at least those that use Its Not great, not terrible. #15408 |
I can't speak to the official plans, but since the experimental feature was made a stable feature in preview.8, If these defaults are retained for the official v7.3 release, they amount to a substantial breaking change.
Assuming you mean encoding all arguments to pass to an executable via a single string: No, you could never do that, and you won't be able to now. Also, the exception with It's also worth repeating that the old parsing is horribly broken, and that continuing to force users to resort to it - which requires knowing the specifics of how it is broken and carrying that unfortunate legacy forward - was a terrible decision. And let's not forget high-profile CLIs such as |
That's what I got from reading the source code with the changes, I thought it would in theory make the The way the code is written, there's no accounting to make the encoding of the single string right for the |
The default, if I'm understanding now, that it will be the correct behavior for anything that's not the But it's not going to be a breaking change if you use
That's what I though it would do when you choose the opt-in And yet again, if they ship it, as-is, they are going to make it worse by not providing the proper behavior. That enum is going to become Hell ! public enum NativeArgumentPassingStyle
{
Legacy = 0, // legacy is legacy
Standard = 1, // this is just CommandLineToArgvW, breaks everything basically
Windows = 2, // this is CommandLineToArgvW for anything but `cmd` on windows
NowRealStandardThatWorksAndIsNotOptOut = 3 // does what your InvokeNative Powershell module does for `cmd` and `Standard` for others.
/// all else is legacylegacy, this becomes opt-in behavior , windows becomes legacywindows and opt-out , what a freak way of dealing with versioning
} Maybe they should just have published a new |
Hear, hear, @Luiz-Monad. While I share your frustration with respect to the issue at hand, the situation isn't quite as dire as you made it sound - though undoubtedly still bad enough: If the current defaults ( Update: They did make into v7.3.0, but it looks like some version after v7.3.1 will revert to the legacy behavior by default and make the new, correct behavior opt-in rather than opt-out, but on Windows only - see #18694. That is, the future defaults will most likely be
That certainly would have made for a less painful situation, but it's hard to justify having to use a cmdlet / function for something as fundamental to a shell as calling external programs with arguments is. Given that It's also worth noting that the current preference-variable-based solution is inherently problematic, as all preference variables are, due to PowerShell's dynamic scoping: Unless their values are carefully managed in terms of their scope (saving and restoring values), it is easy to inadvertently impact unrelated code that runs in the same scope or descendant scopes. |
but only if the path to the external program is evaluated from an expression, which is needed because of the implicit yield; an explicit yield operator would have been a much cleaner concept
&/ CMD /C START '"HELLO WORLD"' PAUSE |
The responsibility to parse the command line is on the called tool. The standard command shell does not even have a concept of an argument list, it expects a dictionary of named options, so the standard procedure to split into an argument list does not apply and there is no shame about that. Named options are better than an argument list unless you really need to pass a homogeneous list of items. UNIX tools simulate parsing option dictionaries using a system‑imposed intermediate step of argument lists—which is a legacy feature and an inexhaustible source of funny misunderstandings and unexpected behaviour. |
Oh no, its not that dire, I can keep using your module any time. I'm just waiting for the proper solution, one day. I have hope still. |
Well, any program can parse its single commandline string whatever way it wants, it is what it is. The shame is not having the feature like
I'll keep using https://github.com/mklement0/Native . No worries ! |
Please see #18660 (comment) for the continuation of this discussion. |
Motivations
Very long history and fears of inevitable breaking changes
#1995
#7388
#1908
#13068
#12491
#12975
#10722
#10741
#9006
#3996
#13393
#13579
#13640 (comment) PSNativePSPathResolution
#10509 (comment)
#9343
PowerShell/PowerShell-RFC#90
PowerShell/PowerShell-RFC#88
It seems to be not all
Proposal
Introduce new PSNativeCmdlet class as an successor of the PSCmdlet class which will work as a wrapper for native apps.
Capabilities
All traditional cmdlet capabilities:
Additionally:
Benefits
The main thing is that this class can be integrated into the Engine subsystems selectively and gradually without total breaking changes.
And all subsystems themselves can be enhanced gradually.
All existing Engine possibilities remain. Wrappers can overlap its or bypass to its.
The class itself can also improve gradually.
We can add flags which subsystems a particular wrapper can support. For example say that only Completions. Or run it like a regular cmdlet. Or run it as a normal external command, but use the output parser.
We could add new option in Start-Process so that it uses capabilities of the new class too.
It is compiled and works fast. (We could think about script support too)
Easy adoption
Easy adoption for #1995 - We can implement @mklement0's Invoke-NativeShell internally in PSNativeCmdlet. Users can load a module with wrappers and get new behaviors or unload the module and get previous behavior. Or we could add flags in PSNativeCmdlet which turn on/off capabilities on the fly for the specific wrapper.
Git is a good example of gradual adoption. We could implement git wrapper to address argument parsing issues and it will work with posh-git module. Later we could enhance the wrapper with posh-git capabilities in some way.
Based on telemetry we will able to decide when to make the new behavior as default for all.
We can collect wrappers for the most popular commands in a separate standard module and a separate PowerShell project repository. The community will be able to quickly add wrappers and improve them. Other vendors and communities may publish modules with their wrappers for their applications.
Additional thoughts
This approach could address questions in @JamesWTruher's blog posts about how PowerShell can take better advantage of native executables
https://devblogs.microsoft.com/powershell/native-commands-in-powershell-a-new-approach/
https://devblogs.microsoft.com/powershell/native-commands-in-powershell-a-new-approach-part-2/
Since all native applications are built without a single parameter schema, these wrappers seem to be the only universal solution.
For Windows OS MSFT could make https://github.com/dotnet/command-line-api standard for all OS utilities. As result, PowerShell could benefits from this automatically loading meta data from managed dll-s.
The text was updated successfully, but these errors were encountered: