Skip to content
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

Compile to wasm32-unknown-unknown / Compile without terminal_size #346

Closed
Boshen opened this issue Feb 21, 2024 · 4 comments · Fixed by #349
Closed

Compile to wasm32-unknown-unknown / Compile without terminal_size #346

Boshen opened this issue Feb 21, 2024 · 4 comments · Fixed by #349

Comments

@Boshen
Copy link
Contributor

Boshen commented Feb 21, 2024

I'm currently stuck upgrading to v7 for wasm builds.

Background

After the terminal_size upgrade from #308, it brought in an extra dependency rustix, which in turn brought in errrno which doesn't support compile to wasm32-unknown-unknown.

error[E0583]: file not found for module `sys`
  --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/errno-0.3.8/src/lib.rs:26:1
   |
26 | mod sys;
   | ^^^^^^^^
   |
   = help: to create the module `sys`, create file "/home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/errno-0.3.8/src/sys.rs" or "/home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/errno-0.3.8/src/sys/mod.rs"
   = note: if there is a `mod sys` elsewhere in the crate already, import it with `use crate::...` instead

After looking at both crates terminal_size and errno, both crates claims they don't support wasm.

Possible Solution

Given that terminal_size is only called when terminal width is not provided, I think it's reasonable to put it behind a feature flag.

miette/src/handler.rs

Lines 374 to 381 in 75fea09

pub(crate) fn get_width(&self) -> usize {
self.width.unwrap_or_else(|| {
terminal_size::terminal_size()
.unwrap_or((terminal_size::Width(80), terminal_size::Height(0)))
.0
.0 as usize
})
}

@zkat
Copy link
Owner

zkat commented Feb 21, 2024

I'm ok putting it behind a feature flag but make it default as part of the fancy feature. Perhaps we can have a fancy-wasm that's specifically wasm-compatible?

@zkat
Copy link
Owner

zkat commented Feb 21, 2024

(that is, I'll take a patch to make these changes, and I can do a release after that for you)

@Boshen
Copy link
Contributor Author

Boshen commented Feb 22, 2024

I have a few projects that are targeting wasm and wasi.

I'd like to add a syscall feature to make it future compatible.

In this way we can put all syscall related functionalities into a module and guard it at the top level. This will also move all the #[cfg(miri)] / #[cfg(not(miri))] to a single place.

This will also resolve issues like #326 where I want to explicitly control all syscalls.

I'll draft a PR and see how it looks.

@Boshen
Copy link
Contributor Author

Boshen commented Feb 22, 2024

I drafted a PR in #349

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 a pull request may close this issue.

2 participants