Fix is_absolute on WASI

[RReverser]

WASI does not match cfg(unix), but its paths are Unix-like (/some/path) and don't have Windows-like prefixes.

Without this change, is_absolute for any paths, including /some/path, was returning falseon a WASI target, which is obviously not true and undesirable.

[rust-highfive]

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

[dtolnay]

Thanks!

[dtolnay]

This sounds reasonable but I don't know anything about WASI, so I'll poke the last few people who touched WASI filesystem stuff for another set of eyes: @ibmibmibm @sunfishcode. Please shout if you notice anything wrong with adopting a unix-equivalent definition of what constitutes an absolute path for WASI.

@bors r+

[bors]

📌 Commit 494d6e5 has been approved by dtolnay

[CryZe]

I believe to some degree that's intentional. There's various documentation stating there's no absolute paths on WASI.

Such as here: https://docs.rs/wasi-common/0.18.0/wasi_common/fs/struct.Dir.html

Unlike std::fs, this API has no canonicalize, because absolute paths don't interoperate well with the capability-oriented security model.

and here: https://docs.rs/wasi-common/0.18.0/wasi_common/fs/index.html

This combined with WASI's lack of absolute paths provides a natural capability-oriented interface.

and here: WebAssembly/wasi-libc#81 (comment)

Yeah, WASI doesn't have absolute paths, so the standard realpath doesn't fit.

[dtolnay]

Thank you!

What do you make of that, @RReverser?

@bors r-

[RReverser]

That's an interesting view. I've rather seen it as "WASI doesn't have relative paths" (because there is no concept of current working directory) not "WASI doesn't have absolute paths" (because those are the only kind of paths that WASI path APIs do accept).

In fact, being able to distinguish between those two in a cross-platform manner is exactly the reason I wanted this function to work.

As a practical example, Rust port of coreutils has its own userland implementation of canonicalize / realpath that uses this function https://github.com/uutils/uucore/blob/6b7e1cd29812aa30e28b720fce0e52dcf25f9a74/src/lib/features/fs.rs#L103-L109 in order to determine whether path should be normalised as-is or first prepended with the current working directory.

Since is_absolute unconditionally returns false, such function would always try to prepend the current working directory by getting std::env::current_dir, and reasonably crash because, as mentioned above, this concept doesn't exist on WASI.

However, if is_absolute would return true for absolute paths, it would be able to normalise such paths and continue execution.

Personally, I think that even though WASI doesn't have absolute paths in the traditional sense (not every path level exists, you can't go between them etc.), it would be still useful to distinguish "/some/path" mount points as absolute, while any other as relative, so that code that can deal with the former would continue work on WASI just like it does on any other system.

[sunfishcode]

@RReverser I agree. The docs are admittedly ambiguous on this point, WASI functions interpret a path starting with / as an absolute path; they just always produce io::ErrorKind::PermissionDenied errors when given such paths.

[RReverser]

they just always produce io::ErrorKind::PermissionDenied errors when given such paths

I assume you mean, unless such path can be matched via preopened dirs, correct?

[sunfishcode]

they just always produce io::ErrorKind::PermissionDenied errors when given such paths

I assume you mean, unless such path can be matched via preopened dirs, correct?

That's a good point, and another reason this PR makes sense. The WASI "system calls" themselves do require relative paths, however in "userspace", library code maps absolute paths into handle + relative path. Above that library abstraction layer, WASI does support absolute paths in a sense, so it makes sense to have Path::is_absolute be able to reflect that.

[RReverser]

@sunfishcode Sounds like we're on the same page, thanks for the confirmation.

[dtolnay]

@bors r+

[bors]

📌 Commit 494d6e5 has been approved by dtolnay