From IRC (primarily notes for myself):

<hachque> hey epriestley
<hachque> will you accept diffs relating to T5831?
<phabot> T5831: Run all build commands through PowerShell on Windows - https://secure.phabricator.com/T5831
<hachque> i'm like 90% sure it's going to break anyone using "Run Command" with Windows
<hachque> and relying on cmd.exe functionality
<hachque> but we need to do this before Harbormaster is in Beta / widely used
<hachque> because cmd.exe command line parsing is unmanagable
<hachque> right now my thoughts are just to encode the command and run it as if it was a powershell command (instead of doing Start-Process)
<epriestley> I'm fine with breaking all that stuff, although maybe it should be an option or something -- what issues are you encountering with cmd.exe?
<hachque> which will allow people to run powershell scripts like
<hachque> .\MyScript.ps1 -Blah -Something
<hachque> instead of doing
<epriestley> The only real concrete stuff we've hit is that we have no way to escape newlines, and null bytes can't be read over pipes.
<hachque> C:\Windows\Microsoft\Powershell\v1.0\powershell.exe -ExecutionPolicy Bypass .\MyScript.ps1 -Blah -Something
<hachque> the problem i have with windows is that things like
<hachque> git pull http://..../ C:\Build\${build.id}
<hachque> don't work
<hachque> because the quotes are treated literally as part of the path
<hachque> and it makes a folder called C:\Build\'1234'
<hachque> so the only option there is to always use %C
<hachque> but then the input variables are unsafe
<hachque> and could have unintended effects
<hachque> (especially if they contain spaces or other characters that need escaping)
<hachque> with powershell at least you can do things like
<hachque> mkdir ("C:\Build\" + ${build.id})
<hachque> and that will have the intended effect
<hachque> since powershell is actually a proper shell language
<hachque> vs cmd.exe where this is impossible
<hachque> we can basically put a caption on the Command field that says
<hachque> "On UNIX, this is executed using the default shell for the SSH user. On Windows, this is executed under PowerShell."
<epriestley> It seems like we could get the "C:\Build\${build.id}" example right by trying a little harder.
<epriestley> Particularly, we probably apply Unix-style escaping rules to the string, since there's no mode flag?
<hachque> i don't think theres any escaping here where windows will behave properly
<hachque> because it doesn't inheritantly handle quotes in the middle of a parameter
<hachque> whereas bash seems to understand the single quotes and take them out
<hachque> windows just treats them literally
<hachque> so the only option would be "remove the quotes"
<hachque> which then means you get no handling at all
<hachque> and you can't always do C:\Program\ Files\
<hachque> to escape spaces
<hachque> because it depends on what the program is and how it parses the argument string
<hachque> powershell is at least *consistent*
<hachque> which is a big step up from cmd.exe
<hachque> the only thing for powershell is that it escapes with ` instead of \
<hachque> but other than that the escaping sematics are the same (except that instead of having to double escape \ you have to double escape ` obviously)
<jeremylee> Glad to see there is a PHabricator channel.
<epriestley> When I run this *on windows*:
<epriestley> execx('mkdir asdf%s', 'qwer');
<epriestley> It produces this command in cmd.exe:
<epriestley> mkdir asdf"qwer"
<epriestley> Which behaves correctly.
<epriestley> I think 90% of this issue is that we're running csprintf() (or some wrapper function) on a unix machine, which applies unix shell rules, and then sending it over the wire to a windows machine, where they're not valid.
<hachque> does it behave correctly when you try something like
<hachque> C:\Windows\System32\WindowsPowershell\v1.0\powershell.exe -ExecutionPolicy Bypass -Command { Write-Host "abcd${build.id}" }
<hachque> or something of that nature
<hachque> (i think it's -Command, but this is off the top of my head)
<hachque> well that would evaluate to
<hachque> C:\Windows\System32\WindowsPowershell\v1.0\powershell.exe -ExecutionPolicy Bypass -Command { Write-Host "abcd"2345"" }
<epriestley> If you remove the quotes it seems fine
<hachque> which is almost certainly going to cause problems
<epriestley> { Write-Host abcd${build.id} }
<jeremylee> I'm new to phabricator here. Just got my instance set up. I'm impressed with the interface. But I'm looking for documentation on diffusion.
<hachque> oh yeah, but assuming abcd has spaces or is some other text?
<hachque> i mean you can do Write-Host "abc sdf" + ${build.id}
<epriestley> I think "abcd"${build_id} works
<epriestley> I'm totally onboard with offering powershell as an option, but our behavior on cmd.exe is clearly wrong/broken right now.
<epriestley> I'm also generally fine with just breaking the existing stuff wholesale.
<epriestley> jeremylee: do you have any questions in particular? I assume you've already found this stuff?
<hachque> i think generally the cmd.exe environment isn't very valuable, because to do anything remotely reasonable (like scripting), you need to jump into powershell
<hachque> and when you run something like
<epriestley> https://secure.phabricator.com/book/phabricator/article/diffusion/
<epriestley> https://secure.phabricator.com/book/phabricator/article/diffusion_hosting/
<hachque> git clone http://...../ C:\blah\
<hachque> from powershell
<hachque> that works as expected and invokes commands
<jeremylee> yep. found those articles.
<hachque> (just as it would under say Bash
<jeremylee> epriestley:I was trying to find documentation on setting up mirrors.
<epriestley> jeremylee: there's no specific documentation on that I don't think, did you find the option in the UI?
<jeremylee> but i figured it might have been recently introduced and lacked the documentation.
<jeremylee> yes
<hachque> we can offer a toggle, but personally i think ditching cmd.exe will reduce brokeness and support issues around it
<epriestley> Realistically, I don't want to write/support a toggle or write escapeshellarg_like_we_are_a_windows_box(), so let's just switch to powershell for now if you want to go that way.
<epriestley> I just want to frame it as "cmd.exe isn't very good", not "cmd.exe is completley unusable for any use case", because most of the major broknenness right now is our fault.
<epriestley> and where appropriate maybe consider the possibility that shells might be selectable in the future
<epriestley> But that could also be in the form of a build step like "Run Command Using cmd.exe"
<hachque> yeah
<hachque> selectable shells gets a bit weird since the platform inheritantly changes what's available