Skip to content
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

Avoid adding empty directories in 'wp-content' and 'wordpress' modes #32

Open
danielbachhuber opened this issue May 12, 2023 · 5 comments
Labels
Bug Something isn't working wp-now

Comments

@danielbachhuber
Copy link
Member

If you run wp-now start in 'wp-content' mode, you will end up with some empty directories: wp-content/database, wp-content/plugins/sqlite-database-integration, and wp-content/db.php.

From WordPress/wordpress-playground#318:

Note: mount() still adds an empty directory (https://emscripten.org/docs/api_reference/Filesystem-API.html#FS.mount) but at least such empty directory is not caught by git status in the developer's project.

This seems like an upstream bug right now; not sure how we'll be able to resolve.

@danielbachhuber danielbachhuber added Bug Something isn't working wp-now labels May 12, 2023
@adamziel
Copy link
Collaborator

Sqlite plugin and db.php could perhaps be somehow handled by VFS, but I’d like the database file to stick around. It’s your data, you should have access to it.

@wojtekn
Copy link
Collaborator

wojtekn commented May 15, 2023

Sqlite plugin and db.php could perhaps be somehow handled by VFS, but I’d like the database file to stick around. It’s your data, you should have access to it.

I think that keeping it in the hidden directory by default will be a preferable default approach as that way, we will avoid making changes to local project files. As a user, when I clone the repository and start the local environment there, I would expect to have no (or minimal) changes done to my project, so git status still shows clean. I implemented it in WordPress/wordpress-playground#313

We could add the behavior you proposed as an option in the feature, so the user could decide to keep the SQLite database file with the project and track it in VCS, for example.

This ticket's goal is to explore the possibility of removing the side effect of FS.mount, which currently adds an empty directory for each mounted resource.

@fmfernandes
Copy link

I'm using Laravel Valet to develop locally and trying out wp-now as well.

On a standard installation of WordPress in a parked domain in Valet if I later run wp-now start in wp-content, the Valet site stops working, throwing a fatal:

Warning: require_once(/test/wp-content/db.php): failed to open stream: No such file or directory in /test/wp-includes/load.php on line 570

Fatal error: require_once(): Failed opening required '/test/wp-content/db.php' (include_path='.:/opt/homebrew/Cellar/[email protected]/7.4.33_2/share/[email protected]/pear') in /test/wp-includes/load.php on line 570

So, I have to manually delete the database and db.php directories created by wp-now for the Valet site to work again. While I'm just testing both tools, it would be good if there's a way to have both working side-by-side.

@danielbachhuber
Copy link
Member Author

One idea is to have wp-now delete db.php/ and database/ when the server stops. However, this doesn't let Valet and wp-now run completely side by side...

@adamziel
Copy link
Collaborator

adamziel commented May 17, 2023

I wouldn't rely too much on any exit hooks. There's a lot of ways a process may die and only one happy path. I'd rather find a way to not create these empty directories in the first place. How are they created again? Is it an Emscripten thing or BasePHP thing from this repo?

We have an mkdir in mount() – perhaps that is the source of the problem?

https://github.com/WordPress/wordpress-playground/blob/f5e1654c0e3b2ab1a420aaf1643d26ad48c12937/packages/php-wasm/node/src/lib/node-php.ts#L129-L132

If so, let's add an option like { createDirectory: false }.

@danielbachhuber danielbachhuber transferred this issue from WordPress/wordpress-playground May 31, 2023
@danielbachhuber danielbachhuber changed the title Local Environment: Avoid adding empty directories in 'wp-content' and 'wordpress' modes Avoid adding empty directories in 'wp-content' and 'wordpress' modes Jun 2, 2023
danielbachhuber added a commit that referenced this issue Jun 6, 2023
See #32
See
WordPress/wordpress-playground#503 (comment)

## What?

Adds a failing test case for
#32 and marks it as
a skipped test.

## Why?

It will make it easier to evaluate whether
WordPress/wordpress-playground#503 fixes the
issue.

## Testing Instructions

1. Change `test.skip` to `test.only`.
2. Run `nx test wp-now --watch`.
3. Verify the test fails on the second assertion.
4. Comment out the second assertion and hit save.
5. Verify the test fails on the third assertion.
6. Comment out the third assertion and hit save.
7. Verify the test fails on the fourth assertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working wp-now
Projects
None yet
Development

No branches or pull requests

4 participants