search

swellaby / rusty-hook

git hook manager, geared toward Rust projects
MIT License
207 stars 11 forks source link

Hooks are not unescaping Quotes before executing #103

Open RefinedSoftwareLLC opened 4 years ago

RefinedSoftwareLLC commented 4 years ago

Environment Details

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 commented 4 years ago

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

calebcartwright commented 4 years ago

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

RefinedSoftwareLLC commented 4 years ago

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/)"""

calebcartwright commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I'm guessing the escaping is due to https://github.com/rust-lang/rust/issues/29494

Mastermindaxe commented 3 years ago

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 commented 3 years ago

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