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

Add ability to initialize cwd from PWD when targeting WASI #5040

Merged
merged 2 commits into from
Aug 26, 2023

Conversation

d3lm
Copy link
Contributor

@d3lm d3lm commented Aug 16, 2023

When compiling to WASI the the current directory is set to / on boot. That's a bit unfortunate and it means that if you'd list files in . it would read the root directory. This PR allows to initialize the current working directory from the PWD environment variable. I have added a target check to only do this for WASI.

@fanninpm
Copy link
Contributor

cc @lastmjs

@youknowone
Copy link
Member

Thank you for contributing!
Because I am not good at WASI, I'd like to understand if this is an expected (either de jure or de facto) standard for WASI apps. Could you confirm if it is or not?

@d3lm
Copy link
Contributor Author

d3lm commented Aug 22, 2023

The way it's implemented is not a de facto solution and WASI doesn't have a notion of a cwd and any paths are always resolved via pre-opened directories. However, IMO it's reasonable to allow to set the cwd to something other than / and it wouldn't have any other implications other than . or .. etc. would be resolved properly to the cwd that gets set. Without this it's not possible to e.g. list files in a directory for both . and / because if the cwd is / then . resolves to / even if the user is running python from some sub-directory.

src/lib.rs Show resolved Hide resolved
@youknowone youknowone force-pushed the delm/feat/wasi/set-current-dir branch from aeb92fd to 3131d56 Compare August 26, 2023 17:31
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thank you so much! Please let us know if you find anything lack of WASI support.

@youknowone youknowone merged commit d056201 into RustPython:main Aug 26, 2023
10 of 11 checks passed
@d3lm d3lm deleted the delm/feat/wasi/set-current-dir branch August 28, 2023 07:59
@d3lm
Copy link
Contributor Author

d3lm commented Aug 28, 2023

Will do! Thanks a ton for getting this in, and also for all your work on this! This is really huge 🙏

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.

None yet

3 participants