Conversation
adamziel
force-pushed
the
proxyfs-mmap
branch
from
ffdcd01 to
d2bdc67
Compare
Collaborator
|
@adamziel Nice! The tests passed! I saw that you explicitly ignored PHP versions below 8. Is that intentional? : const phpVersionsWithIntl: SupportedPHPVersion[] = ['8.0', '8.1', '8.2', '8.3', '8.4', '8.5']; |
Collaborator
Author
no, it's vibe coded 🤣 👀 |
The mmap implementation for PROXYFS now correctly handles file reading for older PHP versions (7.2-7.4). Key changes: 1. Get actual file size from the proxied filesystem using fstat() for non-.dat files, as older PHP versions may pass incorrect length values 2. Read file contents using a loop with subarray view into the heap, matching how MEMFS and the PR #3073 implementation work 3. Fix symbol usage bug - get symbols from both replica and sourceOfTruth instances separately to ensure correct property access when instances come from different module builds (ESM vs CJS) 4. Enable Intl extension tests for PHP 7.2, 7.3, and 7.4
Remove specific PHP versions that support the Intl extension with ICU from the tests.
The Intl extension supports both procedural functions and object-oriented classes. While we were testing numfmt_create() and similar functions, we weren't testing that classes like Collator work correctly. This matters because classes depend on ICU data being properly loaded, and a misconfiguration could break class instantiation even if functions work. The new test creates a Collator instance and verifies it can sort data according to locale rules.
The Intl extension provides both procedural functions (like numfmt_create) and object-oriented classes (like Collator). While both depend on ICU data being properly loaded via mmap, they exercise different code paths in the extension. Testing both ensures the entire Intl implementation works correctly across all supported PHP versions (8.5 through 7.2).
PHP 7.2 and 7.3 are no longer supported in the repository. Updated the Intl extension tests to only run for PHP 7.4 through 8.5, and updated code comments to reflect that PHP 7.4 is now the oldest supported version.
adamziel
force-pushed
the
proxyfs-mmap
branch
from
ab114df to
48d6d44
Compare
Rewrote documentation to be more specific and useful: - Explains exactly why ICU needs mmap (loads icudt74l.dat for Collator/NumberFormatter) - Clarifies what PHP 7.4's bug is (passes wrong stat buffer value instead of file size) - Explains why we check for .dat files (ICU files work correctly, other files don't) - Documents that ICU only maps from offset 0 - Explains the subarray view approach for reading - Notes that msync rarely runs since ICU only reads - Improves proxyFileSystem docs with concrete examples Removed vague phrases like "certain file types" and "older PHP versions" in favor of precise explanations of what's happening and why.
adamziel
commented
Jan 16, 2026
Refactor workaround for incorrect file length in PHP 7.4 mmap implementation.
Removed unused proxyFS checks in the file system.
Collaborator
Author
|
I've reviewed this, corrected a few problems, adapted your tests here @mho22, and they all pass 🎉 I'm tempted to just merge with co-author note. |
This was referenced Jan 16, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What it does
Adds
mmapsupport to PROXYFS so the Intl extension works when/internal/shared(which contains ICU data files) is accessed through a proxied filesystem. This PR is based on @mho22's excellent work in WordPress/wordpress-playground#3073.Rationale
PROXYFS proxies filesystem operations between PHP instances, enabling file sharing. The Intl extension depends on ICU, which uses
mmapto load its data file (icudt74l.dat). Withoutmmapsupport in PROXYFS, ICU initialization fails when accessing the data file through a proxied mount.This showed up when trying to use
CollatororNumberFormatterin a PHP instance where/internal/sharedwas mounted via PROXYFS—the classes would fail to instantiate even though the extension was loaded.Implementation
Added
ensureProxyFSHasMmapSupport()that patches PROXYFS at runtime:mmapimplementation:mallocreadoperationSpecial handling for PHP 7.4:
lengthvalues tommapfor non-.datfilesfstatmsyncimplementation:MAP_SHAREDMAP_PRIVATE(changes stay in memory only)The patch is applied automatically when calling
proxyFileSystem(), so existing code gets mmap support without changes.Testing instructions
Run the new test suite:
npm test -- proxyfs-mmap.spec.tsTests verify:
Collator,NumberFormatter) work through PROXYFSE2E tests verify Intl works in blueprints:
npm run test:e2e -- blueprints.spec.ts -g "Intl"