[FS] Make fstatfs actually work #23381
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this change, `fstatfs` calls `statfs64` with the `char*` path pointer argument set to zero. Generally byte zero of the memory seems to be null, so when `statfs64` converts this `char*` to a JS string it gets the empty string. Then it calls `FS.statfs` with the empty string as an argument. `FS.statfs` calls `lookupPath` with an empty string argument which returns a `null` node, so `fstatfs` returns the default values for when there is no statfs handler in the files system. This fixes the problem and adds testing for statfs/fstatfs on nodefs and rawfs. We have to add separate `FS.statfsNode` and `FS.statfsStream` handlers because in some cases `stream.node` is null and in some cases `stream.path is useless (like for stdout). Also, in noderawfs we need to give paths for the standard streams because there is unfortunately no `fs.fstatfsSync` so we can only access information from the host via a path. This means the noderawfs test won't be portable to windows.
sbc100
reviewed
Jan 13, 2025
sbc100
approved these changes
Jan 13, 2025
bgrgicak
added a commit
to WordPress/wordpress-playground
that referenced
this pull request
May 7, 2025
## Motivation for the change, related issues [Emscripten merged a fix for `statfs`](emscripten-core/emscripten#23381) in `4.0.1`, so this PR updates Emscripten to allow us to run `statfs` in PHP functions like `disk_total_space`. ## Implementation details This PR updates the version of Emscripten to `4.0.5`, rebuilds all PHP-wasm libraries, and recompiles PHP-wasm. While rebuilding PHP-wasm libraries there was a bug in the `libcurl` build script because it expected `libopenssl` to be located in `libopenssl/asyncify/dist`, but in reality `libopenssl` was located in `libopenssl/asyncify/dist/1.1.0h` (similarly for JSPI). We have two versions of `libopenssl` because it [was requires for PHP 7.0](#2038). This PR drops `libopenssl 1.1.0h` support because [Playground doesn't support PHP 7.0 anymore.](Automattic/wordpress-playground-private#74) ## Testing Instructions (or ideally a Blueprint) - CI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this change,
fstatfscallsstatfs64with thechar*path pointer argument set to zero. Generally byte zero of the memory seems to be null, so whenstatfs64converts thischar*to a JS string it gets the empty string. Then it callsFS.statfswith the empty string as an argument.FS.statfscallslookupPathwith an empty string argument which returns anullnode, sofstatfsreturns the default values for when there is no statfs handler in the files system.This fixes the problem and adds testing for statfs/fstatfs on nodefs and rawfs. We have to add separate
FS.statfsNodeandFS.statfsStreamhandlers because in some casesstream.nodeis null and in some casesstream.pathis useless (like for stdout). Also, in noderawfs we need to give paths for the standard streams because there is unfortunately nofs.fstatfsSyncso we can only access information from the host via a path. This means the noderawfs test won't be portable to windows.