libnative uses CreateProcessA on windows

[alexcrichton]

It should use CreateProcessW to properly encode running binaries with utf8 names.

Needs tests to be accompanied with the change.

cc #9822, I can mentor this

[Rufflewind]

Made an attempt at it (no tests yet). I did some manual testing on Windows and it looks like the current working directory and environment variables are being encoded correctly. However, the arguments are still showing garbage (encoded as UTF8 but with zeroes after every byte).

Edit 1: Found the source of the problem: make_command_line doesn't handle Unicode properly.

Edit 2: Changed to GetEnvironmentStrings (os::env_as_bytes) as well. Do you want this as a separate patch or combine with the above?

[alexcrichton]

Nice work! Feel free to lump it all together as 1 PR, but separate commits for separate changes would be nice.

[Rufflewind]

Thanks! Do you have any suggestions on how I should write the tests? Testing spawn and env_as_bytes requires coordinating at least two processes so I'm not sure how that would work in the testing framework.

[alexcrichton]

I'd recommend writing a run-pass test or two with some interesting file names. The test can exec itself to test other argument passing behavior.

[Rufflewind]

Here's a preliminary test that copies itself into a subdirectory with a non-ASCII name and then runs it as a child process. The parent process also adds an environment variable and an argument, both containing non-ASCII characters. The child process makes sure all the strings are intact.

It works fine when compiled and executed manually (on both Debian and Windows), but not with the test runner. On both Debian and Windows the test fails due to missing symbols. I'm looking into this but don't have an explanation/solution yet.

[alexcrichton]

Nice job, that looks great!

You're likely seeing problems due to being unable to find the dynamic library dependencies. You're overriding the dynamic loader path environment variable to nothing, so all the libraries cannot be found.

You can probably get around it by compiling statically, using // no-prefer-dynamic near the top of the file (like other test directives).

[alexcrichton]

It would also probably be best to use std::io::Process instead of native::io::Process directly. That way you can test both libgreen and libnative.

[Rufflewind]

You're likely seeing problems due to being unable to find the dynamic library dependencies. You're overriding the dynamic loader path environment variable to nothing, so all the libraries cannot be found.

That's what I originally thought too, so I made sure to append to the existing environment variables. However, the error still occurs. I'll use // no-prefer-dynamic and see if that helps.

[Rufflewind]

I think the reason why the library can't be found is because the test runner adds a relative path to the environment variables. But, in any case, using static linking works.

(Sorry for the spam; didn't realize "Issue" is a special word on GitHub.)