Proof of concept: Remove the requirement to check out the gutenberg repository and running the full build script#11019
Conversation
With this new approach, a pre-built zip will be downloaded instead.
This is no longer necessary because a Git repository is no longer used.
This switches to downloading a prebuilt zip file uploaded to the GitHub Container Registry for a given commit instead. This eliminates the need to run any Gutenberg build scripts within `wordpress-develop`.
A REF is typically a human-readable path to a branch where as a SHA is a hash representing an individual commit. Since the latter is what's being used here, this aims to avoid confusion.
edf4224 to
2e3f506
Compare
gutenberg repository and running two build scriptsgutenberg repository and running the full build script
|
There is one remaining issue to figure out: how to map the I've removed the chunk of code responsible for mapping those files to their respective locations within |
|
It seems that the icons are not currently included in the Gutenberg plugin, only in @mcsf I see you introduced this in 5b7a3ad. Could you share a bit more context as to why these are not yet included in the Gutenberg plugin? I don't see them in the latest plugin version, but I may be missing them. |
dmsnell
left a comment
There was a problem hiding this comment.
this is a step up @desrosj and should avoid a lot of the delay when checking out the repo. 🙇♂️
does the zip contain the proper built versions of the artifacts or does it contain source copies? that is, are there any build steps that are occurring on the Gutenberg side that occur before that ZIP is available?
it would definitely be helpful to know in the docs, at least in download-gutenberg.js because I think it’s really not clear if we are downloading the Gutenberg source or some output. if output, I would love to know on the wordpress-develop side what our expectations are of that artifact and why .layers[0].digest is special.
Perhaps there is a command we could include which comparably would generate the right files were we given the Gutenberg source.
is it right that this still leaves the icon disparity in place?
| fs.readFileSync( packageJsonPath, 'utf8' ) | ||
| ); | ||
| sha = packageJson.gutenberg?.sha; | ||
| ghcrRepo = packageJson.gutenberg?.ghcrRepo; |
There was a problem hiding this comment.
are we short on bytes? the gh part is obvious to me as a reader, but I cannot figure out what cr means, and since this only appears to be documented inside this file, I’m not sure it would be clear from the package.json file, especially since it looks like a standard GitHub repo name.
perhaps something githubRepo would suffice? it doesn’t seem like the package.json value has anything specific to do with the Github Container Registry
There was a problem hiding this comment.
I am new to this as well, so I found that GHCR stands for the GitHub Container Registry
There was a problem hiding this comment.
I've added a note within the inlind docblock to explain this at the first occurrence of ghcrRepo. Does that address your concerns here @dmsnell?
I also changed the name of the package to gutenberg-wp-develop-build, making the full reference WordPress/gutenberg/gutenberg-wp-develop-build. It's more specific to the purpose, and hopefully won't lead to any confusion.
Within the GHCR, packages are published to a specific repository. While they are listed on a separate screen, you need to use the Org/Repository/package path to reference them.
| const installedHash = fs.readFileSync( hashFilePath, 'utf8' ).trim(); | ||
| if ( installedHash !== sha ) { | ||
| throw new Error( | ||
| `SHA mismatch: expected ${ sha } but found ${ installedHash }. Run \`npm run grunt gutenberg-download -- --force\` to download the correct version.` |
There was a problem hiding this comment.
at first I was going to ask about making this check before downloading, but I see it exists after downloading.
this is probably better because it informs someone that their files are out of date without eagerly downloading a new copy and replacing the old ones.
There was a problem hiding this comment.
I think I also want to create a file in the root of the repository to store a hash of the entire gutenberg directory immediately after unzipping. Then the user can also be made aware of instances where the files in the gutenberg directory were modified, which may be unexpected.
| const packageJsonPath = path.join( rootDir, 'package.json' ); | ||
|
|
||
| /** | ||
| * Execute a command, streaming stdio directly so progress is visible. |
There was a problem hiding this comment.
Is this really streaming? It seems the stdout variable is populated with streaming, but the promise only resolves with the captured stdout when the child process is closed?
There was a problem hiding this comment.
Updated, how does that look?
Oof, I'm so glad you pinged me! Fix at WordPress/gutenberg#75866 |
The Gutenberg repository has been updated to include the icon assets in the plugin build. See github.com/WordPress/gutenberg/pull/75866.
…/github.com/desrosj/wordpress-develop into try/remove-gutenberg-git-checkout-and-build
- Add missing hard stop at the end of inline comment. - Make use if `fetch()` instead of `exec( 'curl' )`. Co-authored-by: Weston Ruter <westonruter@gmail.com>
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| throw new Error( `Failed to download blob: ${ response.status } ${ response.statusText }` ); | ||
| } | ||
| const buffer = await response.arrayBuffer(); | ||
| fs.writeFileSync( zipPath, Buffer.from( buffer ) ); |
There was a problem hiding this comment.
at this point it’s probably possible to remove all of the _Sync versions and stream the output directly into the file.
not essential, but it does feel like we’re possibly going out of our way to do more work to avoid a simpler default.
await fs.writeFile( zipPath, response.body );There was a problem hiding this comment.
I'm not deeply knowledgeable here, but Claude told me that this wouldn't work as you've written "since fs.promises.writeFile doesn't accept a ReadableStream — stream.pipeline is the correct API for this." Does this seem right now?
| // Extract the zip into ./gutenberg. | ||
| console.log( `\n📦 Extracting ${ zipName } into ./gutenberg...` ); | ||
| try { | ||
| await exec( 'unzip', [ '-q', zipPath, '-d', gutenbergDir ] ); |
There was a problem hiding this comment.
if we use zlib.Unzip we should be able to eliminate the exec() wrapper altogether and not depend on the runtime system having unzip in the PATH
of course, if we have a .gz file available from the container registry (which seems like it would exist) then we also have zlib.gunzipSync()
There was a problem hiding this comment.
as an additional bit of polish, which is not quite as impactful as removing the exec() abstraction and runtime-dependence, we can also pipe directly from the fetch() into zlib. this would eliminate the temporary files and reduce the memory load. I hesitate to leave this comment because it truly is polish and not part of the basic requirements of this proof-of-concept.
There was a problem hiding this comment.
The file only exists as a zip currently. If we want that format to be available, we would have to change the workflow generating and pushing the compressed file to the registry.
…rg-git-checkout-and-build
…rg-git-checkout-and-build
…rg-git-checkout-and-build
…rg-git-checkout-and-build
…rg-git-checkout-and-build
|
I still need to go through and polish this, but the PR for the Gutenberg repository is ready. That should be merged first anyway, and these changes can't be committed until after the first hash update following that PR being merged. So I'm hoping to clean this up on Monday. |
This is a work in progress.
This PR attempts to remove the need to checkout the WordPress/gutenberg Git repository locally by instead downloading a copy of the built plugin from the GitHub Container Registry for the respective pinned hash value (see WordPress/gutenberg#75844 for that part of this approach).
This PR also changes usage of
reftoshato avoid confusion. Ref is typically a human-readable branch path. A commit hash or SHA value is the full length, unique hash for a commit.Trac ticket: Core-64393.
Use of AI Tools
I used Claude Code to make some of the adjustments to the
Gruntfile.jsfile andtools/gutenbergfolder.This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.