Skip to content

Commit cc69f49

Browse files
authored
FS Journal: Handle renaming OPFS files via delete + create (instead of file.move()) (#64)
## Rationale With this commit, objects pulled from a Git repo are correctly stored in OPFS. Before, each object was correctly stored in MEMFS, but in OPFS it was reflected as a 0 bytes file. ## Technical details This commit ditches the OPFS file.move() method for renaming files. Why? Each object pulled from a Git repository is first buffered in a file called ".tmp" and then renamed to its final name. However, the WRITE operation does not store the written bytes, only the path. By the time the filesystem journal is flushed, we cannot assume that the "rename from" path still contains the same bytes as it did when the WRITE operation was executed. Therefore, it's safer to delete the old file and create a new one. It is still possible that the new file was already deleted or renamed to another location. That's fine. A later stage of replaying the journal will take care of that. Ideally, PHP.wasm would not use journaling at all, but a native WASMFS layer for handling OPFS. See #1878 for more details. ## Testing instructions Unfortunately, we don't have a unit test suite for the MEMFS<->OPFS journaling. Here's a manual test you can perform: Create a new OPFS site and run this in devtools: ```php playground.run({ code: `<?php file_put_contents('/wordpress/test.txt', 'abc'); rename('/wordpress/test.txt', '/wordpress/test2.txt'); var_dump(file_get_contents('/wordpress/test2.txt')); ?>`}).then(t => console.log(t.text), console.log) ``` Now refresh the page and confirm the test2.txt file is not empty: ```ts await playground.readFileAsText("/wordpress/test2.txt"); ```
1 parent 27c1c39 commit cc69f49

File tree

1 file changed

+36
-4
lines changed

1 file changed

+36
-4
lines changed

packages/php-wasm/web/src/lib/directory-handle-mount.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
PHP,
66
__private__dont__use,
77
} from '@php-wasm/universal';
8-
import { Semaphore, joinPaths } from '@php-wasm/util';
8+
import { Semaphore, basename, joinPaths } from '@php-wasm/util';
99
import { logger } from '@php-wasm/logger';
1010
import { FilesystemOperation, journalFSEvents } from '@php-wasm/fs-journal';
1111
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -370,7 +370,6 @@ class OpfsRewriter {
370370
this.opfs,
371371
opfsTargetPath
372372
);
373-
const targetName = getFilename(opfsTargetPath);
374373

375374
if (entry.nodeType === 'directory') {
376375
const opfsDir = await opfsTargetParent.getDirectoryHandle(
@@ -391,8 +390,41 @@ class OpfsRewriter {
391390
recursive: true,
392391
});
393392
} else {
394-
const file = await opfsParent.getFileHandle(name);
395-
file.move(opfsTargetParent, targetName);
393+
/**
394+
* Delete the old file and creating a new one.
395+
*
396+
* We cannot use the OPFS move() method here. Imagine pulling from
397+
* a Git repository – each pulled object is first buffered in a
398+
* file called ".tmp" and then renamed to its final name. However,
399+
* the WRITE operation does not store the written bytes, only the
400+
* path.
401+
*
402+
* By the time the filesystem journal is flushed, we cannot
403+
* assume that the "rename from" path still contains the same bytes
404+
* as it did when the WRITE operation was executed. Therefore, it's
405+
* safer to delete the old file and create a new one.
406+
*
407+
* It is still possible that the new file was already deleted
408+
* or renamed to another location. That's fine. A later stage
409+
* of replaying the journal will take care of that.
410+
*
411+
* Ideally, PHP.wasm would not use journaling at all, but
412+
* a native WASMFS layer for handling OPFS.
413+
*
414+
* See https://github.com/WordPress/wordpress-playground/pull/1878
415+
* for more details.
416+
*/
417+
try {
418+
await opfsParent.removeEntry(name);
419+
} catch (e) {
420+
// If the directory already doesn't exist, it's fine
421+
}
422+
await overwriteOpfsFile(
423+
opfsTargetParent,
424+
basename(opfsTargetPath),
425+
this.php[__private__dont__use].FS,
426+
entry.toPath
427+
);
396428
}
397429
}
398430
} catch (e) {

0 commit comments

Comments
 (0)