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
Remove File.ResolveLinkTarget from IsWindowsApplication #16371
Remove File.ResolveLinkTarget from IsWindowsApplication #16371
Conversation
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I still think in leau of a public API for AppExecLinks we should delay the checking of the exe type to after the process has started. This results in Windows doing the resolution for you. |
Thanks @jborean93 for finding that workaround -- I learnt something new from you :) The biggest concern I have with that approach is that we need to call native APIs to start a process, and that may break the native arguments parsing/passing work we have done in 7.2 (the Also, there could be other problems due to the complexity introduced by that solution. @SteveL-MSFT started another conversation with the Windows Store team. We will see how that goes. |
I think the best solution is - we could get an public API we need with AppX module (if the module can do some operations with AppX it could tell whether an AppX console or GUI and so on). But WinRT was removed from .Net Runtime and now the module doesn't work in pwsh. So we need primarily new component to plug-in WinRT and than updated AppX module. |
🎉 Handy links: |
PR Summary
As discussed in #16295 we will not get public API for AppX from Windows team so we should remove our workaround for AppX. See #16295 (comment) for details.
As for the symbolic link, it's not well handled in
NativeCommandProcessor
on Windows today, and that's tracked by #16171.(Ideally we would need a public AppX API to detect whether the application is a console one or Windows GUI.)
PR Context
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.