Skip to content

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

Merged
youknowone merged 2 commits intoRustPython:mainfrom
d3lm:delm/feat/wasi/set-current-dir
Aug 26, 2023
Merged

Add ability to initialize cwd from PWD when targeting WASI#5040
youknowone merged 2 commits intoRustPython:mainfrom
d3lm:delm/feat/wasi/set-current-dir

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

fanninpm commented Aug 17, 2023

cc @lastmjs

@youknowone
Copy link
Member

youknowone commented Aug 17, 2023

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.

youknowone
youknowone reviewed Aug 22, 2023
View reviewed changes
@youknowone youknowone force-pushed the delm/feat/wasi/set-current-dir branch from aeb92fd to 3131d56 Compare August 26, 2023 17:31
youknowone
youknowone approved these changes Aug 26, 2023
View reviewed changes
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
@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

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@d3lm @fanninpm @youknowone