Skip to content

Do not error on native binary that handles @ to expand response file contents as args #12491

Closed

Description

Summary of the new feature/enhancement

This might be a stretch but this is another one of those unfortunate gotchas when a dev is trying to use PowerShell to invoke commands that have been documented. Here's the problem:

04-25 14:54:57 84> g++ timer.cpp @conanbuildinfo.args -o timer --std=c++11
ParserError:
Line |
   1 |  g++ timer.cpp @conanbuildinfo.args -o timer --std=c++11
     |                ~~~~~~~~~~~~~~~
     | The splatting operator '@' cannot be used to reference variables in an expression.
     | '@conanbuildinfo' can be used only as an argument to a command. To reference variables
     | in an expression use '$conanbuildinfo'.

[master ≡ +13 ~9 -0 !] /mnt/c/Users/hillr/GitHub/Conan-IO/training/consumer_gcc
04-25 14:55:28 85> g++ timer.cpp '@conanbuildinfo.args' -o timer --std=c++11

In this case, conanbuildinfo.args is a file, specifically a response file that contains a bunch of arguments to g++. It is not that uncommon for exes to use a response file specified with @response_filename.

Proposed technical implementation details (optional)

I'm not entirely sure what the solution is but maybe in this case before erroring when the variable is not found (conanbuildinfo in the example above), consider passing the arg as-is to the native binary assuming it will handle the @. This might be worth emitting a warning. Something like: Could not find splatted variable 'conanbuildinfo', passing argument as-is to the application. To avoid this warning, put single quotes around the argument.. Also, if strict mode is set, maybe this should go ahead and error?

Activity

  1. bpayette commented on Apr 26, 2020

    @bpayette
    Contributor

    Keith Hill (@rkeithhill) I assume quoting the argument is all that's needed to make it work - correct? We could update the error message to say that "blah, blah ... quote arguments starting with '@' blah blah ...". As far as "fixing" this, I just worry that, given how complex parameter binding already is, adding more special cases could ultimately make things worse, not better. How much of an issue do think this is?

  2. rkeithhill commented on Apr 26, 2020

    @rkeithhill
    CollaboratorAuthor

    I assume quoting the argument is all that's needed to make it work

    That is correct. It is a fair point to ask if the increased parameter binding complexity is worth it. As for the question of how much of an issue this is ... that is hard to say but I expect it is more of an issue now that new (Linux/macOS) users are kicking the tires on PowerShell. The problem is sort of a "first impression" issue. For most folks, their first impression is how well PowerShell does at executing commands from the console. The command line I listed above was from a training class on Conan. The first attempt in PowerShell was a failure - not a good first impression.

    That said, there are so many of these issues - ; as a statement sep, $ for variables () for expressions, quoting issues, etc. Maybe there should be a more holistic approach to all these native app arg issues? Yes, I know about the stop parsing operator but anybody kicking the tires on PowerShell will not. And there is nothing in the error message that gives the user any help or clue as to how to rectify the situation. That is not a good user experience. At the very least, if we detected certain types of parameter errors for a native app, we should offer some suggestions on how to rectify the situation - use --% or quote args, etc. Ideally, in an interactive situation, PowerShell would offer to re-execute the command in a dumb parsing mode. There are likely a number of other ways to address this to varying degrees. Most of them are probably better than what we have now.

  3. iSazonov commented on Apr 28, 2020

    @iSazonov
    Collaborator

    Most of them are probably better than what we have now.

    And all will be a huge breaking change. Do we really want this if --% was introduced many years ago?

  4. rkeithhill commented on Apr 29, 2020

    @rkeithhill
    CollaboratorAuthor

    Adding some "mitigation tips" to error messages wouldn't be breaking. Right now we leave new folks with nothing really to go on. Look at the original post's error message. That error message IN NO WAY helps the user in this case.

    I think we could be smarter about parameter errors when they are for native applications and provide some tips in the error message like using --% or show how to invoke a help topic on parameter issues when invoking native applications.

  5. iSazonov commented on Apr 29, 2020

    @iSazonov
    Collaborator

    Another thought is that if we say about interactive user session we could introduce "native command mode" - in the mode first execute input string "as-is" without parsing arguments and use "&" to force parsing.

  6. rkeithhill commented on Apr 29, 2020

    @rkeithhill
    CollaboratorAuthor

    introduce "native command mode"

    If we could do that in a non-breaking way that would be cool. That might also be able to address another issue where you'd like to invoke a native app and bypass any aliases or functions that happen to have the same name. I remember this issue was discussed quite a while back but nothing ever came of it. Well, except that certain aliases like ls and sort were removed on Linux/macOS.

  7. iSazonov commented on Apr 30, 2020

    @iSazonov
    Collaborator

    If we could do that in a non-breaking way that would be cool.

    It is a breaking change I think. I like it.

    PS >& <Enter>           # We could use '&' to switch mode
    PS &>                   # Now we are in native command mode
    PS &>& <enter>          # Switch back in PowerShell command mode
    PS >                    # Now we are in PowerShell command mode

    Perhaps James Truher (@JamesWTruher) Dongbo Wang (@daxian-dbw) and TylerLeonhardt could share thoughts.

  8. SteveL-MSFT commented on Jun 10, 2020

    @SteveL-MSFT
    Member

    For the splatting sigil, we could always treat that as literal for native commands since you can't splat to native commands. However, the other characters that match to pwsh syntax is problematic. One possible solution is to have a suggestion based on a non-zero exit code that could check if any of those characters were used and suggest quoting them or using `--%' (which makes that switch more discoverable as I'm sure most users aren't familiar with it)

  9. vexx32 commented on Jun 10, 2020

    @vexx32
    Collaborator

    Native commands can have arrays splatted to them, so we can't assume @ is always a literal:

    $params = @(
        '/c'
        'ping 1.1.1.1'
    )
    
    cmd @params

    Also, to provide such a suggestion... we should probably fix up the suggestions feature first, so that we don't end up increasing the necessary work for that in future (assuming that's still something we want to do sooner or later).

  10. SteveL-MSFT commented on Jun 10, 2020

    @SteveL-MSFT
    Member

    Rain Sallow (/u/ta11ow) (@vexx32) yeah, forgot about that was only thinking about hashtables. Agree we need to fix up the suggestion feature first for other reasons :)

  11. rkeithhill commented on Jun 10, 2020

    @rkeithhill
    CollaboratorAuthor

    One possible solution is to have a suggestion based on a non-zero exit code that could check if any of those characters were used and suggest quoting them or using `--%' (which makes that switch more discoverable as I'm sure most users aren't familiar with it)

    Me like. :-)

  12. lzybkr commented on Jun 11, 2020

    @lzybkr
    Contributor

    Food for thought - maybe it's possible to work like people expect most of the time.

    • In VariableOps.GetVariableValue - after the check for a failed lookup, defer the strict mode check when varAst.Splatted is true.
    • In this case, return an instance of a new type SplattedUndefinedVariable whose ToString() implementation renders the variable name with the @ before so the native command parameter binder "just works" on the value.
    • In PipelineOps.AddCommand - where splatting is expanded, if the value's type is SplattedUndefinedVariable and the command is not a native command, perform the strict mode check, and if that doesn't throw, replace the value with $null.
  13. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  14. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  15. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  16. microsoft-github-policy-service commented on Nov 23, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue-Enhancementthe issue is more of a feature request than a bugResolution-No ActivityIssue has had no activity for 6 months or moreWG-Languageparser, language semantics

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions