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

mapdir option not compatible with Wasmtime or Windows paths #1487

Closed
RReverser opened this issue Jun 30, 2020 · 4 comments
Closed

mapdir option not compatible with Wasmtime or Windows paths #1487

RReverser opened this issue Jun 30, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@RReverser
Copy link

Describe the bug

mapdir option, added back in #451 (cc @MarkMcCaskey) is described as compatible with Wasmtime, but uses a single : as a separator instead of :: for left and right sides.

Also, it doesn't account for possible colons in paths themselves, making this option impossible to use with absolute paths on Windows.

echo "`wasmer -V` | `rustc -V` | `uname -m`"
> echo "$(wasmer -V) | $(rustc -V)"
wasmer 0.17.0 | rustc 1.43.1 (8d69840ab 2020-05-04)

Steps to reproduce

With arbitrary temp.wasm try the following on a Windows machine:

$ wasmer run --mapdir=/tmp:C:\Users\rreverser\Desktop\wasi\tmp temp.wasm

Expected behavior

Wasm program is ran with /tmp mapped to C:\Users\rreverser\Desktop\wasi\tmp on the host machine.

Actual behavior

Getting an error in the console

Error: Directory mappings must consist of two paths separate by a colon. Found /tmp:C:\Users\rreverser\Desktop\wasi\tmp
@RReverser RReverser added the bug Something isn't working label Jun 30, 2020
@syrusakbary
Copy link
Member

Also, it doesn't account for possible colons in paths themselves, making this option impossible to use with absolute paths on Windows.

That's a great point, we should probably allow separation by two colons :: @MarkMcCaskey

Note 1: it could be possible to also split the by the first encountered colon, to solve the issue in Windows.

Note 2: we are working on a new version of Wasmer and there might be a bit of delay on the resolution of the issue until we make it public. But we will work to make it happen! :)

@syrusakbary
Copy link
Member

We just fixed this on the refactor. Will close the issue once it's merged upstream.

@syrusakbary
Copy link
Member

We merged the refactor back into master a few weeks ago. Now the issue should be fixed in master.
Closing the issue!

@RReverser
Copy link
Author

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants