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

Mount current directory to /home and Set CWD to /home when running WASI #4730

Merged
merged 25 commits into from
May 31, 2024

Conversation

maminrayej
Copy link
Contributor

@maminrayej maminrayej commented May 22, 2024

Resolves #4725
Resolves #4724
Resolves #4753

@maminrayej maminrayej requested a review from syrusakbary as a code owner May 22, 2024 01:13
@theduke
Copy link
Contributor

theduke commented May 22, 2024

Note: this would be a (small) breaking behaviour change.

It shouldn't have a big effect, but it might break users that rely on the path in some logic.

Are we okay with that?
Have we thought through the consequences?
Should this be an opt-in config setting instead?

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I saw that we went into a different framework for testing that wasix-fyi, and I think this has some tradeoffs.

That means, that we should have two steps:

  1. Build (https://github.com/wasmerio/wasmer/blob/main/tests/wasi-fyi/build.sh)
  2. Test (in the similar fashion that wasi-fyi, https://github.com/wasmerio/wasmer/blob/main/tests/wasi-fyi/wasm-test.sh) -> Please see how the mapped dirs (via $input_base.dir), args and so on are provided in a framework-like fashion). We should use the same strategy here

@maminrayej maminrayej enabled auto-merge May 31, 2024 11:27
@maminrayej maminrayej merged commit b05b3c0 into main May 31, 2024
62 checks passed
@maminrayej maminrayej deleted the cwd-to-home branch May 31, 2024 11:49
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.

Fail to get the file discriptor? Set CWD to /home when running WASI Fix wasmer run xyz --dir=.
3 participants