Plugin Directory: Mark releases as built when manually rebuilding ZIPs - #766
Plugin Directory: Mark releases as built when manually rebuilding ZIPs#766obenland wants to merge 5 commits into
Conversation
The `zips_built` release flag was only ever set by the importer, so rebuilding ZIPs via `bin/rebuild-zip.php` left release-confirmation plugins permanently showing "Release confirmed, waiting for processing". Mark the releases after a successful build, mirroring the importer. A commit with no modified files (the ZIPs on disk were already current) is now treated as a successful build rather than a failure, so the script can repair release metadata for plugins whose ZIPs were already rebuilt. Builds where no requested version could be packaged now fail explicitly instead of relying on the empty commit to error out. See #7696. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…ption. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a Plugin Directory release-confirmation edge case where manually rebuilding plugin ZIPs does not update the release metadata (zips_built), leaving releases stuck in a “waiting for processing / ZIPs not built” state. It also adjusts Zip\Builder::build() to treat “no-op” commits (ZIPs already up to date) as successful builds so metadata repair is possible even when artifacts don’t change.
Changes:
- Update
bin/rebuild-zip.phpto mark built releases (zips_built,zips_built_from_revision) after each successful build chunk. - Update
zip/class-builder.phpto treat commit failures with no SVN-parsed errors as success, and to explicitly fail when no requested versions could be built.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wordpress.org/public_html/wp-content/plugins/plugin-directory/zip/class-builder.php | Adjusts build success/failure semantics around empty/no-op commits and tracks whether any versions were actually packaged. |
| wordpress.org/public_html/wp-content/plugins/plugin-directory/bin/rebuild-zip.php | Records zips_built metadata during manual ZIP rebuilds (per chunk) to match importer behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… as a no-op. An absent `svn: E####:` error line alone doesn't prove the commit was a no-op: unexpected shell output or an interrupted commit also parses to no errors. A genuine nothing-to-commit run produces no output at all, so expose the raw output from `SVN::commit()` and only treat an errorless, outputless failed commit as "the ZIPs were already up to date". Any other unrecognized output now fails the build with that output as the message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
wordpress.org/public_html/wp-content/plugins/plugin-directory/tools/class-svn.php:271
- SVN::commit() sometimes returns $errors as a simple list of strings (e.g., when credentials are missing), but callers (including the ZIP Builder) treat commit errors as arrays containing an 'error_message' key. Returning a consistent error structure here avoids fatal errors in misconfigured environments and keeps the API contract predictable.
return [
'result' => false,
'revision' => false,
'errors' => [ 'No SVN credentials configured.' ],
'output' => '',
| if ( ! $res['result'] ) { | ||
| if ( $res['errors'] ) { | ||
| throw new Exception( __METHOD__ . ': Failed to commit the new ZIPs: ' . $res['errors'][0]['error_message'] ); | ||
| } else { | ||
| throw new Exception( __METHOD__ . ': Commit failed without error, maybe there were no modified files?' ); | ||
| throw new Exception( __METHOD__ . ': Failed to commit the new ZIPs: ' . esc_html( $res['errors'][0]['error_message'] ) ); | ||
| } elseif ( '' !== trim( $res['output'] ?? '' ) ) { | ||
| throw new Exception( __METHOD__ . ': Commit failed: ' . esc_html( trim( $res['output'] ) ) ); |
…IP build as a no-op." This reverts commit cc97480.
See Meta Trac #7696 (which introduced the affected marking behavior).
Problem
bin/rebuild-zip.phpcallsZip\Builder::build()directly and never records that the ZIPs were built. Thezips_builtrelease flag is only ever written by the importer (CLI\Import::rebuild_affected_zips()), so any manual rebuild leaves release-confirmation plugins permanently showing "Release confirmed, waiting for processing" / "The ZIP files for this release have not yet been built by WordPress.org" on the release confirmation screen.This has been the case since https://meta.trac.wordpress.org/changeset/13874 moved the
zips_builtmarking from before the build to after a successful build — before that, releases were marked regardless of build outcome.Changes
bin/rebuild-zip.php: After each successful build chunk, mark the built releases viaPlugin_Directory::add_release()withzips_builtandzips_built_from_revision, mirroring the importer.trunkis skipped since it has no release record. Marking per-chunk records partial progress if a later chunk fails.zip/class-builder.php: A commit that fails with no SVN errors means there were no modified files — the ZIPs on disk were already up to date.build()now treats that as success instead of throwingCommit failed without error, maybe there were no modified files?. Without this, the script cannot repair release metadata for plugins whose ZIPs were already rebuilt by hand — the exact case this change targets. SVN failures always emitsvn: E####:lines whichSVN::commit()parses intoerrors, so genuine failures still throw.Since the no-op commit no longer throws,
build()now tracks which versions actually got packaged and throws explicitly when every requested version failed to export (previously that surfaced as the ambiguous no-op commit error). This also closes a pre-existing edge where a brand-new plugin folder alone made the commit succeed with zero ZIPs built.Impact on the importer
rebuild_affected_zips()isbuild()'s only other caller. Itscatchjust returnsfalse, and both call sites ignore the return value — the only effect was skipping thezips_builtmarking loop. With this change a no-op commit there now marks releases as built, which is correct (the artifacts are already current) and fixes the same latent stuck-release edge in the importer.Not addressed here (pre-existing, worth a follow-up): on partial failure — one version builds, another's export fails, commit succeeds — both callers mark all requested versions as built. Fixing that means changing
build()to report which versions succeeded.Testing
php -lpasses on both files.phpcsreports zero issues on the changed lines (remaining findings in both files pre-date this change).🤖 Generated with Claude Code