Skip to content

Fix a cross-device mv error by using a tmp directory inside document root#886

Merged
adamziel merged 2 commits into
trunkfrom
fix-cross-device-mv-error
Dec 20, 2023
Merged

Fix a cross-device mv error by using a tmp directory inside document root#886
adamziel merged 2 commits into
trunkfrom
fix-cross-device-mv-error

Conversation

@adamziel
Copy link
Copy Markdown
Collaborator

@adamziel adamziel commented Dec 19, 2023

What is this PR doing?

Solves the error discovered by @sejas caused by calling php.mv(fromPath, toPath) when fromPath is a mounted local directory and toPath is a MEMFS directory. Emscripten doesn't support such a scenario:

Proceeding without the Notification plugin. Could not install it in wp-admin. The original error was: Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
    at descriptor.value (/playground-tools/node_modules/@php-wasm/node/index.cjs:67481:17)

The solution proposed in this PR replaces a /tmp directory with a randomly-named temporary directory inside wp-content. /tmp doesn't necessarily point to a system temp directory and needs to be revisited anyway. Whether we should use a temporary directory inside wp-content is another matter, but that part may be revisited once the recursive cp feature is added.

Testing instructions

@adamziel adamziel self-assigned this Dec 19, 2023
@adamziel
Copy link
Copy Markdown
Collaborator Author

I just confirmed this fixes Blueprints in wp-now cc @sejas:

CleanShot 2023-12-20 at 00 51 50@2x

@adamziel adamziel merged commit f5082bf into trunk Dec 20, 2023
@adamziel adamziel deleted the fix-cross-device-mv-error branch December 20, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant