Avoid using display on paths.

[Alexhuszagh]

Adds a helper trait ToUtf8 which converts the path to UTF-8 if possible, and if not, creates a pretty debugging representation of the path. We can then change display() to to_utf8()? and be completely correct in all cases.

On POSIX systems, this doesn't matter since paths must be UTF-8 anyway, but on Windows some paths can be WTF-8 (see rust-lang/rust#12056). Either way, this can avoid creating a copy in cases, is always correct, and is more idiomatic about what we're doing. We might not be able to handle non-UTF-8 paths currently (like ISO-8859-1 paths, which are historically very common).

So, this doesn't change ergonomics: the resulting code is as compact and correct. It also requires less copies in most cases, which should be a good thing. But most importantly, if we're mounting data we can't silently fail or produce incorrect data. If something was lossily generated, we probably shouldn't try to mount with a replacement character, and also print more information than there was just an invalid Unicode character.

[Alexhuszagh]

I'll be adding the other as_posix helper in this PR, but a quick explanation on a Linux system:

Say I create a file with an ISO-8859-1 filename, ensuring this is done in Python:

>>> with open("Überraschung".encode("ISO-8859-15"), "w") as f:
...     f.write("Hello")
... 
5
>>> import os; os.listdir(os.getcwd())
['\udcdcberraschung']

We can confirm this is correct in Bash by checking the output in UTF-8 (only works if the terminal emulator also expects UTF-8 encoded output, which it should):

$ LANG=en_us.UTF-8 ls
''$'\334''berraschung

So now I generate my Rust code:

use std::{fs, io, env};

pub fn main() -> io::Result<()> {
    let cwd = env::current_dir()?;
    for entry in fs::read_dir(cwd)? {
        let path = entry?.path();
        println!("debug={path:?}");
        println!("display={}", path.display());
    }

    Ok(())
}

And this outputs:

./main 
debug="/home/ahuszagh/Desktop/tmp/\xDCberraschung"
display=/home/ahuszagh/Desktop/tmp/�berraschung

Or, our output isn't exactly correct: we shouldn't be using display either for error messages or mounting. At least in the debug output, we get the \xDC escape character printed, so we should be able to guess our encoding that our file we failed to mount was.

[Emilgardis]

does it make sense to add Path::display to clippy lint disallowed_methods

[Alexhuszagh]

I have no direct testing, but considering as_os_str would be the encoding of the host locale, and our container should probably always be in UTF-8 I believe, we might also need to look over the code in docker_cwd. Let me test a bit and convert this to a draft.

Update: that would require us having a non-Unicode locale on the host (extremely unlikely) and change the locale on the image to a non-Unicode one as well. Both are very unlikely, especially for new development, so let's just formalize UTF-8 assumptions and fix them if the need ever occurs.

[Alexhuszagh]

does it make sense to add Path::display to clippy lint disallowed_methods

Yes I think so. It is incorrect for our uses, even if it's a good method for more general cases.

[Alexhuszagh]

bors r=Emilgardis

[Alexhuszagh]

Just an additional comment in case this ever needs to be discussed again, as unlikely as that is: we need both the host and the container locale to be the same, I believe, since we need to provide bind mount mappings from the host to the container that are identical. Code like this (I believe) cannot exist if the path is not valid Unicode, since our container expects this to be a path that can be represented in UTF-8:

cross/src/docker/shared.rs

Lines 173 to 186 in 2937032

	pub(crate) fn mount(
	docker: &mut Command,
	val: &Path,
	prefix: &str,
	verbose: bool,
	) -> Result<PathBuf> {
	let host_path = file::canonicalize(val)?;
	let mount_path = canonicalize_mount_path(&host_path, verbose)?;
	docker.args(&[
	"-v",
	&format!("{}:{prefix}{}", host_path.display(), mount_path.display()),
	]);
	Ok(mount_path)
	}

However, nothing here stops you from providing non-UTF-8 filenames or directories as part of your crate data or external mounts to cross, just they cannot be the mount points. For example, if I have /path/to/dir/{{ISO-8859-15}}, I can provide a mount to /path/to/dir, and this should work fine, allowing me to access my files with non-UTF-8 filenames without issue (common if you need to process data extracted from a legacy source onto a modern system).

If any of these assumptions are wrong, this PR has made it easier to patch assumptions about UTF-8 paths in the future.

[Alexhuszagh]

bors r-

This isn't a bug, but it does lead to / being duplicated for the path immediately after root, so /foo/bar becomes //foo/bar. I'll add tests too for this.

[bors]

Canceled.

[Alexhuszagh]

bors r=Emilgardis

[bors]

Build succeeded:

• conclusion