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

PHP: mount() should not create a directory when mounting a file #503

Open
adamziel opened this issue Jun 2, 2023 · 2 comments
Open

PHP: mount() should not create a directory when mounting a file #503

adamziel opened this issue Jun 2, 2023 · 2 comments
Labels
[Feature] PHP.wasm Good First Issue Good for newcomers [Type] Bug An existing feature does not function as intended

Comments

@adamziel
Copy link
Collaborator

adamziel commented Jun 2, 2023

The mount() method calls mkdirTree() internally to ensure the target path exists – otherwise Emscripten throws an error. The problem is, sometimes we want to mount a file and not a directory. In these cases, the mount() method will still create that empty dir:

mount(localPath: string | MountSettings, virtualFSPath: string) {
if (!this.fileExists(virtualFSPath)) {
this.mkdirTree(virtualFSPath);
}

Let's:

  • Add a check like !this.isDir(localPath)
  • Add a unit test in php-spec.ts to prevent regressions
@adamziel adamziel added [Type] Bug An existing feature does not function as intended Good First Issue Good for newcomers [Feature] PHP.wasm labels Jun 2, 2023
@adamziel adamziel changed the title mount() should not create a directory when mounting a file PHP: mount() should not create a directory when mounting a file Jun 2, 2023
@danielbachhuber
Copy link
Member

Will this help fix WordPress/playground-tools#32 ?

@adamziel
Copy link
Collaborator Author

adamziel commented Jun 2, 2023

@danielbachhuber I think so! Would be great to create a failing test in wp-now repo, fix it, and see what it does to the test

danielbachhuber added a commit to WordPress/playground-tools 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
[Feature] PHP.wasm Good First Issue Good for newcomers [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants