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.

Sign up for GitHub

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

Jump to bottom

Fix WASI rename issue: #2297 #2397

Merged
merged 10 commits into from
Jun 23, 2021
Merged

Fix WASI rename issue: #2297 #2397

merged 10 commits into from
Jun 23, 2021

Conversation

chenyukang
Copy link
Contributor

@chenyukang chenyukang commented Jun 6, 2021 •
edited
Loading

Description

Fix issue #2297.

The old code push full path to the parent directory, if we run wasmer as below:

wasmer  run --mapdir=/:. --mapdir=./:.  ./wasm/clang.wasm -- -cc1 -triple wasm32-unknown-wasi -isysroot /sys -internal-isystem /sys/include -emit-obj -o ./source/hello/
hello.o ./source/hello/hello.c

The out_path become to : ./source/hello/source/hello/hello.o, it will trigger an I/O error.

close #2297

@chenyukang chenyukang requested a review from MarkMcCaskey as a code owner June 6, 2021 11:44
@chenyukang chenyukang changed the title Fix rename issue: #2297 Fix WASI rename issue: #2297 Jun 6, 2021
@syrusakbary
Copy link
Member

Good fix! I'll wait for @MarkMcCaskey review as he has more insight on the WASI internals :)

@Hywan Hywan added bug Something isn't working 📦 lib-wasi About wasmer-wasi labels Jun 7, 2021
@chenyukang
Copy link
Contributor Author

chenyukang commented Jun 8, 2021 •
edited
Loading

Spend some time to figure how to add unit test for WASI fs.
Seems we don't have make wasitests now?
https://github.com/wasmerio/wasmer/tree/master/tests/wasi-wast/wasi/test_fs

MarkMcCaskey
MarkMcCaskey reviewed Jun 8, 2021
View reviewed changes
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Just a small tweak on the fix itself and a bit more of a process for updating WASI tests, happy to help out with the generation part of the WASI tests, just ping me!

lib/wasi/src/syscalls/mod.rs Outdated Show resolved Hide resolved
tests/wasi-wast/wasi/tests/path_rename.rs Show resolved Hide resolved
@syrusakbary syrusakbary mentioned this pull request Jun 9, 2021
Merged
@chenyukang chenyukang requested a review from MarkMcCaskey June 9, 2021 04:53
@syrusakbary
Copy link
Member

Can you update the wasi-tests subtree to latest master (with your fix added)?

@chenyukang
Squashed 'tests/wasi-wast/' changes from f575fff42..6035e0a8f
640813c 
6035e0a8f Merge pull request wasmerio#1 from chenyukang/add-path-rename-tests
6c42d0308 add more info in output
178691a41 add path_rename testcase

git-subtree-dir: tests/wasi-wast
git-subtree-split: 6035e0a8f9f6fa668d7618ab03efae9d34da820c
@chenyukang
Merge commit '640813c808389da9f8515eac11d0da4b5ccf8db7' into fix-rena…
c5c6e66 
…me-issue
@syrusakbary
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 23, 2021

Build succeeded:

@bors bors bot merged commit 6773109 into wasmerio:master Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@syrusakbary syrusakbary syrusakbary approved these changes

@MarkMcCaskey MarkMcCaskey Awaiting requested review from MarkMcCaskey

Assignees

@MarkMcCaskey MarkMcCaskey

Labels
bug Something isn't working 📦 lib-wasi About wasmer-wasi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

error: unable to rename temporary
4 participants
@chenyukang @syrusakbary @MarkMcCaskey @Hywan