-
Notifications
You must be signed in to change notification settings - Fork 368
[CLI] Fix null and "latest" WP version resolution and improve unzip error message #2889
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
Conversation
|
Great work. Will remove the fix from #2886 |
|
@zaerl I think @brandonpayton should also add the correction around the Exception too then right ? |
Missing some context here. Do you mean the check performed here? We can move all the fixes in this PR. The |
|
Sorry @zaerl, I meant the Exception from the } else {
- throw new Exception("Could not unzip file: " . $zip->getStatusString());
+ $fileSize = file_exists($zipPath) ? filesize($zipPath) : 'unknown';
+ throw new Exception("Could not unzip file. Error code: " . $res . ". File size: " . $fileSize . " bytes.");
} |
Yes, I think that merging the two PRs is the best choice here. |
…to wordpress-stop-treating-rc-as-latest
|
Unfortunately, some of the test runs have been running into Wasm-related errors. One was out of memory and another was an "unreachable" exception. I just merged the latest from trunk to retry, but I doubt any meaningful changes are there. We probably just have those issues that occur every so often. It would be good to look into. |
|
Thank you, @zaerl! I hadn't noticed that we were making overlapping fixes. I merged your changes into this PR. The changes look good, and all tests are passing. Let's merge. |
|
Actually, the description for this PR needs to be updated first so both it and the commit are accurate. |
|
We should probably also avoid caching a zip that is not a good zip file, but let's address that in a follow-up PR. |
Motivation for the change, related issues
Today, while there is an active WP 6.9 release candidate, Playground CLI is using the release candidate as "latest". As a user, I would expect "latest" to remain the latest production release. This PR makes that change.
In addition, when passing
nullas a CLIwpargument, Playground CLI attempts to downloadhttps://wordpress.org/wordpress-null.zip. In that case, WordPress.org outputs a "Release not found" generic text file that is cached as if it is a good release. This PR addresses that case as well.Implementation details
This PR adjusts the version selection logic to skip release candidates and treat the latest production release as "latest". It also updates the tests to include this scenario.
This PR addresses the null version issue by:
nullversions tolatestTesting Instructions (or ideally a Blueprint)