fix `build.rs`: Use env `RUSTC` if present

bytecodealliance / target-lexicon

Target "triple" support

Apache License 2.0

47 stars 42 forks source link

fix `build.rs`: Use env `RUSTC` if present #89

Closed NobodyXu closed 1 year ago

NobodyXu commented 1 year ago

Use env::var_os to obtain OUT_DIR since it could contains non utf-8 characters.
Improve write_host_rs readability using raw string.
Flush the out: File in write_host_rs before returning.

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

NobodyXu commented 1 year ago

I'm curious; have you ever seen a build environment with a non-Unicode directory name, or is var_os here just theoretical correctness?

Well, no, I haven't seen one, but given that it is possible to do that on Linux and the filesystem can accept any zero-terminated [u8], I think it's only time until some users try to build with non-utf8 characters in their directory name.

Since fixing this only involves one-line change to env::var_os, I think it won't hurt to fix this issue.

sunfishcode commented 1 year ago

Indeed, it doesn't look like it will hurt in this instance. But similar things do hurt in several projects I'm involved in, so I'm always curious who actually needs non-Unicode paths and why, and whether all the complexity that we pass on to everyone for their benefit is worthwhile.

NobodyXu commented 1 year ago

But similar things do hurt in several projects I'm involved in, so I'm always curious who actually needs non-Unicode paths and why, and whether all the complexity that we pass on to everyone for their benefit is worthwhile.

Can you provide me with a few example of these cases please? I guess it's because having to use PathBuf instead of String adds complexity and forces people to learn yet another string-type, plus it's not compatible with String/str and all the other custom string types like CompactString.