Skip to content

Conversation

@wchristian
Copy link

run() here calls IPC::S::S' runx(), which has two paths. On non-windows systems it calls system { $command } $command, @args which works just fine even with complex stuff in the args. On Windows however it calls _spawn_or_die($command, "$command @args"), which smashes all the arguments together separated by spaces without any additional quoting. This results in a garbage process call which mysql consequently rewards with RTFM.

This fix quotes the arguments on windows, and takes care to leave the executable name separate to allow proper naming of the created process object.

fixes #262

run() here calls IPC::S::S' runx(), which has two paths. On non-windows
systems it calls `system { $command } $command, @args` which works just
fine even with complex stuff in the args. On Windows however it calls
`_spawn_or_die($command, "$command @Args")`, which smashes all the
arguments together separated by spaces without *any* additional quoting.
This results in a garbage process call which mysql consequently rewards
with RTFM.

This fix quotes the arguments on windows, and takes care to leave the
executable name separate to allow proper naming of the created process
object.

fixes sqitchers#262
@theory
Copy link
Collaborator

theory commented Dec 20, 2018

I'm torn about this. I intentionally didn't make this change because I was optimistic that pjf/ipc-system-simple#29 would get merged and released. Perhaps too optimistic. :-( Perhaps we should try to figure proof it to only quote the words if the version is <= 1.25.

@wchristian
Copy link
Author

I have absolutely no preference for this at the moment. sqitch is simply broken on windows. Unusuable. No good. Any action is better than no action.

@theory
Copy link
Collaborator

theory commented Dec 23, 2018

Okay, done in 0148359.

@theory theory closed this Dec 23, 2018
@wchristian
Copy link
Author

Looks excellent! 👍

@wchristian wchristian deleted the win32_quoting_fix branch December 23, 2018 17:42
@theory
Copy link
Collaborator

theory commented Jan 15, 2019

FYI, 0148359 had a bug. I added windows testing and found it. Fixed in c697c9b.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants