-
Notifications
You must be signed in to change notification settings - Fork 377
[ Intl ] Fix Intl feature inside Blueprints in Playground #3073
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
base: trunk
Are you sure you want to change the base?
Conversation
f156b6d to
8cb071f
Compare
8cb071f to
d446c00
Compare
|
I found the root cause. The above blueprint will run successfully if I comment the if (!isPrimary) {
const pathsToShareBetweenPhpInstances = [
'/tmp',
requestHandler.documentRoot,
'/internal/shared',
'/internal/symlinks',
];
const pathsToProxy = pathsToShareBetweenPhpInstances.filter(
(path) => !isPathToSharedFS(php, path)
);
// TODO: Document that this shift is a breaking change.
// Proxy the filesystem for all secondary PHP instances to
// the primary one.
proxyFileSystem(
await requestHandler.getPrimaryPhp(),
php,
pathsToProxy
);
}This means something is happening to the If I add these lines in the + if( path !== '/internal/shared' ){
// @ts-ignore
replica[__private__symbol].FS.mount(
// @ts-ignore
replica[__private__symbol].PROXYFS,
{
root: path,
// @ts-ignore
fs: sourceOfTruth[__private__symbol].FS,
},
path
);
+ }It runs correctly. I wonder if mounting a file used by PHP internals like the intl extension will corrupt it. |
|
@adamziel I created a new directory named I tagged you because I wanted to know if |
|
Maybe |
|
Hm, not mounting that file worries me. First, what corrupts that file? Could it affect other files and corrupt other data? Second, the system is wired for sharing files between php instances and I worry about the consequences of not sharing them. The reason we have these mounts is to share a set of files between all the php instances - they're created once in the primary env and then every other php instance sources them from that primary php's filesystem. If we stop mounting, what would happen to icu.dat in non-primary php instances or after the php env is rotated? |
|
@adamziel Here’s my current understanding of why we were still hitting the Intl issue and how it relates to filesystem sharing. Each PHP instance is initialized via Later on, when Even though I can’t say exactly what happens internally inside To fix this, my proposal is to isolate native extensions Each PHP instance would load its extensions from this directory during startup, and that directory would never be proxied or remounted. This keeps ICU and other native extensions stable and instance-safe. In contrast, What do you think ? |
Motivation for the change, related issues
Based on this issue.
Related to this function.
It looks like the ICU data is not found when running the below Blueprint. Even if the file is located in
/internal/sharedbefore running theplayground.run(...)inrun-php.ts.Implementation details
Intl classes should work when intl is enabledtest inplayground/website/playwright/e2e/blueprints.spec.tsBlueprint
{ "features": { "intl": true }, "steps": [ { "step": "runPHP", "code": "<?php \n\n$data = array(\n 'F' => 'Foo',\n 'Br' => 'Bar',\n 'Bz' => 'Bz',\n);\n\n$collator = new Collator('en_US');\n$collator->asort($data, Collator::SORT_STRING);\n\nthrow new Exception( json_encode($data, JSON_PRETTY_PRINT ) );" } ] }Should throw :
But throws :