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.

Sign up for GitHub

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

Jump to bottom

WordPress Major and Beta update workflow is currently failing #1868

Closed
brandonpayton opened this issue Oct 9, 2024 · 8 comments
Closed

WordPress Major and Beta update workflow is currently failing #1868

brandonpayton opened this issue Oct 9, 2024 · 8 comments
Labels
[Package][@wp-playground] WordPress Builds [Type] Bug An existing feature does not function as intended

Comments

@brandonpayton
Copy link
Member

Most recent failure:
https://github.com/WordPress/wordpress-playground/actions/runs/11239897494/job/31248007824#step:5:1660

PHP.request() is deprecated. Please use new PHPRequestHandler() instead.
217 | 	if (!(await isWordPressInstalled(php))) {
218 | 		await installWordPress(php);
219 | 	}
220 | 
221 | 	if (!(await isWordPressInstalled(php))) {
222 | 		throw new Error('WordPress installation has failed.');
              ^
error: WordPress installation has failed.
      at /home/runner/work/wordpress-playground/wordpress-playground/packages/playground/wordpress/src/boot.ts:222:9

Another interesting thing is that this appears to be for WP 6.6.1, and there is already a WP 6.6.2 in the release list from https://api.wordpress.org/core/version-check/1.7/?channel=beta.
@brandonpayton brandonpayton added [Package][@wp-playground] WordPress Builds [Type] Bug An existing feature does not function as intended labels Oct 9, 2024
@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

I get this error when I run node packages/playground/wordpress-builds/build/build.js --wp-version=latest-minus-1 --output-js=packages/playground/wordpress-builds/src/wordpress --output-assets=packages/playground/wordpress-builds/public

I'm now trying to understand how is packages/playground/wordpress-builds/build/build.js calling bootWordPress.

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

This might be caused by the API output
https://api.wordpress.org/core/version-check/1.7/?channel=beta

6.6.2 is included two times.

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

It's related to the output.

We expect this code to include minor increments (6.6, 6.5...) and in this case it has patches (6.6.2, 6.6.1...).

I will update the filter now and see if that fixes the issue.

@brandonpayton
Copy link
Member Author

I'm looking a bit with git bisect. This used to work, so there's a commit to be found somewhere.

@brandonpayton
Copy link
Member Author

bisect yielded 3f1e998:

spring:~/src/playground[:3f1e998|BISECT 2; 1 steps L|⚑41]  1 ↵% git bisect bad
3f1e9989bcf7ffa99e185bb67ae646d6bd88aef9 is the first bad commit            
commit 3f1e9989bcf7ffa99e185bb67ae646d6bd88aef9

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

This line is causing the issue.

The expected output is 1, but we get /Users/bero/Projects/wordpress-playground/packages/playground/wordpress/src/mu-plugins/auto_login.php1.

If I don't add the auto login file, it works.
The problem seems to be in the import and not the file itself, even an empty file produces this error.

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

The ?raw import doesn't work in Node

wordpress-playground/packages/playground/wordpress/src/index.ts

Line 7 in 3f1e998

import autoLoginMuPlugin from './mu-plugins/auto_login.php?raw';

@bgrgicak bgrgicak mentioned this issue Oct 9, 2024
Merged
@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024 •
edited
Loading

I have a PR that should fix this by inlining the auto-login code instead of importing it as raw text.

adamziel pushed a commit that referenced this issue Oct 9, 2024
@bgrgicak
[CLI] Fix isWordPressInstalled() in CLI by inlining the auto_login.…
a6fe2af 
…php in index.ts instead of using import ?raw (#1869)

Fixes [the failing WordPress Major and Beta update
workflow](#1868).
It crashed on import "auto_login.php?raw" so this PR removes the import
and inlines the php script in a JavaScript variable

## Motivation for the change, related issues

We've introduced a bug in
#1856 by adding [a
`raw` import to the `@wp-playground/wordpress`
package](https://github.com/WordPress/wordpress-playground/pull/1856/files#diff-8afd615c3847003caebf2b35102f3a2a107ad9647e095495ecc9302862802191R7).

Raw imports aren't native to JavaScript, they are processed on build,
using Vite in our case.
This is why raw imports work well in our Remote package, but not in the
CLI.

When Playground is started using Node, the [CLI tries getting the raw
text from
`auto_login.php`](https://github.com/WordPress/wordpress-playground/pull/1856/files#diff-8afd615c3847003caebf2b35102f3a2a107ad9647e095495ecc9302862802191R7).
Instead of getting the text, it gets the path to the imported file (e.g.
`/Users/me/Projects/wordpress-playground/packages/playground/wordpress/src/mu-plugins/auto_login.php`).
This path is written into the `1-auto-login.php` mu-plugin as a string.

When Playground tries to boot, it [checks if WordPress is
installed](https://github.com/WordPress/wordpress-playground/blob/ccadc7d0daedda0b59b571e12aade32cf3abaeab/packages/playground/wordpress/src/boot.ts#L243).
The expected result of that check is `1`, but because `1-auto-login.php`
contains the path as a string, and because WordPress uses PHP `include`
to load that file, the path gets written to stdout and mixed with the
script output. The outcome is that `result.text` is
`/Users/me/Projects/wordpress-playground/packages/playground/wordpress/src/mu-plugins/auto_login.php1`
instead of `1`.

## Implementation details

This PR moves the auto-login code from a PHP file to inline code.

The approach matches what we did with other `mu-plugins` that must be
loaded both in the CLI and on the web.

TODO:
- [Add tests to ensure the CLI
works](#1871)
- Consider adding instructions for setting up the CLI as a global
package

## Testing Instructions (or ideally a Blueprint)

- Run `bun packages/playground/wordpress-builds/build/build.js
--wp-version=latest-minus-1
--output-js=packages/playground/wordpress-builds/src/wordpress
--output-assets=packages/playground/wordpress-builds/public`
- Confirm it works
@github-project-automation github-project-automation bot moved this from Inbox to Done in Playground Board Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Package][@wp-playground] WordPress Builds [Type] Bug An existing feature does not function as intended
Projects
Playground Board
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@brandonpayton @bgrgicak