Skip to content
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

Hooks are not unescaping Quotes before executing #103

Open
RefinedSoftwareLLC opened this issue Apr 26, 2020 · 8 comments
Open

Hooks are not unescaping Quotes before executing #103

RefinedSoftwareLLC opened this issue Apr 26, 2020 · 8 comments
Labels
needs upstream help Impacted by any upstream issue, such as Rust os: windows OS specific issue that can be reproduced on Windows

Comments

@RefinedSoftwareLLC
Copy link

RefinedSoftwareLLC commented Apr 26, 2020

Environment Details

  • Operating System
    • Windows
  • Rust Version: 1.43.0
  • Rust Release Channel
    • Stable
  • Cargo Version: 1.43.0
  • rusty-hook Version: 0.11.1

Description

These fail but shouldn't:

pre-commit = "\"echo\""
pre-commit = """("echo")"""

Being unable to use quotes is preventing me from using pre-commit = """(git status --short | grep -q "^...folder/")""" in my logic to only compile when changes are made in my rust folder.

@RefinedSoftwareLLC
Copy link
Author

"\"echo\"" is ran as \"echo\" but should be ran as "echo".
"""("echo")""" is ran as (\"echo\") but should be ran as ("echo").

@RefinedSoftwareLLC RefinedSoftwareLLC changed the title Unable to use Quotes in hooks. Hooks are not unescaping Quotes before executing Apr 26, 2020
@calebcartwright
Copy link
Member

Thanks for the report @RefinedSoftwareLLC! Will try to take a deeper look at this one tomorrow

@RefinedSoftwareLLC
Copy link
Author

As a workaround, I got grep to work by removing all quotes and escape backslashes.
"""(grep -q "^...folder\.com/")""" for now has to be """(grep -q ^...folder.com/)"""

  • apparently I don't need quotes around the regexp.
  • but \. doesn't work so I have to use . which matches all characters instead of only a period.
  • using \\. in grep regexp just hangs the commit and it never stops running.

@calebcartwright
Copy link
Member

I can't reproduce this on Linux, but will check on my Windows machine in a bit.

Could you share two additional things whenever you get a chance?

  1. The output of rusty-hook --version (this will likely be 0.11.1 too but just want to check)
  2. The output of the git command (presumably commit) that you mentioned is failing with those hook scripts
$ git commit -m "foo"
Found configured hook: pre-commit
Running command: ("echo")

[master d9d19f2] foo
 1 file changed, 1 insertion(+), 1 deletion(-)

@RefinedSoftwareLLC
Copy link
Author

RefinedSoftwareLLC commented Apr 27, 2020

Yes, version 0.11.1

Found configured hook: pre-commit
Running command: ("echo")
'\"echo\"' is not recognized as an internal or external command,
operable program or batch file.

In cmd, if you directly type (\"echo\") you get the same error, but ("echo") works.

@calebcartwright
Copy link
Member

I'm guessing the escaping is due to rust-lang/rust#29494

@calebcartwright calebcartwright added os: windows OS specific issue that can be reproduced on Windows needs upstream help Impacted by any upstream issue, such as Rust labels May 3, 2020
@Mastermindaxe
Copy link
Contributor

As this issue is unlikely to be fixed upstream (judging by the time it has been open for) can we fix this by using a different way of executing these commands? Afaict there is some workaround by using CreateProcessW but I haven't looked too much into it. Is this something that's affecting a lot of users?

@calebcartwright
Copy link
Member

can we fix this by using a different way of executing these commands? Afaict there is some workaround by using CreateProcessW but I haven't looked too much into it. Is this something that's affecting a lot of users?

No, there's several viable, if mildly annoying, workarounds that are readily available, and we're definitely not going to dip down to CreateProcessW directly. Folks can use the workarounds for now (even if that means defining the script in a file that's then invoked from the hook script) and we'll update accordingly whenever things are patched upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs upstream help Impacted by any upstream issue, such as Rust os: windows OS specific issue that can be reproduced on Windows
Projects
None yet
Development

No branches or pull requests

3 participants