Skip to content

Conversation

Stebalien
Copy link
Contributor

(draft because this is a bit of a hack and needs some discussion first)

At the moment, it's not possible to use &Path, &OsStr, etc. with rustix on WASIp2 without the nightly compiler (requires the wasip2 feature) as OsStrExt and OsStringExt require the wasip2 feature on WASIp2 so we can't use as_bytes() on OsStr and friends.

However, the recommendation on the upstream issue is to just use to_str as WASI paths are Unicode. This PR does exactly that.

@alexcrichton am I interpreting your comment correctly?

Alternatives:

  • Wait. I'd prefer not to do this as I don't see a quick path to stabilization here.
  • Work around this downstream. I'm submitting this to try to get tempfile working on the wasip2 platform on rust stable. I'm also happy to work around this downstream (in tempfile itself).
  • Use OsStr::as_encoded_bytes on wasi instead of OsStrExt::as_bytes. It will technically work but it also technically relies on an unstable representation (one that will likely never change on WASI, but still...).

This PR has two parts:

  1. The first commit DRYs up the code a bit to avoid having to implement too many platform-specific conditionals. I tried to keep this change minimal but I'm also happy to go ahead and DRY up the rest of the code (there's a fair amount of duplicate code in this file).
  2. The second commit special-cases conversions for OsString and OsStr, converting to utf8 strings where necessary.

Unfortunately, `OsStrExt` and `OsStringExt` require the wasip2 feature
on wasip2 so we can't use `as_bytes()` on `OsStr` and friends. However,
the recommendation on the upstream issue [1] is to just use `to_str` as
WASI paths are unicode.

[1]: rust-lang/rust#130323 (comment)
@alexcrichton
Copy link
Member

I would agree that my thoughts there are being interpreted correctly. I can't comment much as to this PR specifically, however, as I am not familiar with this part of rustix (so would leave that part of review to someone else)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants