-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move back to the main nightly channel #723
Move back to the main nightly channel #723
Conversation
Now that the upstream wasm32-wasi target has been fixed, we should be good to go!
We don't know what version of the wasi api it's compiled against, so let's avoid it.
I believe the one failure is #721, but otherwise should be good to go! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just got one question before I can approve this PR.
let stat = match wasi_unstable::fd_prestat_get(i) { | ||
Ok(s) => s, | ||
Err(_) => break, | ||
}; | ||
if stat.pr_type != wasi::PREOPENTYPE_DIR { | ||
continue; | ||
} | ||
let mut dst = Vec::with_capacity(stat.u.dir.pr_name_len); | ||
if wasi::fd_prestat_dir_name(i, dst.as_mut_ptr(), dst.capacity()).is_err() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is it OK to mix wasi_unstable
and wasi
for the syscalls here? I mean, first, we call wasi_unstable::fd_prestat_get
just to later on call wasi::fd_prestat_dir_name
. It might be OK, but definitely looks confusing!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops this was a mistake! I'll switch this as well to match the above
} | ||
dst.set_len(stat.u.dir.pr_name_len); | ||
if dst == path.as_bytes() { | ||
return Ok(wasi::path_open(i, 0, ".", wasi::OFLAGS_DIRECTORY, 0, 0, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and here as well, we call wasi::path_open
instead of wasi_unstable::path_open
.
Updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we get the green light from the CI, I reckon we're good to go with this one!
Looks like we're running into a slew of bugs related to duplicate symbol names in LLVM IR, which I've posted a fix for at rust-lang/rust#67363. I'll try to think of a way to work around that in the meantime. |
Well, that's what the tests and CIs are for, right? ;-) |
Ok I'll close this for now while the rustc PR bakes. |
Now that the upstream wasm32-wasi target has been fixed, we should be
good to go!