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

feature request: Implement both path_remove_directory and path_unlink_file #49

Closed
HarikrishnanBalagopal opened this issue Nov 14, 2023 · 3 comments · Fixed by #50
Closed

Comments

@HarikrishnanBalagopal
Copy link
Contributor

Feature Request

We are implementing 2 of the syscalls that deal with removing files and empty directories.

path_remove_directory(path): number {
return -1;
}

browser_wasi_shim/src/fd.ts

Lines 108 to 110 in 7349f7d

path_unlink_file(path): number {
return -1;
}

https://wasix.org/docs/api-reference/wasi/path_unlink_file
https://wasix.org/docs/api-reference/wasi/path_remove_directory

Status

We have implemented both and it seems to work for empty file and empty directory.
The code is based on https://github.com/bjorn3/browser_wasi_shim/pull/42/files#diff-177a21b9f40c72ea679cf8658d14ed0d9702afcea694e92e0936f11ab182d2a3R331-R333
with some additional checks.

Challenges

  • Fails on a directory that is not empty. In Golang with https://pkg.go.dev/os#RemoveAll it uses a recursive algorithm with unlinkat to unlink the files and remove the empty directories. This goes into an infinite loop.
  • The WASI spec requires that we check file and directory permissions/rights before removing/unlinking but the rights base isn't stored anywhere? We couldn't find many examples of checking rights
    export const RIGHTS_PATH_UNLINK_FILE = 1 << 26;

    other than
    (fs_rights_base & BigInt(wasi.RIGHTS_FD_WRITE)) ==
    BigInt(wasi.RIGHTS_FD_WRITE)
@HarikrishnanBalagopal HarikrishnanBalagopal changed the title feature request: Implementing both path_remove_directory and path_unlink_file feature request: Implement both path_remove_directory and path_unlink_file Nov 14, 2023
@bjorn3
Copy link
Owner

bjorn3 commented Nov 14, 2023

The WASI spec requires that we check file and directory permissions/rights before removing/unlinking but the rights base isn't stored anywhere?

Correct. Currently we entirely ignore permissions except at the location you mentioned. #42 has an implementation of this, but I need to look further into how exactly wasi's rights system works to check if the implementation is correct.

Fails on a directory that is not empty. In Golang with https://pkg.go.dev/os#RemoveAll it uses a recursive algorithm with unlinkat to unlink the files and remove the empty directories. This goes into an infinite loop.

Weird.

@HarikrishnanBalagopal
Copy link
Contributor Author

@bjorn3
Copy link
Owner

bjorn3 commented Nov 15, 2023

I will take a look tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants