Opened 4 months ago
Closed 32 hours ago
#64393 closed task (blessed) (fixed)
Change how we include Gutenberg in Core
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.0 | Priority: | high |
| Severity: | blocker | Version: | |
| Component: | Build/Test Tools | Keywords: | has-patch |
| Focuses: | Cc: |
Description (last modified by )
When we setup the initial architecture and dependencies between Gutenberg and Core, Gutenberg was still considered a post content editor. It was setting at the same level of abstraction as TinyMCE. This assumption let to the use of npm packages to backport code from the Gutenberg repository into Core.
The reality though is that this assumption is not valid for some time now:
- Gutenberg includes a lot of iterations to PHP code base: endpoints, pages, menu items, block supports, block server side rendering, theme.json rendering
- The Gutenberg repo outgrow the "post content" for some time now and it now powers multiple pages in wp-admin, multiple extensions to existing pages...
During all that time, we "hacked" the process to accommodate some of this:
- We have a lot of manual php backports
- We do some automatic renames to php functions
- We have npm packages that contain both php and JS and a lot more "hacks".
In WordPress 7.0, we'll have a new addition to the Gutenberg repository that will force us to either add a new "hack" or reconsider the process. We'll have a new "font library" page added to wp-admin and the way it's being added is by using a new tool called @wordpress/build that automatically registers WP-Admin pages and uses "routes" to power these pages. This is a generalization of what we've been doing before for the site editor (and to some degree the post editor) to allow for easier iteration, scalability and extensibility.
The build tool is likely to grow as well to absorb newer more and more extensibility points (or concepts) that don't exist today: Blocks, dashboard widget, field collections (these are just ideas at this point).
The question becomes how do we backport these to Core (routes, generated pages, ...). I see two options in front of us:
- Continue adhoc backports, copy the generated "php" code from the build folder of Gutenberg manually, sync the "routes" folder manually from Gutenberg and have some kind of build process on Core to recreate the same code. Very likely to create sync issues and more.
- Reconsider how we "embed" Gutenberg into Core.
I'm proposing that we take some time to explore the second option here and potentially simplify the whole process. What if we make Gutenberg a git submodule of WordPress Develop, pin the submodule to the Gutenberg branch that we want to include in Core (wp/* release branches) and update Core's build tooling to just call Gutenberg's (most of the time).
I'm sure there are a lot more complexities to be discovered here. But I also see a lot of potential to both address the "routes" problem but also streamline some of the current php backports (not all of them).
Related work
- #64521 Restore the block parser PHP class in Core, remove from Gutenberg.
- dmsnell#27 Combined view of sequence of patches for this ticket for easier understanding of the change as a whole.
Attachments (1)
Change History (251)
#2
@
4 months ago
Monorepo requires a lot more work: aligning trac and GitHub issues, updating workflows releases, tests... While it would also be a solution, it's not one that can happen in time for 7.0 and the above is needed for 7.0. It's a programmatic proposal.
I think what we need is technical explorations at this point.
#4
@
4 months ago
Keep in mind that the source of truth is Subversion still, we can't simply add a git submodule to wordpress-develop, which is just a Git mirror
#5
@
4 months ago
Keep in mind that the source of truth is Subversion still, we can't simply add a git submodule to wordpress-develop, which is just a Git mirror
Not planning to change that, but a git submodule (or an svn external for that matter) is similar to an npm package, it's an external dependency you bring during the project initialization.
#6
@
4 months ago
Instead of a git submodule, could it just be an empty folder that the build process uses to checkout Gutenberg?
#7
@
4 months ago
Instead of a git submodule, could it just be an empty folder that the build process uses to checkout Gutenberg?
Yes, that works for me. I'm not really opinionated about how we fetch the Gutenberg repository itself, important part is to treat it as a single external dependency (instead of multiple small ones like today)
#8
@
4 months ago
Instead of a git submodule, could it just be an empty folder that the build process uses to checkout Gutenberg?
The main difference I see between these two options is that, if we intend Core to use PHP files stored in Gutenberg directly, with the empty folder strategy it will no longer be possible to run the PHP parts of Core without a build step.
I guess my question is how do we determine which PHP files live in Gutenberg and which in Core. For instance, it might make sense for all the editor-related files such as block supports to live in Gutenberg, as they're closely tied to the JS (and currently they're duplicated across both codebases).
But if we intend to continue using the Gutenberg plugin as an experimental development platform there will still be a need to filter, extend or override parts of the Core PHP from time to time so new features can be tested in the plugin. So from that perspective it's unlikely we'll ever fully resolve the problem of porting PHP changes from Gutenberg to Core.
#9
@
4 months ago
with the empty folder strategy it will no longer be possible to run the PHP parts of Core without a build step
Not sure if I see a difference between the two. A git module would still require the same step in an svn repo?
I guess my question is how do we determine which PHP files live in Gutenberg and which in Core.
Yes, I don't think this solution aims to fully solve PHP "backports". The way I see it is that we'd still require a hook in Gutenberg and a "backport" in core, even though they're pretty much added at the same time. The Gutenberg plugin will also still have to run without WP trunk, on the current WP version. But what it does solve is removing the need for npm package syncs, and backporting PHP changes at a later time (this pretty much forces us to commit both at the same time).
#10
@
3 months ago
A git module would still require the same step in an svn repo?
I was assuming that the git module would translate to an svn external in the svn repo and it would "just work" but I may be wrong :)
But what it does solve is removing the need for npm package syncs, and backporting PHP changes at a later time
Absolutely, if we can solve this just for package-related code it will make our lives much easier.
This ticket was mentioned in PR #10638 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#11
- Keywords has-patch added
Trac ticket: https://core.trac.wordpress.org/ticket/64393
## Summary
This PR changes WordPress Core's Gutenberg integration from npm packages to a checkout-and-build approach. Instead of syncing individual npm packages, Core now checks out the Gutenberg repository, builds it, and copies the build artifacts. This enables Core to use Gutenberg's advanced features like route-based navigation, full-page rendering, and the new Font Library. But also offers us the possibility to streamline more backports later (an example in this PR is the automatic copy of theme.json file). Check the issue for more details on the issue at hand.
## New Build Pipeline
- Checkout:
npm run gutenberg:checkoutclones Gutenberg at a specified ref - Build:
npm run gutenberg:buildruns Gutenberg's build process - Copy:
npm run gutenberg:copycopies and transforms build output to Core - Integrate:
npm run gutenberg:integrateruns all three steps
## What Gets Copied
From Gutenberg build to Core:
/build/routes/→/src/wp-includes/build/routes//build/pages/→/src/wp-includes/build/pages//build/modules/→/src/wp-includes/js/dist/script-modules//build/scripts/→/src/wp-includes/js/dist//build/styles/→/src/wp-includes/css/dist//build/blocks/→/src/wp-includes/blocks/
## Path Transformations
The copy script transforms Gutenberg plugin paths to Core paths:
plugins_url()→includes_url()plugin_dir_path()→ABSPATH . WPINC . '/build/'- Boot module paths adjusted for Core's directory structure
- Enqueue conditions work with both menu pages and direct file access
Ideally these transformations shouldn't be needed, maybe we can make our "build" tool more generic to streamline the integration better and allow "building for core" directly. I'll look into that separately.
## Webpack Changes
Removed webpack configs that are now replaced by Gutenberg's build:
- ❌
tools/webpack/blocks.js- Using Gutenberg's block builds - ❌
tools/webpack/packages.js- Using Gutenberg's script builds - ❌
tools/webpack/script-modules.js- Using Gutenberg's module builds - ❌
tools/webpack/development.js- Using Gutenberg's dev builds - ❌
tools/webpack/vendors.js- Using Gutenberg's vendors - ✅
tools/webpack/media.js- Kept (Core-specific files). to be honest, I'm not really sure if Webpack is needed for these files but I kept it unchanged for now, we could decide to remove the webpack dependency later entirely.
## Font Library Integration
Added /wp-admin/font-library.php as the first proof-of-concept using this new architecture.
### Some other changes.
src/wp-includes/script-modules.php
- Removed outdated debug version handling for
@wordpress/interactivity(this has been removed from Gutenberg)
Gruntfile.js
- Added a vendor copy step that was previously done in webpack/packages.js
package.json
- The "hash" or Gutenberg version used is defined here. Right now I picked a recent Gutenberg commit but this is likely to refer commits in
wp/*release branches in Gutenberg.
## Testing
- There should be no big disruptions in the core development workflow. I tried to keep the flow the same, the output files in the same places...
- The initial build might be slow because of typescript building, the next ones should be ok.
---
I think there are a lot of simplifications we could do later (specially to the copy script) by aligning the folder structures more closely together (the build folder of Gutenberg, and the file structure for built scripts, styles, modules, pages... on Core). It does require changing some paths on Core. I do think it would be for the better but I kept it out of this PR.
@youknowriad commented on PR #10638:
3 months ago
#12
Looks like there's an infinite loop because wp-build use time based hashes which triggers the "commit unsaved changes" on each commit. I'm going to disable that workflow temporarily.
@ellatrix commented on PR #10638:
3 months ago
#13
This is exciting! Also paves the way to run Gutenberg e2e tests :)
@youknowriad commented on PR #10638:
3 months ago
#14
What I like about this process is that it actually clarifies what it takes to "integrate Gutenberg into Core" it all happens in a single command. While previously it was scattered all over the place.
@youknowriad commented on PR #10638:
3 months ago
#15
Ok I think this is ready to land personally. I'd love reviews though :)
#16
@
3 months ago
- Keywords commit added
Approved on Github: Works great and makes a lot of sense. Let's try it. 🙂
@desrosj commented on PR #10638:
3 months ago
#17
I've started to take a look at this, but it's a pretty big PR and I need a bit more time.
Is this a blocker for anything specific? Or is there room for a few more days to review?
@youknowriad commented on PR #10638:
3 months ago
#18
Can wait for a few more days. Maybe we should aim to land it early next week.
@isabel_brison commented on PR #10638:
3 months ago
#19
I gave this a test run on my local env and it's working really well so far! I did have to point the env to the build directory (the default in the .env file is src) and then of course npm run build.
Without having looked extensively at the code yet, I love the direction! It will save us so much manual work in porting code from Gutenberg to Core ❤️
@youknowriad commented on PR #10638:
3 months ago
#20
I thought npm run build was meant to build the "build" folder and npm run dev was for the "src" folder. Now, it's possible that this assumption is wrong and I'm happy to correct my scripts if needed. It's really unclear to me what script is supposed to target which output folder.
@isabel_brison commented on PR #10638:
3 months ago
#21
I thought
npm run buildwas meant to build the "build" folder andnpm run devwas for the "src" folder
Yes that's correct! The WORKING_DIR is set to src whenever the dev flag is passed. So:
npm run devwatches the src foldernpm run build:devbuilds the src foldernpm run watchwatches the build foldernpm run buildbuilds the build folder
With this PR, I wasn't able to get my env running or build it from the src directory, it only worked from the build one. I thought I'd mention that in the comment so other folks who test it don't run into the same issue.
@youknowriad commented on PR #10638:
3 months ago
#22
I just tried and confirmed that both npm run dev and npm run build:dev work for me with the default "src" environment.
@youknowriad commented on PR #10638:
3 months ago
#23
@desrosj Hi Jonathan, did you have time to look at the PR. I'd love to avoid letting it sit for too long. Regardless, I'll be available for any follow-ups...
@desrosj commented on PR #10638:
3 months ago
#24
I did not get to look much further.
One suggestion I was going to make was around the new files that are replacing old ones that did the same thing. For example, managing packages. I was going to suggest that those are added to version control using an svn copy, and then all of the changes be applied.
While ya a new approach, this maintains the history of those files so that a visual comparison is available instead of just a sea of red and green in separate files.
@youknowriad commented on PR #10638:
3 months ago
#25
One suggestion I was going to make was around the new files that are replacing old ones that did the same thing. For example, managing packages. I was going to suggest that those are added to version control using an svn copy, and then all of the changes be applied.
While this is a new approach, this maintains the history of those files so that a visual comparison is available instead of just a sea of red and green in separate files.
Thanks for quickly chiming in, I guess what you're saying is that we shouldn't git ignore the files that come from Gutenberg.
I found that the previous approach of gitignoring some files (JS, CSS) and no others (PHP, JSON) was not very consistent and also created a lot of confusion of where the source of truth of these files is. I think with the changes in this PR, it's a lot more clear. These are external dependencies maintained in Gutenberg.
@desrosj commented on PR #10638:
3 months ago
#26
Sorry, I'm not at my computer currently, so that was poorly worded without an example. That comment was specifically targeted at any files within the tools directory only.
@youknowriad commented on PR #10638:
3 months ago
#27
Ah I see :) Thanks for clarifying. It makes more sense now. I don't really think there's a 1 to 1 relationship with what was being done before though, so it's a bit hard to think of this as "file renames", it's a completely different pipeline.
@youknowriad commented on PR #10638:
3 months ago
#28
Given the holidays and all, I decided to let this sit for a few more days. We do need to ship it soon enough before 7.0 beta 1 so I'm planning to do that early January, everyone will be back and we'll have time before 7.0 to polish. I don't really expect any major issues, just taking the safe road here.
#29
@
3 months ago
- Owner set to youknowriad
- Resolution set to fixed
- Status changed from new to closed
In 61438:
This ticket was mentioned in PR #10681 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#30
This fixes a couple of issues raised by @swissspidy on the Gutenberg integration script.
- It restores the theme.json i18n file for wp-cli to work properly.
- It renames gutenberg_ prefixes to wp_
Trac ticket: [](https://core.trac.wordpress.org/ticket/64393)
#32
@
3 months ago
- Keywords needs-patch added; has-patch commit removed
- Priority changed from normal to high
- Resolution fixed deleted
- Severity changed from normal to blocker
- Status changed from closed to reopened
Reopening as this has broken the build.
It works fine if define( 'SCRIPT_DEBUG', true ); is included in the config file but if it's set to false/uses the default value the styles do not load as many of the *.min.css files have been removed.
See screen shot above. Trac won't display the combined diff due to the size but the affect can be seen on GitHub's clone of the build repo WordPress/[email protected].
Bumping the priority due to the need for a fix.
#33
@
3 months ago
I've created a draft pull request which appears to fix the issue. I am far from confident it won't have unintended side effects so haven't marked it ready for review.
In addition to the issue above:
- I've noticed that the SVN properties were not updated so running
npm run buildin the svn repo results in a dirty repo. Very dirty as the directory that is been included includes the entire Gutenberg git repository. .mapfiles have been added to the build repo in https://build.trac.wordpress.org/changeset/60750, they're not usually included in the distribution.
Having reviewed the changes further, I am strongly inclined to revert the commits on this ticket.
My primary issue is that not enough time was given for people to review the pull request. While it was opened on December 16, the majority of contributors have been absent from December 19 through to the new year. Effectively the PR was only open for three or four business days. Some of the busiest business days of the year.
Secondly, I don't think the changes were tested anywhere near closely enough. The Playground link on the originating pull request showed the issue but it went unnoticed. Catching this issue was literally two clicks away from the PR and it wasn't done.
#34
@
3 months ago
Thanks Peter for spotting that and for the follow-up. This is exactly why we need this commit to land early before beta, so it gets as much testing as possible which may or may not be caught in PRs.
For the issue itself, the main problem is that Gutenberg and Core have different expectations for built CSS. In Gutenberg only one file is built, Core expects both. For JS on the other side they both built two files. So a good path here is to make the behavior consistent. It should be a smallish a change, I'll be working on it today.
This ticket was mentioned in PR #10685 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#35
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/64393
## Description
Updates the Gutenberg hash to include WordPress/gutenberg#74380, which fixes the CSS build to generate both minified (.min.css) and non-minified (.css) versions. Previously, Gutenberg only produced a single CSS file, causing 404s when SCRIPT_DEBUG is false (Core expects .min.css files).
## Testing
- Run npm run build:dev
- Verify both style.css and style.min.css exist in src/wp-includes/blocks/*/
- Test with SCRIPT_DEBUG=true and SCRIPT_DEBUG=false — editor should load correctly in both cases
@jsnajdr commented on PR #10685:
3 months ago
#36
The dea73 commit points to somewhere in a branch? The corresponding trunk commit is 4856d5d1842.
@mcsf commented on PR #10685:
3 months ago
#37
The
dea73commit points to somewhere in a branch? The corresponding trunk commit is4856d5d1842.
Good catch, that's the commit in the extinct fix/build-tool-old-hash-do-not-remove-branch branch
#39
@
3 months ago
The main minified file issue should be fixed in the last commit, that said, I didn't update the svn:ignore yet. I looked at it and it seems very different from the .gitignore, so I'm not really sure why is it the case.
Here are the list of new gitignored files
/gutenberg /src/wp-includes/blocks/* !/src/wp-includes/blocks/index.php /src/wp-includes/build /src/wp-includes/class-wp-block-parser.php /src/wp-includes/class-wp-block-parser-block.php /src/wp-includes/class-wp-block-parser-frame.php /src/wp-includes/theme.json
#41
@
3 months ago
The checkout-gutenberg.js script unconditionally checks out the Gutenberg repository; after checking out it contains 1.8 GB on my machine as I write this comment.
It makes multiple rounds of git fetching and introduces network dependencies into the Core development workflow, making a build fail when unable to reach the Internet.
This has added many minutes to my local build, which otherwise is practically instant with php -S 0.0.0.0 -t ./src. Core already required building the JS files, but this adds another step which is serial, unnecessary for typical development, and network-dependent. (Whereas previously, letting those JS files stagnate caused very little impediment to development on unrelated files, this new step adds the build unconditionally and thus obstructs where the previous didn’t).
It also seems to run git checkout FETCH_HEAD which could be combined into the previous fetch, which could be combined into the previous clone.
The npm ci step is particularly heavy, which in the case of building a release or running CI jobs seems very appropriate, but it intentionally deletes all 1.8 GB of packages, reruns the dependency resolution when it doesn’t need to, re-downloads every package, and builds everything, when nothing has changed or needs to change. This could be avoided by a number of different mitigations, including an optimization that checks if any of this work is necessary before doing it on every build.
I would like to propose that we add these steps for now only with the addition of some new CLI arg or ENV value. It’s too costly to introduce this overhead for every change or invocation of any development server, and why not make it opt-in to isolate the impact to where its value is greatest?
#42
@
3 months ago
I should note as well that this has basically trashed my ability to develop on Core without multiple minutes of lag between every change. I never had to run npm run dev before, but now I have to or else WordPress crashes with the message to build the files.
This needs resolving IMO, and something that I think is uncovered with brief and basic testing. The page WordPress provides which instructs to run npm run build no longer works reliably; when I follow its steps I’m left seeing the same page, unable to load WordPress. This is discouraging me greatly.
While I support the monumental work to better converge Gutenberg with Core, I am concerned about sweeping changes like this being made without broader understanding of the impact on different parties. We know there will be pain, so let’s do everything we can to avoid dumping it unnecessarily on bystanders.
This ticket was mentioned in PR #10688 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#43
This updates the Gutenberg build integration to use the new --fast flag, which skips TypeScript-related steps (version validation, tsc --build, and type declaration checks) that aren't needed when building for WordPress Core. These steps only produce .d.ts type declaration files which aren't shipped with Core.
The Gutenberg ref is also updated to include the commit that adds the --fast flag support.
@youknowriad commented on PR #10688:
3 months ago
#44
For reference, I compared the build step in three jobs:
1- Before the move to the new integration flow 1m19 seconds
2- After the move to the new integration flow 6m17 seconds
3- After this PR 1m49 seconds
So I think with this PR we're effectively back to similar decent performance levels for the build commands and comparable to previous jobs without losing the benefits for the "checkout & build" approach.
#45
@
3 months ago
We also need to talk about how this negatively affects https://developer.wordpress.org/reference/
Now that many classes and functions no longer exist in trunk without a build step, this probably breaks pages like https://developer.wordpress.org/reference/classes/wp_block_parser_block/
#46
@
3 months ago
All, as I tend to do from time to time I overreacted and was needlessly emotionally charged when I wrote my comments. I apologize for any disrespect I showed towards @youknowriad or others working on this.
In response I’ve updated my comments to better reflect what I hope is more helpful commentary, summarized here:
- Can we ensure that we find a way to make Core build without introducing network dependencies after cloning? or certainly after having run
npm install? As someone who frequently works tethered to a capped data plan, it’s not sustainable for me to wait for hundreds of megabytes of compressed JS files to download and build when working. - Can we adjust this so that nothing more than an absolutely minimum build is required for WordPress to function? Releases cannot have stale files, but I work productively in stale files all the time. I am not 100% always on the latest
trunk, and I will not always have the latest JS files built either, but that doesn’t impede me from working on email, or HTML safety, or on web standards support. If and when I realize that things are broken I can choose to run the update and the build. - Can we immediately stop running multiple
gitfetches, especially when they aren’t necessary.gitis immutable, and if we have therefthere’s no need to try to fetch it again. At a minimum, if we have a commit SHA it’s essentially free to check if the ref is already fetched. - Similarly, can we avoid running
npm ciif therefis unchanged since the last run? or ideally gate it behind a CLI arg or an ENV value so that it doesn't constantly run and download and reinstall packages?
#47
@
3 months ago
Now that many classes and functions no longer exist in trunk without a build step, this probably breaks pages
@swissspidy what’s the mechanism here? is it because the API docs don’t run the build process?
For balance, our API docs have been broken and have not updated since WordPress 6.7.0 and many classes are already missing or stale, so there is plenty of work there beyond this.
#48
@
3 months ago
An unrelated PR is failing on the new step because of a network issue when running npm install. GitHub’s log shows ECONNRESET.
Might be good to monitor the ratio of flakey test failures and if this is a coincidental fluke or if the new network dependencies are going to make a notable impact.
#49
@
3 months ago
To add to the above concerns, the addition of npm run build:dev in the GitHub Actions workflows adds 6 minutes to every PHPUnit test job and Local Docker Environment test job. The PHPUnit tests have gone from taking ~16 minutes to taking ~22 minutes.
#50
@
3 months ago
Some of my normal scripts which run on a fresh checkout of wordpress-develop no longer work, because the PHP files are missing without the build step.
This particularly affects my benchmarking scripts, which is what led me to this now (I did not set out looking to find problems with it). My benchmarking of WordPress releases and HTML API and other performance-related changes involves running tests for hours, as it is, flipping between fresh database and code states.
With the addition of the build step this is multiplying my required test runtimes into days and more.
This is all due to the PHP files which were removed from the repository and then added to .gitignore. Could we leave the PHP files intact so that Core isn’t missing necessary classes on a checkout? And then when we pull in the code from Gutenberg, allow for those “backports” to also update the version-controlled files? Surely we could detect if there were changes in Core that weren’t in Gutenberg with something like git diff PREV_REF...CURRENT_REF -- src/wp-includes/class-wp-block-parser.php as a way to avoid overwriting changes.
#52
@
3 months ago
With the last commits, the build time are back to comparable numbers to before. You can see a recap here https://github.com/WordPress/wordpress-develop/pull/10688#issuecomment-3717902779
#53
@
3 months ago
Can we ensure that we find a way to make Core build without introducing network dependencies after cloning? or certainly after having run npm install? As someone who frequently works tethered to a capped data plan, it’s not sustainable for me to wait for hundreds of megabytes of compressed JS files to download and build when working.
I don't see this that different from what we were doing previously as the same code was being downloaded into node_modules and required network dependencies. I guess the main difference is that the heck.
This needs resolving IMO, and something that I think is uncovered with brief and basic testing. The page WordPress provides which instructs to run npm run build no longer works reliably; when I follow its steps I’m left seeing the same page, unable to load WordPress. This is discouraging me greatly.
I'm happy to look at this, I did test this and I can't reproduce personally but again there are multiple flows to develop on WordPress Core and I'm happy to look specifically at your flow.
This ticket was mentioned in PR #10691 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#54
Trac ticket: https://core.trac.wordpress.org/ticket/64393
Moves Gutenberg checkout to postinstall hook, separating dependency setup from build. Removes checkout step from the build step.
## Benefits:
- Faster build times: checkout happens once at install, not every build (although the script was already accounting for this before)
- Cleaner separation of concerns: dependencies are ready before any build runs
## Test plan
- Run npm install and verify Gutenberg checkout runs automatically
- Run npm run build and verify build completes successfully
- Run npm run build:dev and verify dev build works
#55
@
3 months ago
The last PR effectively moves the gutenberg checkout to the npm install (post install script) effectively restoring a very similar flow to what we had before.
@youknowriad commented on PR #10691:
3 months ago
#56
Noting that with this PR the "build" step of WordPress takes 30 seconds less than what we used to have before the change in Gutenberg integration. The tradeoff is that the "install" step is a little bit slower. This feels like a win since building is something you do more often and install is done once.
#57
@
3 months ago
Thanks for your follow-up and continuing work on this @youknowriad.
I don't see this that different from what we were doing previously
The main difference is that once those files were built they could remain stale for a very long time. I think occasionally they needed to be rebuilt, perhaps when files were deleted or added. Obviously if the JS files were stale things wouldn’t be 100% right, but I tried to address above how this is different than removing required PHP files.
There is another distinction: prior to this change, with a valid wp-config.php file it was possible to checkout the code and run require __DIR__ . '/src/wp-load.php';. This is how plenty of my scripts are built, and how I do interactive development within the PHP shell, and how my tests run.
After this change, attempting to require wp-load.php crashes because the PHP files aren’t there which Core imports. in other words, trunk is broken without running corrective build steps, which require network access, heavy downloads, and long delays.
There’s a coincidence in my reporting because I happened to be drawn to parser work after finding a longstanding issue for a particular kind of malformed post. The git history is gone from the file and I can’t see my changes or scan back in history.
It’s actually worse than before because with Gutenberg as a plugin I could have the files separate and load Gutenberg the plugin. Still, I could modify Core files to explore solutions and then create patches on both sides. Now, however, not only have I lost all version-control information in the PHP files that are now “built” but I also can’t reset those files because they are ignored. I have to manually copy and paste files between projects and files and make sure I don’t get confused.
This is why I think we should at a minimum leave all required PHP files in Core that it needs to run. Moving the npm ci step into the installation is better than running it on every change, but when bisecting, benchmarking, investigating history, it’s still dramatically obstructive.
What is the reason for deleting these files and adding them to .gitignore?
Ideally, Core should work, albeit in a degraded state, without running any build, especially without running npm commands.
#58
@
3 months ago
@youknowriad I've created a test repo that includes only the build changes, ie without commits to wp-dev that are unrelated in the mean time. A comparison of the before and after changes can be seen at peterwilsoncc/[email protected]. I'm updating this manually each morning Australian time.
- The *.map files are still being committed to the build, these will need to be removed as they're not referenced in the generated files & the references are to uncommitted code.
- The auto generated functions introduced in GB#73877 will need to be correctly prefixed with
wp_- egfont_library_wp_admin_render_page() - These auto generated files are also very difficult to review
- A new
index.phpfiles has been introducedwp-includes/build/routes/index.php- it would be good to avoid this as we've had issues with using index file names in the past, see #60237. - I'm seeing a few package.json files, eg
wp-includes/build/routes/navigation/package.json wp_register_development_scripts()has been removed without been deprecated- The global variables in
wp-includes/build/routes.phpare unprefixed. It also adds actions in such a way that's it's not possible for third party devs to remove them
These changes also make it very unclear what PHP changes are been introduced. This is a problem as there are often issues picked up during the merge of packages to core. Package bumps will need to provide a diff that can be reviewed for the build effects.
There's a lot going on in these changes so it's probable that I am missing a few things.
We'll also need to consider whether gitignore and SVN props will need to be backported to branches receiving security updates to avoid the accidental commit of the gutenberg repo in to wp-dev.
@youknowriad commented on PR #10691:
3 months ago
#60
Commtted.
#61
@
3 months ago
What is the reason for deleting these files and adding them to .gitignore?
Yes, there is, these files are maintained in Gutenberg, and the source of truth for them is there. So their version history is there. In the same way it doesn't make sense to commit the "vendor" of composer dependency, these are php dependencies but we're just not using composer yet.
Ideally, Core should work, albeit in a degraded state, without running any build, especially without running npm commands.
I think this is not the case for a long time now, if you don't build Core, you get an empty page asking you to build (before and after the commit)
#62
@
3 months ago
These auto generated files are also very difficult to review
The best way to review this is in Gutenberg, we need to review the build tool templates. These functions are not just about Core, they are also about plugins registering their own pages.
I've tracked all the enhancement suggestions that you have in https://github.com/WordPress/gutenberg/issues/74488
I'm seeing a few package.json files, eg wp-includes/build/routes/navigation/package.json
Yes, this is expected, What bothers you with this?
I'm seeing a few package.json files, eg wp-includes/build/routes/navigation/package.json
wp_register_development_scripts() has been removed without been deprecated
Restored and deprecated the function here. https://github.com/WordPress/wordpress-develop/pull/10702
These changes also make it very unclear what PHP changes are been introduced. This is a problem as there are often issues picked up during the merge of packages to core. Package bumps will need to provide a diff that can be reviewed for the build effects.
This is not needed, this a build tool maintained in Gutenberg with the same guidelines as Core, it's just another repository. Changes are verified there already. Similarly to all the JS, packages and all the dependencies.
We'll also need to consider whether gitignore and SVN props will need to be backported to branches receiving security updates to avoid the accidental commit of the gutenberg repo in to wp-dev.
Since this only affects trunk, I don't think there's anything needed here unless I'm missing something.
The *.map files are still being committed to the build, these will need to be removed as they're not referenced in the generated files & the references are to uncommitted code.
I'm having trouble understanding what exactly need to be svn ignored and what not. Can we instead just map .gitignore as is in svn:ignore? Happy to get some help here as well.
#63
@
3 months ago
I've noticed that changing the Gutenberg hash doesn't trigger the install-changed script to reinstall, this is due to the package generating the hash from the dependencies rather than the entire contents of the package.json file.
Reproduction:
- Run
npm ci; npm run build - Change the Gutenberg reference in the package file to
trunk - Run
npm run build - Observe the script reports
package.json has not been modified.
#64
@
3 months ago
wp_register_development_scripts() has been removed without been deprecated
Restored and deprecated the function here. https://github.com/WordPress/wordpress-develop/pull/10702
I think wp_register_development_scripts() is a function to enqueue the library for react hot reload. If we delete this function or change it to a function that does nothing, won't block developers no longer be able to use hot reload?
#65
follow-up:
↓ 93
@
3 months ago
It looks like this change is also affecting common plugin unit test setups :-(
For example, I'm checking out wordpress-develop to have the WP unit test framework available.
However, when running tests I only get
PHP Warning: require(/home/runner/work/TablePress/TablePress/src/wp-includes/build/routes.php): Failed to open stream: No such file or directory in /home/runner/work/TablePress/TablePress/src/wp-settings.php on line 245
PHP Fatal error: Uncaught Error: Failed opening required '/home/runner/work/TablePress/TablePress/src/wp-includes/build/routes.php' (include_path='.:/usr/share/php') in /home/runner/work/TablePress/TablePress/src/wp-settings.php:245
Stack trace:
#0 /home/runner/work/TablePress/TablePress/tests/phpunit/includes/install.php(40): require_once()
#1 {main}
thrown in /home/runner/work/TablePress/TablePress/src/wp-settings.php on line 245
#66
@
3 months ago
1) When doing an update test to trunk I noticed that the gutenberg/package.json file ends up dirty following a build. When switching back to the initial hash, this resulted in the following error.
Command errors: error: Your local changes to the following files would be overwritten by checkout: package.json Please commit your changes or stash them before you switch branches. Aborting ❌ Fetch/checkout failed: git checkout FETCH_HEAD failed with code 1 npm error code 1 npm error path /Users/peterwilson/Sites/wp-dev/wordpress-develop npm error command failed npm error command sh -c npm run gutenberg:checkout npm error A complete log of this run can be found in: /Users/peterwilson/.npm/_logs/2026-01-11T22_25_46_800Z-debug-0.log
This was introduced in r61438 due to the build step changing the package file.
2) When doing the test checkout with trunk, I've noticed that the gutenberg-experiments check introduced in Gutenberg@3bf3e488fd is included.
This will mean that if the Gutenberg plugin is disabled without turning off the experiment, the experiment will continue to run in WordPress-Develop.
This is not needed, this a build tool maintained in Gutenberg with the same guidelines as Core, it's just another repository. Changes are verified there already. Similarly to all the JS, packages and all the dependencies.
I agree, this would be the ideal case but it's not the reality. It's common that deprecation errors are caught during the import: either functions removed without proper deprecations (as happened on this ticket), or missing calls to _deprecated_*().
Some merge PRs get a few comments, some get dozens of comments.
The issue I noted above re: the navigation experiment is a case in point.
Since this only affects trunk, I don't think there's anything needed here unless I'm missing something.
I thinking about when switching branches as the gutenberg directory will be left behind so could be committed in error.
I'm having trouble understanding what exactly need to be svn ignored and what not. Can we instead just map .gitignore as is in svn:ignore? Happy to get some help here as well.
The map files and package files I am seeing are generated during the build step so aren't committed to wp-dev. They'll need to be removed in a change to how the build script runs.
I am still inclined to revert the commits on this ticket.
It's a huge change and is causing too much friction for people attempting to work on WordPress-Develop.
Frankly, I think it was irresponsible to commit this in the first palce. The PR had had numerous change requests since the initial approval along with numerous commits. It was unsuitable for a YOLO commit.
We're already at four follow up commits and more will need to come.
@jorbin, @johnbillion, @desrosj as build tools component maintainers, I'd value your thoughts.
#67
@
3 months ago
I am still inclined to revert the commits on this ticket.
I think reverting creating more issues that it solves. This change is necessary for features slated for 7.0 and it's better to solve them way before alpha/beta than run to solve them at the last moment.
Also at this point, the workflow is very similar to what we had before, aside some slight changes on what scripts run (wp-build instead of webpack...).
I agree that maybe it was committed a bit too soon but I'm not seeing any major issues at this point to be honest.
#68
@
3 months ago
It's a huge change and is causing too much friction for people attempting to work on WordPress-Develop.
Let's also see things from the other side a bit: it's a huge change that resolves a ton of friction on the Gutenberg dev side. I can't understate how much work and overhead it is every release to sync and merge the Gutenberg codebase into core.
Frankly, I think it was irresponsible to commit this in the first palce.
Worth noting that we're in the alpha stage and commits were made swiftly to resolve issues. It's really hard to make these changes without trying them.
#70
@
3 months ago
2) When doing the test checkout with trunk, I've noticed that the gutenberg-experiments check introduced in Gutenberg@3bf3e488fd is included.
This will mean that if the Gutenberg plugin is disabled without turning off the experiment, the experiment will continue to run in WordPress-Develop.
This seems unrelated to how the code is integrated. cc @get_dave for awareness and potential follow-up in Gutenberg.
This ticket was mentioned in PR #10718 on WordPress/wordpress-develop by @youknowriad.
3 months ago
#71
## Trac ticket
## Summary
This PR improves the Gutenberg build integration to address several issues reported in the ticket. The changes simplify the developer workflow and restore a flow similar to how package dependencies worked before the Gutenberg checkout-and-build approach was introduced.
### Key Improvements
- Automatic rebuild on ref change: Adds a new
gutenberg:syncscript that follows the same pattern asinstall-changed(which WordPress already uses for npm dependencies). It stores a hash of the built ref in.gutenberg-hashand only rebuilds when the ref changes. This means whengutenberg.refin package.json changes (e.g., aftergit pull), the nextnpm run buildwill automatically detect this and rebuild Gutenberg.
- Full integration on
npm install: Updatespostinstallto run the complete checkout + build + copy flow. Runningnpm installnow produces a fully working development environment with Gutenberg assets insrc/.
- Conditional PHP file loading: Adds
file_exists()checks around the PHP files fromwp-includes/build/(routes.php and pages.php). This prevents WordPress from fatal erroring if these files don't exist yet - addressing concerns from developers who need to run PHP before the build completes.
- Clean Gutenberg checkout: Restores Gutenberg's
package.jsonafter the build completes usinggit checkout, keeping the Gutenberg directory clean and avoiding uncommitted changes.
- Simplified scripts: Removes the
gutenberg:integratenpm script andgutenberg-integrategrunt task since their functionality is now handled bygutenberg:sync.
### New Flow
npm install → gutenberg:sync (checks .gutenberg-hash, runs checkout+build if ref changed) → gutenberg:copy --dev (copies to src/) npm run build / build:dev / dev → gutenberg-sync (checks hash, rebuilds only if ref changed) → gutenberg-copy (copies to build/ or src/)
This is very similar to how things worked before with package dependencies - npm install handles everything, and subsequent builds are fast because they skip the already-built Gutenberg unless the ref has changed.
## Test instructions
- Clone the branch and run
npm install - Verify Gutenberg is checked out, built, and copied to
src/ - Run
npm run build- should skip Gutenberg rebuild ("already synced") - Change
gutenberg.refin package.json to a different commit - Run
npm run build- should detect the change and rebuild Gutenberg - Delete
src/wp-includes/build/routes.phpand verify WordPress doesn't fatal error
## Files changed
.gitignore- Added.gutenberg-hashGruntfile.js- Addedgutenberg-synctask, removedgutenberg-integratetask, updated build taskspackage.json- Updatedpostinstall, addedgutenberg:sync, removedgutenberg:integratesrc/wp-admin/font-library.php- Updated error message to referencenpm installsrc/wp-settings.php- Added conditionalfile_exists()checks for build PHP filestools/gutenberg/build-gutenberg.js- AddedrestorePackageJson()after buildtools/gutenberg/sync-gutenberg.js- New script for hash-based rebuild detection
🤖 Generated with Claude Code
@youknowriad commented on PR #10718:
3 months ago
#72
@TobiasBg has kindly accepted to help me test this PR properly and the unit tests in TablePress are now running without any changes.
#73
@
3 months ago
Here’s a pragmatic proposal: what if we introduced a weekly cadence on this one in order to make it easier to find all the places that are broken but without obstructing everything in the meantime?
- Immediately revert the entire chain of patches on this issue. This will restore
trunkto a working state. Unfortunately this involves resolving merge conflicts inGlobal Styles: Lift classic block restrictions.31e2aaabb because of indexing in$submenu, but this is the point of the revert. The more we keep adding ad-hoc patches to work around the issues the harder it is to isolate the changes and the unintended damage they are causing.
- Liberally update the build change in a separate branch. Everyone is encouraged to merge this branch into their local development and report issues on this ticket.
- Every Tuesday we merge the
build-changebranch intotrunkand see if new reports turn up. If they do, we revert the change again and attempt to fix the issues in thebuild-changebranch.
This will add a number of merge/revert commits in trunk, but under the principle of don’t break trunk it helps to prevent this change from obstructing everyone else’s ongoing work. It also means that once the change has been sufficiently vetted, tested, and corrected, then it will have a single commit where it can be analyzed as a whole. Right now it’s already getting hard to understand the scope of this change and the impacts. For example, in PR#10718 there is a change to the .gitignore file, but because it’s a single commit on a chain of changes, the diff view doesn’t show whether the relevant PHP files are still in the .gitignore or not.
I really appreciate the energy going into this to make the Gutenberg sync easier! We still have ample time before 7.0, meaning we should feel no duress to rush hastily. I want this work to continue, but I hope we do so thoughtfully, considerably, and without breaking Core (doing things like skipping security processing by default does not seem to align with those goals either).
Personally I do all my development in a long-running stacked branch which merges multiple in-development branches together, now including a revert of the entire patch-series for this change. I don’t experience any meaningful trouble by maintaining this branch and would be happy to help anyone who is worried about fixing things in a long-running side-branch so that we can have a stable and atomic commit in Core when this change is ready.
@TobiasBg commented on PR #10718:
3 months ago
#74
I've tested various iterations of this PR throughout the day and thanks to @youknowriad's adjustments was able to get my plugin's unit tests running again.
So, for the common scenario of running unit tests on wordpress-develop, the additional file_exists/class_exists checks are helpful. (Having to run npm commands between checkout and running tests would, in contrast, be ugly, due to the heavily increased built time, storage, etc.)
@youknowriad commented on PR #10718:
3 months ago
#75
due to the heavily increased built time, storage, etc.
Just want to note that the built time is actually faster than before.
@TobiasBg commented on PR #10718:
3 months ago
#76
due to the heavily increased built time, storage, etc.
Just want to note that the built time is actually faster than before.
Oh, this was meant as "increased in comparison to not having to run a build step before" (for plugin unit tests, and similar). :-)
That the build time for when a build is needed/desired is faster is a good side effect, of course!
#77
follow-up:
↓ 78
@
3 months ago
Here's a summary of issues I am seeing (repeating myself so it's all in one comment):
Technical:
- Build now includes 17 package.json files
- Build now includes 172 *.map files, most referencing files that don't exist
- Building results in Gutenberg directory being dirty, preventing subsequent updates
- install-changed doesn't include gutenberg hash in packagehash.txt, outdated dependencies aren't detected
- changes are difficult to review, as a result it's easier for Gutenberg Experiments and other unintentional changes to end up in Core (as seen with Navigation block)
- SVN ignore still hasn't been updated
- Per GB#74490 (comment) this will require more frequent changes to build tools in WP-Dev
Administratively
- The pull request was only allowed to sit for the holiday period.
- Multiple commits have been made without review approvals
- The initial commit was not tested, the build bug was available in the playground link
I think reverting creating more issues that it solves. This change is necessary for features slated for 7.0 and it's better to solve them way before alpha/beta than run to solve them at the last moment. -- Riad
Let's also see things from the other side a bit: it's a huge change that resolves a ton of friction on the Gutenberg dev side. I can't understate how much work and overhead it is every release to sync and merge the Gutenberg codebase into core. -- Ella
@youknowriad, @ellatrix
The issue I am seeing is that this is making it very difficult to do development confidently in the WordPress-Develop repository. While these changes remain in this repo in an incomplete state, it's not possible to know where an error is coming from.
I know it's complex to import changes from the Gutenberg repository, this is demonstrating the past friction in updating them. By committing in a broken state, the ease for one set of developers is at the expense of blocking another set of developers.
Blocking, or at least, significantly hindering developers is not worth the cost.
Worth noting that we're in the alpha stage and commits were made swiftly to resolve issues. It's really hard to make these changes without trying them.
As mentioned above, this is introducing a significant hinderance for the development of code in WordPress-Develop.
All of these changes could and SHOULD have been tested in a pull request.
This seems unrelated to how the code is integrated. cc @get_dave for awareness and potential follow-up in Gutenberg.
This is incorrect, the package.json file is changed in build-gutenberg.js#L108-L135.
Here’s a pragmatic proposal: what if we introduced a weekly cadence on this one in order to make it easier to find all the places that are broken but without obstructing everything in the meantime?
@dmsnell I largely agree with this but am hesitant about committing then reverting in the event something breaks. The testing can be done on a feature branch so breakages should be detected in the branch on the PR. The actions generate the build, provide a playground link and other means of testing.
I currently have a Gutenberg-Build repo that I update each day. I'm happy to do the same for the feature branch so we can test more easily.
#78
in reply to:
↑ 77
@
3 months ago
Replying to peterwilsoncc:
Here's a summary of issues I am seeing (repeating myself so it's all in one comment):
Thanks for this summary, @peterwilsoncc. I'm still trying to get up to speed on the changes here and how they work, but this was very helpful to understand the current state of things.
It was also flagged today in Slack that the distributed hosting tests appear to be broken as well. There are currently only 3 hosting bots that have reported results in the last 25 commits. It seems the commit prior to [61438] had 15 unique environments report back, but since then the WP GHA Bot has been the only one consistently reporting.
It seems that the error message described there is the same one that @TobiasBg is seeing above with TablePress.
The hosting tests are mostly a "set it and forget it" type thing that does not require any changes. Depending on how the host is implementing the test runner, they may not pull in updates to the testing script regularly. If changes are required, we will likely need to perform some outreach to the participating hosts with instructions to fix their test reporting.
#79
@
3 months ago
- Description modified (diff)
Thanks for the summary :)
Build now includes 17 package.json files
I asked about this above, how is this a problem?
Build now includes 172 *.map files, most referencing files that don't exist
This is about adding the svn ignore properties which I'm going to take care of.
Building results in Gutenberg directory being dirty, preventing subsequent updates
install-changed doesn't include gutenberg hash in packagehash.txt, outdated dependencies aren't detected
These are both addressed in the latest PR
changes are difficult to review, as a result it's easier for Gutenberg Experiments and other unintentional changes to end up in Core (as seen with Navigation block)
This is for me something that was missed in Gutenberg PR review rather than a result of this change, it can happen for any change including JS changes that were gitignored before.
Per GB#74490 (comment) this will require more frequent changes to build tools in WP-Dev
I don't understand this, this basically means that it's easier to update the build tool, while previously it was duplicated between Gutenberg and Core, now it's using the same build tool. So this is more like pro to me.
Administratively
This seems more related to how we approach iterations and work in general during alpha period. I prefer commit early and fix quickly rather than leaving a huge PR at the least moment before beta 1, but maybe better discussed separately. Something that is largely missing here and I that I personally felt was disregarded is that this is the result of months of work on the Gutenberg repository with testing on both Gutenberg repository and third-party plugins. Sure bringing to Core is another step, but it's also a step that shouldn't disregard all the iterations that happened before.
The issue I am seeing is that this is making it very difficult to do development confidently in the WordPress-Develop repository. While these changes remain in this repo in an incomplete state, it's not possible to know where an error is coming from.
Again, I'm happy to get reviews on the follow-ups and ship them. At this point I'm personally very satisfied with how this is working to be honest. Outside a couple of fixes, the rest seems like enhancements to me that I'm committing to doing in the next days.
The crux of the change: move from npm packages to checkout and build is working perfectly.
As mentioned above, this is introducing a significant hinderance for the development of code in WordPress-Develop.
I've spent a long time discussing with @dmsnell today to understand his struggles. We discussed a few things:
- The fact that the block parsing code is split between Gutenberg and Core is problem that can be solved separately by moving this code entirely to Core.
- The difference in opinion on whether vendor php code should be versioned in consumer repositories or not. It's fair to say that the practice in most php repositories is to gitignore dependencies. It's not a hill I'm willing to die on, as much as I hate seeing unrelated changes in my diffs, if you all prefer it that way, this part is something we can revert.
I've also spent some time with @TobiasBg mentioned above to understand his own issues and we've addressed these in the last PR as well, which should also hopefully fix the issues you're raising @desrosj
I'm happy to look at any other thing you consider "hinderance" that I'm not seeing right now.
This ticket was mentioned in Slack in #hosting by desrosj. View the logs.
3 months ago
#82
@
2 months ago
Quick update
Build now includes 17 package.json files
This is now solved in the Gutenberg PR addressing the original comments from @peterwilsoncc
Build now includes 172 *.map files, most referencing files that don't exist
This is also solved in the latest PR attached to this ticket.
#84
@
2 months ago
In 61472:
Indeed, the Gutenberg project may not be using hot reloading, but the @wordpress/scripts library supports the --hot parameter. I believe the --hot parameter no longer works because WordPress core no longer enqueues react-refresh-runtime and react-refresh-entry. Block developers can no longer take advantage of hot reloading unless they add their own code equivalent to the gutenberg_register_vendor_scripts() function.
#85
@
2 months ago
I see, my understanding was that the hot reloading was for core scripts, but I can take a look at wp-scripts based plugins.
#86
@
2 months ago
I did a little research and realized that there might not actually be that many plugins that use hot reload.
Search results on WP Directory Searcher: https://wpdirectory.net/search/01KEVCJPRYBY8T518FQ70MGK8T
This ticket was mentioned in PR #10725 on WordPress/wordpress-develop by @youknowriad.
2 months ago
#87
## Summary
Restores the wp_register_development_scripts() function and associated build infrastructure to enable hot module replacement (HMR) when using @wordpress/scripts with the --hot flag.
The React Refresh scripts were removed in [61438] as part of the Gutenberg build restructuring, but they are still needed for plugin developers using wp-scripts start --hot for block development.
## Changes
- Adds
react-refreshand@pmmmwh/react-refresh-webpack-pluginnpm dependencies - Creates
tools/webpack/development.jsto build the development scripts - Restores
wp_register_development_scripts()inscript-loader.php - Removes the deprecated stub from
deprecated.php
The scripts are only registered when SCRIPT_DEBUG is true and are not loaded during Core tests.
## Trac Ticket
## Test Plan
- Set
SCRIPT_DEBUGtotrueinwp-config.php - Create a plugin using
@wordpress/scripts - Run
wp-scripts start --hot - Verify hot module replacement works when editing React components
🤖 Generated with Claude Code
@youknowriad commented on PR #10725:
2 months ago
#88
Hot reloading has proven to be not very valuable in Gutenberg and WordPress in general since it's not a typical SPA. That said, this PR restores the previous behavior so we can make an informed decision about whether we still want it or not.
@youknowriad commented on PR #10725:
2 months ago
#89
I tested this with an example plugin, it works but I understand why no one really uses this, it's not the seamless experience that you get with SPAs. Anyone, we can see whether to commit or not this PR.
@youknowriad commented on PR #10725:
2 months ago
#92
@t-hamano What do you think we should do here? Maybe we can just merge this PR to restore previous status quo even if imperfect.
#93
in reply to:
↑ 65
@
2 months ago
Replying to TobiasBg:
It looks like this change is also affecting common plugin unit test setups :-(
For example, I'm checking out wordpress-develop to have the WP unit test framework available.
However, when running tests I only get
PHP Warning: require(/home/runner/work/TablePress/TablePress/src/wp-includes/build/routes.php): Failed to open stream: No such file or directory in /home/runner/work/TablePress/TablePress/src/wp-settings.php on line 245 PHP Fatal error: Uncaught Error: Failed opening required '/home/runner/work/TablePress/TablePress/src/wp-includes/build/routes.php' (include_path='.:/usr/share/php') in /home/runner/work/TablePress/TablePress/src/wp-settings.php:245 Stack trace: #0 /home/runner/work/TablePress/TablePress/tests/phpunit/includes/install.php(40): require_once() #1 {main} thrown in /home/runner/work/TablePress/TablePress/src/wp-settings.php on line 245
Just came here to report this as well. It appears that unit tests can no longer be run from the src directory in the current state, essentially breaking [50441] / #51734.
This ticket was mentioned in Slack in #hosting by jorbin. View the logs.
2 months ago
#95
@
2 months ago
Hi,
I may have missed it if someone already said these things:
The Hosting Team phpunit test runner is failing to report across most configs as of the 5th when this was merged. https://github.com/WordPress/phpunit-test-runner/issues/293
The runner is throwing the same error that Sergey mentioned. On my test environment (LAMP) it's this fatal error: PHP Fatal error: Uncaught Error: Failed opening required '/tmp/wp-test-runner/src/wp-includes/build/routes.php' (include_path='.:') in /tmp/wp-test-runner/src/wp-settings.php:245
During the build process the gutenberg-copy task is setting the build target to 'build' which it then is building out routes.php in (in /build) instead of building in /src.
Running "gutenberg-copy" task 🔍 Checking Gutenberg build... Build target: build/ ✅ Gutenberg build found 📦 Copying PHP infrastructure... ✅ routes.php ✅
"Gutenberg copy" task function code:
const buildDir = grunt.option( 'dev' ) ? 'src' : 'build';
This if else may be returning false and therefore building under 'build'. The statement appears again on line 1466 https://github.com/WordPress/wordpress-develop/blob/a8924f58aa038fcb892b55fd31d37aeac65c34e5/Gruntfile.js#L1468 (but I haven't checked this entire file).
#96
@
2 months ago
@SergeyBiryukov and @amykamala:
If you have a chance, please test https://github.com/WordPress/wordpress-develop/pull/10718 for this, which aims at keeping tests working on wordpress-develop.
@youknowriad commented on PR #10725:
2 months ago
#97
My thinking is that we should ship this PR to "fix" things for folks using the previous version of wp-scripts but I won't be against removing this entirely from wp-scripts in Gutenberg.
@youknowriad commented on PR #10725:
2 months ago
#100
This was committed.
@youknowriad commented on PR #10718:
2 months ago
#101
With the update of the build tool, we also now have prefixed functions, removed the lingering package.json files.
@youknowriad commented on PR #10718:
2 months ago
#102
@WordPress/gutenberg-core I'd appreciate a review of this PR. This unblocks folks doing unit tests without building and address all the remaining feedback on the build change. Would be good to land this one.
@youknowriad commented on PR #10718:
2 months ago
#103
though I really think we should move the block parser files to core
Agreed :)
#105
@
2 months ago
The sync seems to be broken with the latest Gutenberg trunk (237e6cabddf95ae9e6a5e173d1a0749a89107931) following the changes in https://github.com/WordPress/gutenberg/pull/74461 which added block-library/src/navigation-link/shared/helpers.php file.
Steps:
- Update the
gutenberg.refinpackage.jsonto237e6cabddf95ae9e6a5e173d1a0749a89107931 - Run
npm installto see the error
📦 Copying blocks...
❌ Unexpected error: Error: ENOTSUP: operation not supported on socket, copyfile 'wordpress-develop/gutenberg/build/scripts/block-library/navigation-link/shared' -> 'wordpress-develop/src/wp-includes/blocks/navigation-link/shared'
at Object.copyFileSync (node:fs:3085:11)
at copyBlockAssets (wordpress-develop/tools/gutenberg/copy-gutenberg-build.js:247:9)
at main (wordpress-develop/tools/gutenberg/copy-gutenberg-build.js:1029:2)
at Object.<anonymous> (wordpress-develop/tools/gutenberg/copy-gutenberg-build.js:1116:1)
at Module._compile (node:internal/modules/cjs/loader:1706:14)
at Object..js (node:internal/modules/cjs/loader:1839:10)
at Module.load (node:internal/modules/cjs/loader:1441:32)
at Function._load (node:internal/modules/cjs/loader:1263:12)
at TracingChannel.traceSync (node:diagnostics_channel:322:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:237:24) {
errno: -45,
code: 'ENOTSUP',
syscall: 'copyfile',
path: 'wordpress-develop/gutenberg/build/scripts/block-library/navigation-link/shared',
dest: 'wordpress-develop/src/wp-includes/blocks/navigation-link/shared'
}
@wildworks commented on PR #10725:
2 months ago
#106
Hot reloading doesn't seem to work.
Steps:
- Run
npm run env - Run
cd src/wp-content/plugins && npx @wordpress/create-block test-block - Run
cd test-block - Run
npm run start -- --hot - Create a new post and insert a Test Block
- Make changes to the Edit component of the test-block plugin.
- Nothing happens in the block editor.
@youknowriad commented on PR #10725:
2 months ago
#107
Indeed @t-hamano I did test this, so I'm guessing I had some weird setup. I'll push a fix.
This ticket was mentioned in PR #10757 on WordPress/wordpress-develop by @youknowriad.
2 months ago
#108
## Trac Ticket
## Summary
- Fixes hot reloading (HMR) not working for block plugins when using
@wordpress/scriptswith the--hotflag - Root cause:
react-refresh-entry.jswas bundling its own copy ofreact-refresh/runtimeinstead of usingwindow.ReactRefreshRuntime, creating two separate runtime instances - Fix: Split webpack development config into two configs - one for runtime (creates the global) and one for entry (uses it as external)
## Test plan
- Create a test block plugin:
cd src/wp-content/plugins && npx @wordpress/create-block test-block - Activate the plugin
- Start hot reload:
cd src/wp-content/plugins/test-block && npm run start -- --hot - Open the block editor and add the Test Block
- Edit
src/edit.js- changes should appear automatically without page refresh
🤖 Generated with Claude Code
@youknowriad commented on PR #10725:
2 months ago
#109
#110
@
2 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for notes in PR#10725 and comment #35 which I've been able to reproduce.
Additionally the hash check needs to take place at the start of each grunt command along with the install changed check, see Gruntfile.js#L98-L99.
For package changes, they need to be detected on each build task. I usually run npm run grunt clean:qunit to trigger it as it's nice and quick:
- Modify an npm dependency
- Run
npm run grunt clean:qunit - Observe reinstall trigger
- Modify Gutenberg hash
- Run
npm run grunt clean:qunit - Observe GB update fails.
#112
@
2 months ago
I’ve attempted to gather all of the disparate changes in dmsnell#27 and will try to keep this updated. If I miss any commits please let me know and I’ll cherry-pick them into that branch. I find this is easier to review the entirety of the change since it’s spread among different PRs and fixes.
#114
@
2 months ago
I’ve proposed in PR#10761 a revert of the changes to remove the parser from Core. Ironically, the parser files were previously being copied in the same exact way as they are in this updated build, but by webpack instead of a new generated script. This ticket did not have to change the way the parser is maintained and really should be handled in another ticket: that ticket is #64521. Perhaps it would have been less disruptive to either not address the parser question in this ticket (since it wasn’t necessary or related to the other changes) or to have made the structural changes first before attempting it.
While reviewing the changes in dmsnell#27 I noticed also that we are attempting to manually parse PHP code using regular expressions and basic parsing in JS, even though we should have PHP available. @youknowriad did you consider using PHP to evaluate the files and return a reliable JSON-serialization of their data?
#115
@
2 months ago
I’ve merged [61504] which restores the block parser in Core, fixing a change that wasn’t necessary in order to make the changes to the build system, though while the parser code is back and that should fix a number of workflows that were broken by the overall change, the version history is messed up now and not directly traversable.
It’s possible still to see the version history with git log -- src/wp-includes/class-wp-block-parser.php but going back through the blame/annotation on the file leads to a dead-end before the restoration commit.
I’ve never considered rewriting the trunk history for a project this size, used by so many in so many places, and who knows what would happen to so many forks, but it’s really confusing me to sort through the scale of this change and the many fixes, which still feel hasty. As some of this gets sorted out, I’ve been examining the collective whole change and noting a lot of curiosities in the code which raise maintenance concerns.
I’m particularly weary about all of the ad-hoc file_exists() check which skirt the issue of removing the PHP files by leaving WordPress in an unfunctional state. If we’re copying all of these files from Gutenberg, as we have been doing, why not leave them all and overwrite them during the build?
It would be trivial to detect if, after copying, there are updates from Gutenberg, and to show those or recreate the commits that changed them. This would make Core a mirror of sorts of Gutenberg changes, but would bring even more history into Core than we had before.
This ticket was mentioned in Slack in #hosting by amykamala. View the logs.
2 months ago
@youknowriad commented on PR #10757:
2 months ago
#118
This was committed
This ticket was mentioned in Slack in #core-committers by westonruter. View the logs.
2 months ago
This ticket was mentioned in PR #10826 on WordPress/wordpress-develop by @peterwilsoncc.
2 months ago
#120
Restores the workflow file .github/workflows/reusable-test-gutenberg-build-process.yml removed in 22294af4ed3b46f577f42590ba4cc19ca0a93f3f.
This file is required by the WordPress 6.9 branche for the Test Build Processes test to complete. At present these tests are silently failing. See this action run on the 6.9 branch for an example of a silent failure.
An alternative approach may be to update the workflow on the 6.9 branch to pull this file from there. Let me know what you would prefer.
Trac ticket: https://core.trac.wordpress.org/ticket/64393
This ticket was mentioned in PR #10852 on WordPress/wordpress-develop by @peterwilsoncc.
8 weeks ago
#121
[!Warning]
Do not merge, testing another PR
Trac ticket: https://core.trac.wordpress.org/ticket/64393
@peterwilsoncc commented on PR #10826:
8 weeks ago
#122
I'm running a test against the 6.9 branch in https://github.com/WordPress/wordpress-develop/pull/10852 and will merge once it completes. This may take a while. https://www.githubstatus.com/
@peterwilsoncc commented on PR #10826:
8 weeks ago
#123
The test PR appears to be working. The playground link uses the pull_request_target event so I think that's why it's not appearing, I think I'll merge and re-check.
@peterwilsoncc commented on PR #10826:
8 weeks ago
#125
This ticket was mentioned in Slack in #hosting by amykamala. View the logs.
6 weeks ago
#127
follow-up:
↓ 128
@
6 weeks ago
I'm not sure why yet, but I can't get npm install or npm run build to complete successfully. I keep getting this error:
❌ npm ci failed: npm ci failed with code 127
❌ Sync failed: node tools/gutenberg/checkout-gutenberg.js failed with code 1
I'm not sure what happened, but it was working for me previously. The process in general feels extremely slow. I'm finding I need to wait 2-3+ minutes for the command to run.
Is this something others are also experiencing?
#128
in reply to:
↑ 127
@
6 weeks ago
Replying to desrosj:
I'm not sure why yet, but I can't get
npm installornpm run buildto complete successfully. I keep getting this error:
❌ npm ci failed: npm ci failed with code 127
❌ Sync failed: node tools/gutenberg/checkout-gutenberg.js failed with code 1
I just now tested with the latest trunk and it worked fine for me. May be try deleting the .gutenberg-hash file and try again.
I'm not sure what happened, but it was working for me previously. The process in general feels extremely slow. I'm finding I need to wait 2-3+ minutes for the command to run.
Is this something others are also experiencing?
Yes, the process is actually slow when the Gutenberg hash changes. Otherwise, it's fine.
#129
@
6 weeks ago
It works fine in my environment too. I re-cloned wordpress-develop just to be sure, then ran npm install.
#130
@
6 weeks ago
My apologies in advance for sounding disheartened. I recognize that we are all striving for the same goal of making WordPress better for everyone. But I'm currently unable to work with wordpress-develop at all and I'm finding the developer experience incredibly frustrating.
It's very difficult to follow through the process to figure out where the issue actually is, what changes have been made to related code, etc. I recognize some of this is due to my own personal habits, but this is how I have been investigating.
- Figure out what the error is saying.
- Search
wordpress-developcode base for the command that is failing. - Nothing is found, so I search the Gutenberg directory.
- Find the command that's failing.
- Look up when the pinned SHA value last changed in
wordpress-develop. - Try to find changes possibly related to this error in the
./gutenbergfolder. - The clone in the
./gutenbergdirectory only has a depth of 1, so while thegit logis accessible, tools likegit blamedo not show accurate information and I'm unable to see the specific commits responsible for changing a given line or when the last time each line was modified. - I either switch to GitHub.com to look into this, or switch to the other checkout of Gutenberg I have in my
src/wp-content/pluginsdirectory. But this is separate, s now I need to make sure I havetrunkchecked out and the latest changes pulled. - The latest is not installed for the build step, a specific hash is. So I need to look for that hash in the
package.jsonfile or the.gutenberg-hashfile and check out that revision. But I deleted this file while troubleshooting, and becausenpm installfailed, it's now not there at all.
I'm also concerned that we're still finding things that are broken today.
- The distributed hosting tests are still broken for many of the hosts participating. On January 13th, I noted that there were only 3 participants that had reported results since the original changes in [61438] and 15 had been reporting prior. I checked the page periodically to make sure the number increased again, and it did return to around 12 reporting again, but today there are now only 6.
- There are reports of GitHub Actions workflows failing within private forks.
- As stated above by @johnbillion, the GitHub Action workflows that run the build script were taking 30-40% longer after the changes here. That still seems to be the case, and I also feel that locally with just
npm installtaking minutes longer than it did before.
I may be missing it, but I don't see a GitHub Actions workflow in the gutenberg repository that tests the wordpress-develop build script. I know that seems counterintuitive, but if wordpress-develop is going to run the gutenberg build script as part of its own, there should be some level of confidence that changes will not negatively affect the other repository before merging. The opposite was added to wordpress-develop (for testing the gutenberg build process within the src/wp-content/plugins directory to ensure this common contributor workflow continues to work as expected. I think that could also be done here.
In many ways, it feels like this is trying to reinvent the concept of a monorepo without many of the benefits of having an actual monorepo:
- Simplified dependency management: there are still two separate
package.jsonfiles with different dependency sets. - Atomic changes across the project: changes still need to be made in multiple locations, and coordinating these changes feels more complicated.
- Consistent tooling and standards: there are still two separate build processes. And because of how the
gutenbergrepository is pulled in, it feels very "black box". The standards-related tooling is also not the same. It's been several months, but the last time I checked there were several inconsistencies in how the coding standards rules were enforced. - Better visibility: the visibility, in my opinion, is worse with the current implementation. Many of the files that were under version control in
wordpress-developbefore are now not (block files, defaulttheme.json, etc.), so their history has disappeared, and tracking new changes requires you to know where to look withingutenberg, and you need to switch between two different repositories to find any of the history.
I'm not confident that all of the ripple effects have been discovered yet.
I'm also still not clear on a few things. For example, how am I _supposed_ to work with or contribute to the Gutenberg plugin now? My previous workflow of having the plugin checked out into the src/wp-content/plugins directory _should_ still work, but I'm not yet clear on what adjustments I need to make. I have two clones of the same repository in the same project folder that are frequently out of sync (the pinned hash intentionally lags behind).
For the next thing, I am going to use the word "backport" in the way it's historically been used (merging changes from trunk back to a branch for an older version of WordPress), not how it's being used today (to describe merging or porting code from gutenberg into wordpress-develop). I've repeatedly requested a different term be used to avoid confusion or accidentally mistaking one action for the other.
What do these changes mean in practice for instances where a change must be backported to branches that do not use this new build process? For example, a change merged into gutenberg#trunk may be backported to gutenberg#wp/6.8, gutenberg#wp/6.7, etc., and then the resulting changes are published to the npm packages before updating the respective branches of wordpress-develop. The only mention of this process I see so far is this:
We'll also need to consider whether gitignore and SVN props will need to be backported to branches receiving security updates to avoid the accidental commit of the gutenberg repo in to wp-dev.
Since this only affects trunk, I don't think there's anything needed here unless I'm missing something.
The issue here is that contributors frequently run commands like svn switch '^/branches/6.8' to work on or commit to older branches. If someone has trunk or a branch >=7.0 checked out and runs the build scripts before switching to an old branch, then any newly introduced files and directories not under version control/in .gitignore will remain present and the contributor could accidentally add them to version control.
Candidly, I am really on the fence about leaving this in. Initially it remained because there was time to work out the kinks. There still is some time, but at what point does this cross into "there is too much risk remaining and we need to try again in 7.1"? I recognize that changes were required for 7.0 features, but given the current state of things and the headaches so far, is this truly better than manually syncing certain PHP files and updating npm dependency versions as necessary?
I keep asking myself if we're handling this differently because it's not a user-facing feature. I think that we are. I can't come up with a scenario where we would ship this to users in the state it's in. When it comes to the beta/RC phases of a release cycle and branching, we are essentially shipping the current state of the build tools to our contributors (who are users too). I feel like we shouldn't treat that subset of users any differently when it comes to deciding "readiness", especially since we need the tooling to be stable and maintained within each branch long term (until security updates are dropped for that version of WordPress).
Climbing out of the rabbit holes I fell down investigating the current issue I'm experiencing, here is what I've tried and looked into so far:
rm -rf node_modulesthennpm installrm -rf node_modules gutenbergthennpm install- Triple checked that I was using the correct version of Node.js/npm.
- At one point, I noticed that hash stored within the
.gutenberg-hashfile locally did not match the one defined in thepackage.jsonfile. I updated the hashes to match and still encountered the issue. - I deleted the
.gutenberg-hashfile and thegutenbergdirectory and re-rannpm install. - I created an entirely fresh clone of
wordpress-developin a new directory and rannpm install.
I don't know how, but I've somehow been able to change the error message. I now see the following:
sh: patch-package: command not found npm error code 127 npm error path /path/to/checkout/wordpress-develop/gutenberg npm error command failed npm error command sh -c patch-package && node ./patches/patch-xcode.js
I took a look, and the patch-package dependency is not installed in my node_module folder.
I looked into what the command does, and it seems to apply all of the patches in the the patches directory. There is a README file in that directory that explains each patch, but there's no description for the lighthouse-related one, one is react native-related (which should not be relevant to wordpress-develop, and the RN work has been halted for many months now), and the patch-xcode.js file with no documentation. Looking at the PR that introduced it, this also seems related to React Native, so probably not relevant to the wordpress-develop. I suspect there are many more files, steps, and functions within the Gutenberg build process that are needlessly running within wordpress-develop.
The package is listed in the devDependencies group. If I remove the postinstall script from the package.json file, I get the same error but instead about husky (also in devDependencies).
I have to stop there for now, but the final concern I'll raise here is a realization I just had while trying to wrap this up. If a Build/Test Tool component maintainer is having this much trouble just getting started, what will the experience be like for a new contributor?
#131
@
6 weeks ago
Did some more testing on this. I decided to just run npm install -g for every package that gave me an error for the time being because I need to get my local environment back up and running. So far I've installed:
patch-packagehuskyconcurrentlycore-js-builder(was not listed in thepackage.jsonfile for Gutenberg, but was in the lock file and thebabel-preset-default/package.jsonfile)esbuild-esm-loader(also only specified in the lock Gutenberg file, but defined within thetheme/package.jsonfile)
I cannot get past Cannot find module 'core-js-builder' and Cannot find package 'esbuild-esm-loader' warnings now, though.
📦 Building workspaces...
[1] npm run build:php exited with code 0
[0] npm run build:js exited with code 0
Command errors:
node:internal/modules/cjs/loader:1210
throw err;
^
Error: Cannot find module 'core-js-builder'
Require stack:
- /path/to/checkout/wordpress-develop/gutenberg/packages/babel-preset-default/bin/index.js
at Module._resolveFilename (node:internal/modules/cjs/loader:1207:15)
at Module._load (node:internal/modules/cjs/loader:1038:27)
at Module.require (node:internal/modules/cjs/loader:1289:19)
at require (node:internal/modules/helpers:182:18)
at Object.<anonymous> (/path/to/checkout/wordpress-develop/gutenberg/packages/babel-preset-default/bin/index.js:6:17)
at Module._compile (node:internal/modules/cjs/loader:1521:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1623:10)
at Module.load (node:internal/modules/cjs/loader:1266:32)
at Module._load (node:internal/modules/cjs/loader:1091:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:164:12) {
code: 'MODULE_NOT_FOUND',
requireStack: [
'/path/to/checkout/wordpress-develop/gutenberg/packages/babel-preset-default/bin/index.js'
]
}
Node.js v20.20.0
node:internal/modules/esm/resolve:873
throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
^
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'esbuild-esm-loader' imported from /path/to/checkout/wordpress-develop/gutenberg/packages/theme/
at packageResolve (node:internal/modules/esm/resolve:873:9)
at moduleResolve (node:internal/modules/esm/resolve:946:18)
at defaultResolve (node:internal/modules/esm/resolve:1188:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:708:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:657:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:640:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:264:38)
at ModuleLoader.import (node:internal/modules/esm/loader:605:34)
at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:112:36)
at runEntryPointWithESMLoader (node:internal/modules/run_main:141:19) {
code: 'ERR_MODULE_NOT_FOUND'
}
Node.js v20.20.0
❌ Build failed: Command failed: npm run --if-present --workspaces --silent build
❌ Build failed: npm run build -- --fast --base-url=includes_url( 'build' ) failed with code 1
🔄 Restoring Gutenberg package.json...
✅ package.json restored
❌ Unexpected error: Error: npm run build -- --fast --base-url=includes_url( 'build' ) failed with code 1
at ChildProcess.<anonymous> (/path/to/checkout/wordpress-develop/tools/gutenberg/build-gutenberg.js:66:6)
at ChildProcess.emit (node:events:524:28)
at maybeClose (node:internal/child_process:1104:16)
at Socket.<anonymous> (node:internal/child_process:456:11)
at Socket.emit (node:events:524:28)
at Pipe.<anonymous> (node:net:343:12)
❌ Sync failed: node tools/gutenberg/build-gutenberg.js failed with code 1
npm error code 1
npm error path /path/to/checkout/wordpress-develop
npm error command failed
npm error command sh -c npm run gutenberg:sync && npm run gutenberg:copy -- --dev
#132
@
6 weeks ago
I think I figured out the specific issue on my machine.
It seems that NODE_ENV was somehow being set to production and --omit=dev was being configured. This was being caused by some combination of:
- Installing pnpm
corepackbeing installed globally whenpnpmwas installed using Homebrew.- iTerm2's session restoration.
I installed pnpm ~1-2 weeks ago to test Gutenberg-74689 and something seems to have set NODE_ENV to production during the installation, and that environment variable was persisting due to iTerm2's feature.
Running this in Terminal (not iTerm) seems to have fixed the issue for me:
rm -rf ~/Library/Application\ Support/iTerm2/SavedState/ defaults delete com.googlecode.iterm2 "PerWindowSavedState" 2>/dev/null
This ticket was mentioned in PR #10971 on WordPress/wordpress-develop by @desrosj.
5 weeks ago
#133
This adds caching for the /gutenberg directory in GitHub Actions workflows in an attempt to avoid every single job from performing a full checkout, install, and build of the Gutenberg plugin at the pinned SHA reference.
Trac ticket: Core-64393.
## Use of AI Tools
#134
follow-up:
↓ 141
@
5 weeks ago
It's very difficult to follow through the process to figure out where the issue actually is, what changes have been made to related code, etc. I recognize some of this is due to my own personal habits, but this is how I have been investigating.
Wasn't this the same previously for all GB related code? We'd have to search both repos?
The distributed hosting tests are still broken for many of the hosts participating.
Could you link to these? It's hard to understand what's happening without specifics.
There are reports of GitHub Actions workflows failing within private forks.
Could you also link to these reports to understand the issues?
GitHub Action workflows that run the build script were taking 30-40% longer after the changes here.
Is that fixed since [61673]?
Atomic changes across the project: changes still need to be made in multiple locations, and coordinating these changes feels more complicated.
Syncing Gutenberg is now much easier without the need for publishing packages, so I'm not sure how coordinating changes is now more complicated?
Consistent tooling and standards
Better visibility
What's different here from before? We didn't have these benefits of a monorepo before either? A huge amount of files were not version controlled. There's only a tiny fraction of files that have changed to be no longer version controlled in core.
#135
@
5 weeks ago
On the helphub pages for each version, the packages and their current version is listed. See: https://wordpress.org/documentation/wordpress-version/version-6-9-1/#list-of-packages-revised
This data has historically come from package.json and a diff can be done with the last released version to see what has changed. How can this data be generated from the new build process?
@desrosj commented on PR #10971:
5 weeks ago
#137
Props to @westonruter for the idea to include the tools/gutenberg directory in the cache key.
#138
@
5 weeks ago
This data has historically come from package.json and a diff can be done with the last released version to see what has changed. How can this data be generated from the new build process?
Maybe @youknowriad can correct me if I'm wrong, but we don't use these packages anymore, so these mentions can be removed? It's pretty much the state we were in before the first version of Gutenberg landed in core with packages, and we directly include and build scripts. Perhaps instead we can point to the current commit hash and the relevant Gutenberg branch that was used (wp/x.x), if that's useful at all.
@jorbin commented on PR #10971:
5 weeks ago
#139
Would it make sense to have a new reusable workflow so that the same code doesn't need to be maintained in 9 different places?
#140
@
5 weeks ago
Thanks for the suggestion of removing pnmp, @desrosj.
Unfortunately I tried that locally and am still getting hash differences when running npm run build compared to https://core.trac.wordpress.org/, etc. I've confirmed I'm using the same node version as used on the build server. I've checked on the 6.9 branch and they match the build server hashes.
A difficulty I'm having with the multiple scripts is that it's no longer clear where I should be checking code, I attempted to add a bunch of NODE_ENV defintions to the GB package file but they get wiped out during the build process.
#141
in reply to:
↑ 134
@
5 weeks ago
Replying to ellatrix:
It's very difficult to follow through the process to figure out where the issue actually is, what changes have been made to related code, etc. I recognize some of this is due to my own personal habits, but this is how I have been investigating.
Wasn't this the same previously for all GB related code? We'd have to search both repos?
The build processes were separate in the past and it was much more clear which one encountered a specific problem. Now the one in wordpress-develop calls the one from the Gutenberg repository and it can be difficult to track down where error messages originate from.
I'm going to concede this point because there are other issues that are more concerning so far.
The distributed hosting tests are still broken for many of the hosts participating.
Could you link to these? It's hard to understand what's happening without specifics.
The distributed host tests are found here. It's hard to know why a number of hosts stopped reporting without performing direct outreach because error logs don't send, only the PHPUnit output for completed runs. But at one point the WP.org GitHub Actions runner was the only one reporting for a number of commits and there is a clear drop off immediately after [61438].
Looking at [61418] (which I chose because there was a 12-24 hour gap between this one and [61419] and some bots run daily or once every several hours), only 5 of the 14 hosts that reported for that changeset have reported results for any of the 25 most recent commits. Looking at the changesets following [61438], there were many where only the w.org GH bot reported. While some have recovered, it's concerning that many of the bots reporting prior to the build scripts changes have not.
There are reports of GitHub Actions workflows failing within private forks.
Could you also link to these reports to understand the issues?
I cannot share links publicly because of their private nature. But the specific errors that I am aware of seem to have been resolved after [61673]. Will continue monitoring this.
GitHub Action workflows that run the build script were taking 30-40% longer after the changes here.
Is that fixed since [61673]?
There is not enough data to know for sure yet. But I went back and looked at the data for two time periods and compared each workflow:
- December 1, 2025 through January 4, 2026
- January 6, 2026 through February 15, 2026
The GitHub Actions performance metrics page presents an average run time metric, number of workflow runs, and a percentage of failures. I only looked at the average run time for this and only for workflows that run npm install or npm ci. By comparison, here is the percentage change in average run time:
coding-standards.yml: +99.42%end-to-end-tests.yml: +16.18%javascript-tests.yml: +2.53% (negligible)local-docker-environment.yml: +55.33%performance.yml: +5.35%phpunit-tests.yml: +17.32%test-build-processes.yml: -41.14% (I am uncertain why this decreased so drastically)test-coverage.yml: +4.6%upgrade-develop-testing.yml: +8.86%
I have the data in a spreadsheet for viewing. These metrics are not 100% because I believe the average run time includes runs that fail early.
I'm looking into caching the gutenberg directory in GHA workflows, but I'm not seeing any meaningful change in the overall runtime so far.
Consistent tooling and standards
Better visibility
What's different here from before? We didn't have these benefits of a monorepo before either? A huge amount of files were not version controlled. There's only a tiny fraction of files that have changed to be no longer version controlled in core.
The version history has now ended for the files added to .gitignore where previously you could track every single change made to block files (one example) made in the context of one repository (wordpress-develop or gutenberg). Now the only way to track the progression of a file's history is to find when the refs changed and then compare the files in question in the gutenberg repository.
I've noticed a new issue over the last 24 hours and I don't have an explanation for it. I was looking at updating developer dependencies for the release as part of #64230. The typical process I follow is to:
- Apply the available updates that look safe and reasonable according to the changelogs/release notes.
- Push a PR to run the automated testing.
- If that passes, compare the contents of the
builddirectory locally after runningnpm buildwith that of themasterbranch of core.svn.wordpress.org (mirrored to GitHub as WordPress/WordPress) - There should be no unexpected changes found in the comparison.
However, I'm finding that even when no changes are made to the actual files, I'm seeing the file hashes changing and most of the related build files are changing as a result. I've been unable to figure out what is causing those changes, or why the build server is not doing the same with every commit.
#142
@
5 weeks ago
It's hard to know why a number of hosts stopped reporting without performing direct outreach because error logs don't send, only the PHPUnit output for completed runs.
Couldn't other commits cause similar issues when there are changes to the build process? Could we change it so that error logs are included? Could we figure out what the errors are? Do you have any suspicions on what the issue could be? I'm entirely unfamiliar with this area.
The GitHub Actions performance
Could we measure again after the changset that skips the types? The performance issue sounds very important to you and I get that, we should definitely try to improve these as much as we can. We actually raised performance issues with the Github Actions in the last release due to a huge amount of actions for a number of different environments, but that was subsequently reverted? And If we actually merged both repositories, wouldn't the amount of additional code and additional tests cause an increase for the wordpress-develop repository?
Looking at the one test that went up by 100% (coding-standards.yml), why did the new build process affect that? Is it necessary to build Gutenberg to run that test?
The version history has now ended for the files added to .gitignore where previously you could track every single change made to block files (one example) made in the context of one repository
I believe only block PHP files were tracked, not the block JS. And that's just blocks, most of the code coming from Gutenberg was not tracked at all. Only a tiny fraction of those files ended up being tracked in the WordPress repo. I also think that the way we sync Gutenberg does not affect this. Versioning or not versioning specific files seems like a separate discussion to me.
#143
@
5 weeks ago
Build changes discussion doc: https://docs.google.com/document/d/1spMXW3GxMtvyert4fEExEO5eBiefEmJwDyuBhhUA7TQ/edit?tab=t.0 . As mentioned during yesterday's dev chat: https://wordpress.slack.com/archives/C02RQBWTW/p1771432793776409.
This ticket was mentioned in PR #11019 on WordPress/wordpress-develop by @desrosj.
5 weeks ago
#145
This is a work in progress.
This PR attempts to remove the need to checkout the WordPress/gutenberg repository locally by instead downloading a copy of the built plugin from the GitHub Container Registry for the respective pinned hash value.
Trac ticket: Core-64393.
## Use of AI Tools
I used Claude Code to make some of the adjustments to the Gruntfile.js file and tools/gutenberg folder.
@desrosj commented on PR #11019:
5 weeks ago
#146
There is one remaining issue to figure out: how to map the icons package correctly from the built plugin.
I've removed the chunk of code responsible for mapping those files to their respective locations within wordpress-develop and the build process succeeds.
#147
@
5 weeks ago
Couldn't other commits cause similar issues when there are changes to the build process? Could we change it so that error logs are included? Could we figure out what the errors are? Do you have any suspicions on what the issue could be? I'm entirely unfamiliar with this area.
It's always possible that changes to the build process can adversely effect the hosting tests. But ideally that is considered before making changes, or participants can be made aware of any changes required on their end to continue reporting. This tool is not perfect by any means, and adding some reporting when errors happen before reaching the phpunit command is a good idea. Without that though, we can only use the number of bots that report to determine when problems were introduced.
Could we measure again after the changset that skips the types?
Yes, I intend to measure that once there's enough data to perform a fair comparison. At the time of my last comment, there was data for less than 24 hours. I was planning to come back after a full week.
The performance issue sounds very important to you and I get that, we should definitely try to improve these as much as we can.
There are 4 variables that are very important to consider and balance when it comes to making any changes to the project's build scripts and test workflows: completeness, speed, volume, developer experience.
- Completeness: we should be confident that enough scenarios and combinations are tested to promote a strong level of confidence in any change. This mainly surfaces in two ways: the number of variable combinations (PHP/database versions, etc.), and the sheer number of paths tested (code coverage).
- Volume: The workflows maintained across the project should be able to recover from drastic increases in contributor activity (leading up to beta 1, during a Contributor Day, etc.) in a reasonable amount of time.
- Speed: the build scripts and GitHub Actions workflows (both for automating tasks and running test suites) should be as fast as they can be.
- Developer experience: the scripts should have as little friction and be as disruptive as possible so that contributors can get work done and others can be easily on-boarded.
For GitHub Actions, the WP organization has a quota of 500 concurrent jobs at any given time. This is shared across all organization repositories, not just the current one. The organization also receives a quota of free action minutes monthly. While public repositories do not consume these minutes (all public repositories are free), private repositories and private mirrors do. A meaningful increase in workflow runtime can result in a large bill.
I just wanted to share this context because it's important to understand when trying to explain why these changes have been so problematic.
We actually raised performance issues with the Github Actions in the last release due to a huge amount of actions for a number of different environments, but that was subsequently reverted?
The issues that were flagged during the previous release were caused because the volume that occurred was not handled reasonably and a large queue resulted due to the influx of activity. However, the solution that was implemented sacrificed completeness and resulted in a lower level of confidence in the changes due to fewer supported combinations being tested.
There is ongoing work to create a new suggested list of combinations for wordpress-develop so that the volume of jobs triggered by a given commit or PR can be trimmed down without sacrificing completeness (there is a lot of overlap, but it should be done intentionally). There is also some exploration happening to remove the Docker requirement for running PHPUnit tests with the hope that the unit test suite will run faster overall, limiting the overall time it takes to run within each test job.
The Build/Test Tool maintainers invest a lot of time and effort into ensuring these factors and outcomes are balanced fairly. However, based on the collective feedback on this ticket so far, all of these variables seem to have regressed considerably except completeness. I understand that there are several other issues being solved as a part of the changes here and I don't want to discount those. But the net cost of the solution in place right now is clearly negative.
I appreciate the willingness to fix the performance issues. There seemed to have been good engagement with feedback immediately after [61438]. But over the last ~5 weeks, it has felt like the feedback given has been taken less and less seriously and instead just continuing to "plow ahead".
And If we actually merged both repositories, wouldn't the amount of additional code and additional tests cause an increase for the wordpress-develop repository?
I think it would be the opposite. If there were a true monorepo, there would be one build script running, not two. This would mean that the overall volume of jobs would be nearly cut in half. Running unit tests, E2E tests, etc. would also likely take less time overall because instead of two separate workflows with several jobs running a build script before running tests, it would just be one workflow running the same set of combinations once.
Looking at the one test that went up by 100% (coding-standards.yml), why did the new build process affect that? Is it necessary to build Gutenberg to run that test?
There are two jobs in the coding standards workflow: one that runs PHPCS, and one that runs JSHint. Before [61438] the build script was not necessary and did not run at all, only npm ci. However after the changes in [61438], the postinstall script runs the following:
- Checks out the
gutenbergrepository at the specified commit hash. - Runs the
npm cicommand within thegutenbergfolder. - Runs the entire
gutenbergrepository build script. - Copies the results of the build to the correct locations within
wordpress-develop.
Given the unwillingness to consider reverting so far, I've tried to think of some alternate ways to solve the problem outlined in the ticket description: making it easier to merge PHP changes from the gutenberg repository to wordpress-develop. It's still not 100% clear to me how some parts of the changes on this ticket are required to accomplish that goal (such as removing packages as npm dependencies), what other problems additional changes aim to solve, or what the actual requirements for 7.0 are.
I'm not comfortable with leaving the build script as is for this release. But I've come up with a proof of concept that I think simplifies everything greatly, and if others agree, it could be a reasonable compromise to consider.
In my POC, I've removed the requirement to perform a git checkout or git clone and running the gutenberg build process is no longer necessary. The build process already runs for the Gutenberg repository several times within GHA workflows. In PR-75844 for the Gutenberg repository, the Build Gutenberg Plugin Zip workflow will now publish the built plugin to the GitHub Container registry tagged by the commit hash on push events. In PR-11019, the build scripts have been updated to pull down and extract this zip file before copying the files to the appropriate locations.
There is one outstanding bug: the WordPress icons library files are not the same within the built plugin file as they were in the cloned repository. The mappings need to be updated for this. If I comment out the code responsible for copying these files, the build process is successful.
This POC at least solves some the remaining challenges outlined in the Google Doc so far:
- This essentially reverses the performance problems, even on the first
npm install(the 2 cURL requests does take extra time, but nowhere near the amount of time the current process does). - It avoids running a build process within a build process where one is obfuscated and harder to find.
It possibly solves (testing needed):
- The issues hosting test reporters (to be seen)
- The issues with inconsistent hashes of generated script assets.
It does not solve:
- The broken contributor workflows, such as cloning the
gutenbergrepository tosrc/wp-includes/plugins/gutenberg. - Interacting with source history for many required files is still broken, affecting existing workflows.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
5 weeks ago
This ticket was mentioned in PR #11024 on WordPress/wordpress-develop by @dmsnell.
5 weeks ago
#149
Trac ticket: Core-64393
When [61438] changed how the Gutenberg build was merged into Core, it removed all of the existing files which exist on both sides of the project.
The goal of this additional change was to force a single source of truth for those files. Unfortunately, there is considerable value in having those files in the wordpress-develop repo, even though from time to time there are minor inconveniences from having them exist in both places.
- Comparing how the files changed from one release to the next.
- Debugging changes and bisecting project history to discover where a bug appeared.
- Checking out different commits without network access, without downloading almost 2 GB of data, without spending minutes waiting for files to build and still being able to use WordPress on a local checkout.
- Benchmarking along various commits.
Thankfully, the change was entirely unnecessary to accomplish the goal in the Trac ticket, so reverting the deletion of all of these files, and removing the .gitignore entires, leaves everything about the build change in place while mitigating the long-term impact on the project history.
Unfortunately, most git tooling will forever fail when looking at the history of these files. There is no way to restore the continuity of changes over time once they have been deleted and added to .gitignore, so this is a disappointing mitigation at best, but if it merges into 7.0 then at least we will be able to review all of the changes from one version to the next, with a command like git diff 6.9.1..7.0 -- src/wp-includes/blocks
#150
@
5 weeks ago
I’ve created PR#11024 to add the deleted and .gitignored files back in. They were added into this build change even though they are a separate issue, worth a separate ticket, worth discussion.
Restoring the files does not repair the repository history, but it will restore the future history and make it possible for comparison across versions. For this reason I find it is important to get this right and merged before 7.0 because otherwise it will not be possible to review the total changes from 6.9 to 7.0.
Restoring these files makes it possible once again to operate WordPress without running the build step on every checkout, which can save hours or days or time when traversing the history, bisecting to debug issues, benchmarking, and understanding how the project has changed and when.
@dmsnell commented on PR #11024:
5 weeks ago
#151
blocked by WPCS. @youknowriad can you advise here? I don’t understand the role of the removed script loader files but I think they are tripping up when trying to re-add them
@desrosj commented on PR #11019:
5 weeks ago
#152
It seems that the icons are not currently included in the Gutenberg plugin, only in wordpress-develop.
@mcsf I see you introduced this in https://github.com/WordPress/wordpress-develop/commit/5b7a3ad163b5b5aad890c1e7098d746bccf1012d. Could you share a bit more context as to why these are not yet included in the plugin?
@youknowriad commented on PR #11024:
5 weeks ago
#153
Personally, I think this PR doesn't make sense.
Comparing how the files changed from one release to the next.
This can be done on WordPress/WordPress repo if needed.
Debugging changes and bisecting project history to discover where a bug appeared.
This can still be done, it will result in the "hash" upgrade that introduced the issue, which mean you'll have the exact Gutenberg hash used in Core where the issue started happening. If you want to go deeper, you can go into gutenberg, check the changelog or bisect there. Exactly like we do with any npm dependency version upgrade
Checking out different commits without network access, without downloading almost 2 GB of data, without spending minutes waiting for files to build and still being able to use WordPress on a local checkout.
The same thing can be said for any npm version change, there's no difference with keeping these files or not, half of the files was still not in Core before the change.
---
Personally, I think seeing these files changes in PRs is a problem, it hides the purpose of the PR (upgrade a dependency) with additional changes that implementation details. When you update a npm or composer dependency, you're not going to look at all the files changed that happened in that dependency.
@youknowriad commented on PR #11024:
5 weeks ago
#154
The issue we're seeing in some of the failures on this PR is also one of the things that I think shows why the addition of these files to .gitignore is the right move.
Basically we're forced to ensure that whatever is committed is exactly the same files that are generated when npm install runs, so running npm install shouldn't result in a git diff. Right now in this PR it does because the formatting of some of these files that you committed doesn't match the expectations of the php unit tests.
---
I can't tell what all the other failures are about.
@mcsf commented on PR #11019:
5 weeks ago
#155
It seems that the icons are not currently included in the Gutenberg plugin, only in
wordpress-develop.
@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.
Oof, I'm so glad you pinged me! Fix at https://github.com/WordPress/gutenberg/pull/75866
@dmsnell commented on PR #11024:
5 weeks ago
#156
thanks for your help @youknowriad — I do understand your view here; I’m just trying to preserve the changes you wanted while restoring a collection of broken flows.
the presence of these files should not break anything, unlike how their removal has.
Basically we're forced to ensure that whatever is committed is exactly the same files that are generated when npm install runs, so running npm install shouldn't result in a git diff.
And yeah, this is an additional guard that will/would alert us to any changes that occurred if we do get a diff from the process.
I will see if maybe I hit some issue which caused the formatting to be off.
#157
@
5 weeks ago
I created #64716 as a follow-up ticket to not draw away from the main discussion here, but wanted to mention it to connect the two tickets as related.
I noticed a few additional problems today that are pretty serious from a maintainability and extensibility stand point.
First, the page-wp-admin.php file is being built into the wp-includes directory. This is the wrong context, it should be placed into wp-admin. I haven't audited all of the files being built and included after the changes here, but there are likely others.
Is it possible to direct these files to the right location? Or does the build package/routes require them to be shipped within wp-includes?
I also noticed that many of the functions in the built files are wrapped around function_exists() checks. While this makes sense within the Gutenberg plugin, this does not make sense within wordpress-develop. In the past, this was known as a "pluggable" functions, and adding new pluggable functions has long been prohibited for several reasons:
- They are inflexible by nature as multiple people cannot alter the function at the same time. The first one loaded "wins".
- "Plugged" versions cannot be trusted to maintain the intended core behavior.
- The quality of a plugged function is unknown and therefor is unreliable.
- It's very difficult to reliably add support for new features within a pluggable feature because there's no guarantee that any plugged function will do the same.
- It creates significant technical debt. Once the function is shipped, it's nearly impossible to change the signature because plugged versions of the function will not be updated reliably.
I know that the intention is not for these functions to be recognized as pluggable, but they essentially are because of the function_exists() wrappers. This introduces a ton of possibilities for poor and fragmented user experiences.
comment:14:ticket:761 has some context about the related problems, as well as #8833, #11388, #18405.
Next, I was looking at the wp_font_library_wp_admin_enqueue_scripts() function locally and was trying to find where it was maintained. A search came up with nothing in Gutenberg. It does note in the file header that the file is auto-generated and changes should be made elsewhere, but there's no guidance as to where.
After trying different substrings of the function name, I finally located the source code in a .php.template file.
This is a significant problem for a few reasons. The first is that someone contributing to WordPress through wordpress-develop cannot easily find where the source code is maintained. Even searching the function name shows no results.
The second is that to my knowledge, every one of the PHP coding standards and analysis tools that the project currently uses only scan files ending with the .php. This includes:
- PHPCS scanning enforcing coding standards
- PHPCBF for auto-fixing PHP coding standards issues that can be accurately fixed.
- PHPCS scanning to detect potential PHP compatibility issues
- (new in 7.0) PHPStan static analysis.
These tools cannot process non-PHP files because tokens and substitution syntax will cause parse errors. If the built files were included in version control here in wordpress-develop, then the scanning could process them. But that doesn't prevent issues from being merged into the template upstream in gutenberg. They would only be spotted when the SHA hash is bumped in this repository.
It does seem like there are inline phpcs:ignore statements in the code, and the directory is not excluded in the phpcs.xml.dist file. So the built files will be scanned if the build script has been run. But it's not obvious that npm install && npm run build:dev need to be run before running PHP-related scans. The GitHub Actions workflows in Core running PHPCS do not currently run those commands, so the files are not currently being scanned and validated.
When the build process has not been run, this also breaks many other essential development features standard in IDEs:
- Code navigation
- Code completion/autocomplete
- Parameter hinting
- Auto docblock generation
- Quick Documentation/IntelliSense (showing the function's documentation on hover)
- Real-time error detection
- Refactoring tools like safe rename/replace.
- Breadcrumbs showing the path to the function
Yes, someone contributing should be expected to run the build script first. But some of these features are supported by GitHub through the browser, and that will always be broken because the repository will never be aware of the built source files.
Next new concern I have is the way that scripts and styles are being registered and enqueued. In wp_font_library_wp_admin_enqueue_scripts(), the function returns early if the current page is not the expected font library. This means that WordPress is not always aware of the fact that the related scripts and styles are available.
What if someone wanted to extend one of these scripts, or load the font library experience within a custom admin screen? That's not possible given the current code. Perhaps the get_current_screen() results could be manipulated in some way to force this to run, but that's not ideal and would likely cause many side effects.
these files are maintained in Gutenberg, and the source of truth for them is there.
The deeper I dig into these changes to try and suggest ways to improve them, the more I strongly disagree with this statement from earlier on in the ticket.
Until now, JavaScript files have been maintained on the Gutenberg repository (because they are published to npm) but the PHP files have been iterated on within the Gutenberg repository. The gutenberg repo is shipped as the Gutenberg plugin, which is meant to allow users to receive newer features faster to test. But once PHP code is merged into wordpress-develop, it should almost entirely be removed from gutenberg.
This also helps to test the extensibility aspects of the code. If the plugin can't modify something, then no one can and that should be improved.
I recognize the amount of manual work merging PHP changes from gutenberg to wordpress-develop requires, but I strongly disagree that this attempt to reduce that friction is the right solution as it's created many more problems than it has solved. There is a genuine desire to collaborate to a better solution, but the path to that is not clear to me right now without everyone stepping back, getting on the same page, and regrouping during 7.1.
#158
follow-up:
↓ 167
@
5 weeks ago
First, the page-wp-admin.php file is being built into the wp-includes directory. This is the wrong context, it should be placed into wp-admin. I haven't audited all of the files being built and included after the changes here, but there are likely others.
Is it possible to direct these files to the right location? Or does the build package/routes require them to be shipped within wp-includes?
If you think about this from how WP has always been organized, I'd agree with you but the important bit here is that we're introducing the idea of a build tool that generates code for us and for the ideal DevX we shouldn't care where the files are added or organized at all. The important part is that we should be able to load build/build.php and that's all we need for our routes, scripts, modules (and more later) to be registered properly at the right moment, with the right performance... Removing all boilerplate and often hard to pick integration points.
So moving some parts of the build to a folder, and another to another folder is IMO the wrong solution, I wouldn't mind moving the build folder to src/wp-build or something but that feels like a bigger departure. We can think of this folder as the "includes" necessary to build and register WP-Admin pages but not necessary the admin pages themselves. The font-library.php file is still outside and it calls these includes.
function_exists
I don't recall the details here, it's has been there for months now. My guess though is that these are not needed in any context because right now we have a unique "prefix" per wp-build usage and these just serve as extra safety. We can try removing them I think.
On the output php not matching expectations
I think we can make a lot of improvements there, I'd love to have had help in Gutenberg when I was working on this but didn't get enough support at that point. What I would say though is that we should be looking and analyzing these files differently from the file we write manually. the same the output of webpack or esbuild or any build tool is not something you review manually when you use these in your own projects. For instance, we may have a rule to only use "const" in our JS files but the build tool might optimize and produce files that only use var or hoist or whatever. what I'm trying to say is that for generated files, the expectations are different because the goal is different. There's no expectation that one person is going to write and maintain all of this code manually (there is for templates) but the goal is to make the most optimized code first.
That said, I do think there's a lot to improve there and please feel free to do so.
Next new concern I have is the way that scripts and styles are being registered and enqueued. In wp_font_library_wp_admin_enqueue_scripts(), the function returns early if the current page is not the expected font library. This means that WordPress is not always aware of the fact that the related scripts and styles are available.
This is wrong, the scripts are always registered, they are only "enqueued" on the required pages.
What if someone wanted to extend one of these scripts, or load the font library experience within a custom admin screen? That's not possible given the current code. Perhaps the get_current_screen() results could be manipulated in some way to force this to run, but that's not ideal and would likely cause many side effects.
Yes, this is not possible and not a goal we were building for. We want plugins to be able to add pages, remove pages, replace pages but not use the content of a page in another page.
Until now, JavaScript files have been maintained on the Gutenberg repository (because they are published to npm)
This is not the reason for this, maybe it's something to clarify but for me the reason has always been and iteration on the gutenberg plugin, shipping quicker and Github over Trac. Integration over npm or git submodules or else is side effect. In fact I do recall early on 5.0 merge process that @azaozz has tried to change from npm to submodule (exactly like we did here) but that experiment concluded for a reason that I don't recall.
@youknowriad commented on PR #11024:
5 weeks ago
#159
the presence of these files should not break anything, unlike how their removal has.
I'd say that the presence of these files break the flow of making PRs on WordPress develop for Gutenberg updates. It results in huge PRs that are mostly generated files that don't need to be part of my PR. The same way when I update an npm package, I don't want to see the diff on that npm package in my PR.
I don't understand why we have two different flows for the same concept depending on whether it's an "npm package" or a "git package".
#160
@
5 weeks ago
we keep mentioning “generated files” but aren’t pretty much all of the PHP files except for a couple manifest files hand-written and maintained?
I don’t mind quarantining built files, but the first breakage I noticed was the block parser, something absolutely not auto-generated.
or should we step back and move all PHP out of Gutenberg and back into wordpress-develop? I think that would solve a lot of the problems. with one source of truth, the PHP repo would be the end-all.
last night, after a lot of hassle getting the documentation at https://developer.wordpress.org/reference restored (it had been stale for over two years), we realized that the build changes break the docs-generation again and we will not be able to produce updates for 7.0 until we discover how to fix the build process, which is another unknown since nobody has worked in these systems since at least August. 2023.
it could be that any file on the Gutenberg side just disappears from the docs, or it could be that those files don’t receive updates. it’s been a mess for unrelated reasons, but this makes the mess that much more deflating
solving the mess would have been significantly harder and took much more time because I was trying to bisect the changes to find where various problems appeared while parsing. all of the new breakage has come from the Gutenberg side, so having those extra few minutes to download, install, build, and then test while bisecting will be much more heinous in the future. at least having potentially-stale copies of those files in Core dramatically cuts down the time to find the range of commits in Gutenberg where bugs were introduced.
#162
@
5 weeks ago
we keep mentioning “generated files” but aren’t pretty much all of the PHP files except for a couple manifest files hand-written and maintained?
I don’t mind quarantining built files, but the first breakage I noticed was the block parser, something absolutely not auto-generated.
1- Just to make things clear. I'm all for moving as much PHP to Core from Gutenberg.
2- There are some php code and files that is intrinsically related to JS code, scripts, modules, routes and styles registration. All of this code is "glue" code. It's not really important in Core and it doesn't make sense to live on its own. For this kind of php file, I think it doesn't make sense to live separate from the JS files... and I don't think make sense to write manually either. This is code that is declaratively extracted from configuration and conventions. If we resort to manual code here, it's just error prone for no real benefit.
3- I'm also all in favor of unifying Gutenberg and Core repository under one repository. It's a decision that I have no say in and would be happy to support when/if it happens. My understanding is the main blocker is probably trac/Github and processes.
#163
@
5 weeks ago
@youknowriad of the 98 PHP files I see removed, it looks like 14 were auto-generated, and that accounts for seven blocks because there’s a view.asset.php file and a view.min.asset.php file.
are we in agreement then to restore the 84 (86%) of the files that are normal PHP files, mostly block code that’s required for WordPress to boot and which is hand-maintained?
I wouldn’t have any problem version-tracking the asset files, and we could .gitignore those or even better, perhaps, mark those as binary files to short-circuit the extra review stage when combining updates from the projects.
That would have been a great way to address what I hear you saying, which is that you are frustrated that generated files show up during merges, while not breaking all of the existing workflows dependent on the non-auto-generated files.
#164
@
5 weeks ago
are we in agreement then to restore the 84 (86%) of the files that are normal PHP files, mostly block code that’s required for WordPress to boot and which is hand-maintained?
For me it doesn't really make sense for PHP code for blocks to live separately from its JS code (edit, view...), these things evolve together. So for blocks I don't think it makes sense to maintain these in Core at the moment unless the JS/Styles... of the blocks is also maintained in Core which I don't really see happening unless we merge the repos.
Now my opinion is that these are still best gitignored. Now it's not a hill I'll die on, if you're not convinced by my arguments, I'm not sure what I can say more than what I already said.
#165
@
5 weeks ago
thanks for clarifying @youknowriad — given the focus on “generated code” and treating these like dependencies, I misread your opinion on the majority of files that aren’t in that category.
I’m approaching this pragmatically, so evidence is what I’m trying to follow rather than arguments, and the evidence shows that breakages are still widespread and unknown. I appreciate philosophical arguments about code structure when things are normal. the only argument I’m holding to is that it’s incumbent upon myself to perform due-diligence to avoid breaking things for others, and if I do, to work to undo that damage so it doesn’t needlessly harm everyone else.
please know that I have been trying to remain factual in this ticket and find ways to mitigate the damage from the build change while accommodating your change as much as possible. the best way to convince us all is by fixing the things that broke — you’ve done a great job at fixing some of those things, but perhaps the scope of impact of this change was simply much bigger than anticipated.
#166
@
5 weeks ago
the only argument I’m holding to is that it’s incumbent upon myself to perform due-diligence to avoid breaking things for others, and if I do, to work to undo that damage so it doesn’t needlessly harm everyone else.
I understand and I agree. The reality is any change can break something, I hope that I've shown in this ticket that I've followed and fixed all of what has been raised. There are probably some remaining things like you said, and I'm happy that you're chiming in to try to find solutions. If reverting partially is the only solution, so be it, but reverting also has tradeoffs that I mention above (introduce another kind of a breakage IMO). So maybe we can try to find a solution that keeps both things in place.
Also I have to say that I have been working almost solo on this for months and it's depressing to be met with only resistance to chance and not support and openness to what this unlocks. It is totally possible that I'm not receiving this the way I should be, but the reactions are all about "protecting" and less about thinking about achieving the vision. This is not encouraging at all to keep pushing the boundaries of possibilities of WordPress, we'll just be protecting our "fields" and leaving progress for others.
It's fair to say that I'm also a bit burnout of this at this point.
#167
in reply to:
↑ 158
@
5 weeks ago
Replying to youknowriad:
If you think about this from how WP has always been organized, I'd agree with you but the important bit here is that we're introducing the idea of a build tool that generates code for us and for the ideal DevX we shouldn't care where the files are added or organized at all. The important part is that we should be able to load build/build.php and that's all we need for our routes, scripts, modules (and more later) to be registered properly at the right moment, with the right performance... Removing all boilerplate and often hard to pick integration points.
So moving some parts of the build to a folder, and another to another folder is IMO the wrong solution, I wouldn't mind moving the build folder to src/wp-build or something but that feels like a bigger departure. We can think of this folder as the "includes" necessary to build and register WP-Admin pages but not necessary the admin pages themselves. The font-library.php file is still outside and it calls these includes.
Who is we in this part of your response? And where was this discussed before hand? Can you share who was involved when it was agreed that changing how WP is organized is the correct way forward after considering all of the requirements and historical context?
This is a very significant divergence from how WP has been organized for 15-20+ years now. It also seems to circumvent several well established patterns and expectations that plugin developers and the extender community have.
I want to understand this more so I can support it, but I don't know where to look for more information.
Next new concern I have is the way that scripts and styles are being registered and enqueued. In wp_font_library_wp_admin_enqueue_scripts(), the function returns early if the current page is not the expected font library. This means that WordPress is not always aware of the fact that the related scripts and styles are available.
This is wrong, the scripts are always registered, they are only "enqueued" on the required pages.
Can you help me understand where they are registered?
I see these lines returning early, and the only instance of wp_register_script( 'font-library-wp-admin-prerequisites' )/ wp_register_script( 'site-editor-v2-wp-admin-prerequisites' ) after this conditional.
Based on this template and the resulting PHP file, I don't think that these assets are always registered, only enqueued when needed.
I'd love to have had help in Gutenberg when I was working on this but didn't get enough support at that point.
I'm sorry you felt unsupported while working on this. That said, I don't recall seeing any posts on Make Core asking for feedback or review, no outline of the challenges this was trying to solve, the goals it was trying to accomplish, or requests for help in #core during or outside of Dev Chat. With the volume of work happening across the project, it's genuinely impossible for people to monitor everything, so the right people often won't know help is needed unless it's explicitly and loudly asked for.
These extra steps do take more time upfront, but actively seeking consensus and building wider awareness pays off greatly in the long run.
To be transparent: I've found it difficult to get some of the concerns raised here, by multiple committers and respected community members, taken seriously, and that's been discouraging. Concerns don't need to be universally shared to deserve fair consideration, and everyone involved should at least feel heard.
Based on the responses here, it seems many of the changes in this ticket are intentional and meant to shift some well-established expectations and practices. That's okay! The project should adapt when it needs to. But for that kind of change to succeed, the process needs to be collaborative process from the start.
My primary frustration isn't with any one specific aspect of these changes. It's with the process and the lack of opportunity to participate early enough to have a meaningful impact. Some of that appears to be a visibility problem. There wasn't wide awareness that this type of change was being worked on, and that was compounded by some unfortunate timing.
Collaboration did happen here, and I appreciate that. But it started too late, after the overall approach was already complete. Engagement at the proposal-to-trunk stage leaves very little room to course-correct, and that makes the process feel stressful for everyone. This isn't unique to this change. It's a pattern the project has been working around for years, and it's worth addressing directly. I also recognize that this is a piece of the puzzle to address that.
Historical continuity of the code base is very important, especially when looking back at past changes to understand the reasoning and rationale many years in the future. Many of the established processes are intentionally a certain way to ensure specific problems don't occur. Breaks in this continuity not only makes it more difficult for current contributors, but also makes it harder and more confusing to get involved contributing. With the WP Credits program, the latter part is especially important. The project needs more contributors, and it's very important to me that it remains easy to learn about the project's past in order to contribute in a well-informed way.
Replying to youknowriad
(this response came in after I typed the above).
I understand and I agree. The reality is any change can break something, I hope that I've shown in this ticket that I've followed and fixed all of what has been raised.
Also I have to say that I have been working almost solo on this for months and it's depressing to be met with only resistance to chance and not support and openness to what this unlocks. It is totally possible that I'm not receiving this the way I should be, but the reactions are all about "protecting" and less about thinking about achieving the vision. This is not encouraging at all to keep pushing the boundaries of possibilities of WordPress, we'll just be protecting our "fields" and leaving progress for others.
Thank you for all the work you've done on this, both initially and in following up to address feedback.
Everyone here is trying to make WordPress better. It's healthy to disagree about what that means or how to get there, and everyone should feel heard and understood in that process.
I've tried to be intentional in my feedback by detailing what is broken and why it matters, with as much surrounding context as I can provide. While I've shared my opinions throughout based on my understanding and the information available, I hope nothing came across as directed at any one person or as though I'm unwilling to change my mind.
I genuinely agree with the vision of reducing friction between the two code bases as much as possible. It frees up significant contributor time, and I'm participating here because I want to support that and arrive at a solution that's as maintainable as possible for the next 5 to 10 years. I also don't disagree with the broader vision of a process that handles things for contributors automatically.
Where I'm currently stuck is the implementation as it stands. The cumulative impact of the known concerns so far is tipping the scales in the wrong direction, and I think we can do better, for the sake of our current and future selves.
#168
@
5 weeks ago
I just want to share my two cents. I don't completely understand both sides of the discussion, like the concerns with version controlling some of the files, and all the remaining issues to be fixed on the other hand.
But what I do know is that compared to WP 6.9, syncing Gutenberg in 7.0 is so much easier. Thankfully last time we had two frequent Gutenberg contributors in the release team because if we still had packages, it would have been a lot more stressful for one person to handle.
Another really nice thing about the current setup is that contributors can update the Gutenberg ref in a core sync/merge/backport to point to their branch in Gutenberg and test things without needing to wait for a package update and hope that everything turns out well.
It would be a pity to me to go back to package and I hope we can resolve the issues. Perhaps a middle road is to version control some of the files again, I don't really know if that would fix everything.
#169
@
5 weeks ago
I'm also not inside the full discussions but we have used wp-build system to implement a new route for the WP AI Client connectors and it was really smooth to work with this system and implement a new React rendered page. It feels like a waste to not include the routing system that makes things much easier to work with. I guess for any change we do we should keep the routing system.
Another really nice thing about the current setup is that contributors can update the Gutenberg ref in a core sync/merge/backport to point to their branch in Gutenberg and test things without needing to wait for a package update and hope that everything turns out well.
This is really important when things are being developed at fast pace we can not wait for syncs and package publishing to test them against core.
#170
@
4 weeks ago
Perhaps a middle road is to version control some of the files again, I don't really know if that would fix everything.
@ellatrix I think there is a misread if you find anyone arguing against making Gutenberg updates smoother. This middle ground is conflating two related issues.
- The PHP files necessary for booting WordPress were removed from
wordpress/wordpress-developwhich is the primary cause of breakage in a number of workflows that probably weren’t familiar when this change was merged. - The version history for those files was also removed via
.gitignorewhich makes historical analysis, debugging, and understanding harder. That doesn’t lead to the immediate breakage, and it’s understandable how someone who doesn’t rely on that information might underestimate how important it is.
My proposal that I’m working on in PR#11024 is primarily about these two factors, specifically the first one. While we can restore those files (which will prevent the breakage), we sadly cannot restore the continuity of the source versions. We can preserve it going forward though.
This has been my primary call to action throughout this entire thing:
- these workflows were broken by intentionally removing required files.
- removing these files was not required to accomplish the change in this ticket; and adding them back doesn’t impede the change in how Gutenberg is sync’d
- let’s add them back, even though we cannot fully restore what was lost.
The work in PR#11019 is related but focused on addressing the ways that build step has impacted working on wordpress/wordpress-develop. In combination with bringing the files back it helps restore broken workflows.
I just want to reiterate that there’s been broad support for making the sync easier; and as far as I can tell, of eliminating the package process. If I have communicated that I think this shouldn’t move forward in any way, then I apologize, but I have tried hard to only ask that the way in which we move it forward is considerate of the people it affects and aware of the complicated and sensitive nature of a change this big. It’s too late to revert; it’s too late to get back the (easy) source history for these files, but it’s not too late for the process mitigations.
There was one more idea I had in the back of my head that I want to try, which might restore version history if we make some allowances in the repo, but primarily I just want to ensure that WordPress continues to run where it’s currently broken.
Docs generation is a good example of that. I wish the documentation generation process were smooth and clear and testable and widely-understood, but it isn’t. We barely got the docs at developer.wordpress.org updated after almost two and a half years of the process being broken, and it’s immediately broken again with WordPress 7.0 because of the required build step, and broken in a way it wasn’t broken before this change. I think those docs are valuable to people; I hate that they were stale so long; I hate that we didn’t step in sooner to fix them; and I hate that we have to do so much more work now to try and figure out how to make this deeper change on a system nobody seems to have knowledge of anymore.
Nobody will be able to see the docs for the new AI client until we figure out how to fix the docs-generation system to run the build step.
Or we restore the files that didn’t have to be removed. That’s where I’m coming from. I can buy the philosophical goals of removing those files, and I can sympathize with the work of maintaining the sync, as I have personally maintained a number of files on both repos and dealt with the technical and social challenges involved there. There are 100 other systems broken because of the build just like docs — it seems like the host tests are another example of that.
None of this is about Gutenberg sync, or the change itself. This is all about middle ground and figuring out if we can accomplish the intended goal without causing harm to everyone else. And yeah, an early revert would have made this so much easier for everyone because the change could have been isolated, but that’s water under the bridge now because it can’t be undone.
I hope that there will be lessons we can learn though from it: to be quicker to revert changes when people report breakage; to better understand the far-reaching impact that basic changes like this have on the broader WordPress ecosystem; and to never remove and .gitignore files that remain part of the WordPress distribution.
This highlights something I brought up back at WCUS — it would be nice to try and adopt the RFC system PHP uses for big changes. As @desroj mentioned, it can be hard to draw attention to major changes like this when there is so much broader activity going on. It’s even harder when a change is controversial to build alignment and move forward. I do think that having more formality around changes that affect everyone would lesson the personal impact this ticket has caused, giving everyone more time to notice the proposal, have a clear deadline for when the author expects it to move forward, and to allow everyone with quorum an opportunity to vote or request changes.
This is a hard multi-year process. I wish it were simple. Props to you and @youknowriad and everyone working on it.
#171
@
4 weeks ago
@dmsnell I'm just trying to share some positives about this change from my perspective. It was not a response to your PR. I don't have anything against restoring version control for some files that are required for booting WP. I know everyone wants an easier sync too, I just wanted to share how it fared in practice. :)
@dmsnell commented on PR #11024:
4 weeks ago
#172
This is more challenging than I hoped, beyond simply marking the files as binary. There is a complication that I don’t know if some of the issues I’ve hit are because I’m adding these files back in this same PR where their metadata is being changed.
It’s possible that this would be smooth were it to merge. Nonetheless, I have marked these files as auto-generated and wrote a script to use for ensuring that _non-auto-generated_ files are not changed.
The changes will still be visible if someone wants to see them, and they show up in git status, but the CI jobs will overlook them. I think this matches what we want. We essentially only want those files to be updated when changing the Gutenberg hash, and those changes should be checked in. If they are ever modified otherwise we just want to ignore that.
I do think that this extra work with git attributes is probably unnecessary and all we need to do is just add the files back and then things would work, but this is continuing the experiment to see how far we can go with the problem:
Track these files and commit their changes when changed, but ignore updates that aren’t intentionally added.
#173
@
4 weeks ago
In PR#11064 I have proposed a merge which restores the source history of the deleted files, but there are caveats.
gitis able to retrieve the history if we perform a non-fast-forward merge with a parent commit from before the build change. This restores the continuity across the delete.- I’m completely ignorant on whether we could preserve this structure of a merge across the Subversion bridge. We would need to, at a minimum, manually create the merge from the
svnside. Even then, I don’t know if it’s possible to have that automatic commit.
So this is technically possible anyone doing dev work locally could always merge in this branch to fix things, but that doesn’t help all of the external integrations we can’t control. If we could test this in a simulated svn-git bridge we could know for sure. If we could get this merged properly back into the repo then it should resolve every existing case that was broken.
#174
@
4 weeks ago
Happy update here: I have been able to simulate this entire flow and see that the version history can be restored for these files through the svn > git bridge, but it’s going to need some manual intervention, including help with those who manage that process.
I think it would be incredibly prudent for us to restore the continuity of these files into the repo before releasing 7.0, and I would appreciate anyone’s help if you work with the system pushing updates from subversion into git.
In PR#11064 I have added a script and a screencast demonstrating the flow. There were a lot of mines in the path to get there, and while I think there’s a chance it could be done from the git side, and potentially easier so, I wanted to be sure that a svn-first approach works.
Fundamentally, if we can merge into trunk a copy of the deleted files whose merge base is before 22294af4ed3b46f577f42590ba4cc19ca0a93f3f / [61439], and perform a non-fast-forward merge, then git and all git tooling (such as GitHub, IDE integrations, Git GUIs) should by default trace the history of these files through the merging and no longer represent them as if they were brand new today.
Steps (simplified):
- from the SVN repo…
svn copy trunk@61438 ^branches/restore-required-files— this creates the branch with the fork point before the build change.- copy the files from a clean checkout of
wordpress/wordpress-developwherenpm ci && npm run build:devhas already run. this is going to copy in the latest version of these built files and create a merge conflict that will give us a chance interrupt the auto-merge process and “resurrect” the files, as SVN puts it. - commit these changes
- go back to
trunk svn merge ^/branches/restore-required-files --accept postponewill start the merge and pause on the conflicts- remove the relevant patterns from
.gitignore - since the conflict is a “tree conflict” we can’t just “accept theirs” but we can mark the repo as “working copy includes the resolved version of the changes.” —
svn resolve --accept working . svn addall of the relevant files that appear insvn statussvn committo complete the merge and restoration
- from the
gitrepo- after the merge is done, perform
git svn fetch --allto ensure that the restoration branch is fetched. If we leave out this step thengitwill linearize the merge and our history will not connect to the past. git svn rebasethis should pull in the changes while creating the merge commit containing both appropriate parents!
- after the merge is done, perform
Obviously the automation synchronizing git with svn will need to be paused during this time otherwise we could get the repo into a messed-up state. That’s why I created the simulation in the linked PR, to ensure that everything is done the way we expect, and to make it easier to ensure that the steps are right, as well as to compare changes to the merge to see if they could be better.
I would be grateful if anyone who works with the svn and git repos could help me out with this. Once we release 7.0, if the files are not restored into that commit then any workflow which skips between official releases will hit a wall at 7.0; any attempt to review the total changes or examine a changelog will show that these files exist no more, even though they are entirely necessary for WordPress to boot. They will appear in the ZIP without any clear link back to the source.
You can review the script and screencast in the linked PR. the code of that PR isn’t as important at this moment. Thank you.
This ticket was mentioned in Slack in #core-editor by juanmaguitar. View the logs.
4 weeks ago
#176
@
4 weeks ago
I've spotted an issue just now with how the build server is committing updates to files generated by the new build script steps.
It seems that the build server adds and changes files as expected, but it never deletes files that are removed. I believe that this is caused by the fact that the files in question were committed to version control prior to [61476], which adjusted the svn:ignore rules to exclude the wp-includes/blocks and wp-includes/build dircetories (which is where the majority of the issues are.
Here is a list of files that I see present that should have been removed from the build server:
wp-includes/blocks/archives/editor-rtl.css wp-includes/blocks/archives/editor-rtl.min.css wp-includes/blocks/archives/editor.css wp-includes/blocks/archives/editor.min.css wp-includes/blocks/tag-cloud/editor-rtl.css wp-includes/blocks/tag-cloud/editor-rtl.min.css wp-includes/blocks/tag-cloud/editor.css wp-includes/blocks/tag-cloud/editor.min.css wp-includes/build/pages/site-editor/loader.js wp-includes/build/pages/site-editor/page-wp-admin.php wp-includes/build/pages/site-editor/page.php wp-includes/build/routes/font-list/package.json wp-includes/build/routes/fonts-home/package.json wp-includes/build/routes/home/package.json wp-includes/build/routes/index.php wp-includes/build/routes/navigation-edit/package.json wp-includes/build/routes/navigation-list/package.json wp-includes/build/routes/navigation/package.json wp-includes/build/routes/pattern-list/package.json wp-includes/build/routes/pattern/package.json wp-includes/build/routes/post-edit/package.json wp-includes/build/routes/post-list/package.json wp-includes/build/routes/post-new/package.json wp-includes/build/routes/post/package.json wp-includes/build/routes/styles/package.json wp-includes/build/routes/template-list/package.json wp-includes/build/routes/template-part-list/package.json wp-includes/build/routes/template-part/package.json wp-includes/build/routes/template/package.json wp-includes/css/dist/block-library/archives/editor-rtl.css wp-includes/css/dist/block-library/archives/editor-rtl.min.css wp-includes/css/dist/block-library/archives/editor.css wp-includes/css/dist/block-library/archives/editor.min.css wp-includes/css/dist/block-library/tabs/editor-rtl.css wp-includes/css/dist/block-library/tabs/editor-rtl.min.css wp-includes/css/dist/block-library/tabs/editor.css wp-includes/css/dist/block-library/tabs/editor.min.css wp-includes/css/dist/block-library/tag-cloud/editor-rtl.css wp-includes/css/dist/block-library/tag-cloud/editor-rtl.min.css wp-includes/css/dist/block-library/tag-cloud/editor.css wp-includes/css/dist/block-library/tag-cloud/editor.min.css wp-includes/css/dist/index.php wp-includes/css/dist/theme/style-rtl.css wp-includes/css/dist/theme/style-rtl.min.css wp-includes/css/dist/theme/style.css wp-includes/css/dist/theme/style.min.css wp-includes/js/dist/react-refresh-entry.min.asset.php wp-includes/js/dist/react-refresh-entry.min.js wp-includes/js/dist/react-refresh-runtime.min.asset.php wp-includes/js/dist/react-refresh-runtime.min.js wp-includes/js/dist/script-modules/index.php
Here's what I've found so far to confirm that these should have been deleted.
Changes merged in [61605]:
- The
wp-includes/blocks/archivesandwp-includes/blocks/tag-cloudCSS files were removed in GB-74255 and GB-74228, respectively. These PRs should have also removed the correspondingwp-includes/css/dist/block-library/archives/*.cssandwp-includes/css/dist/block-library/tag-cloud/*.cssfiles. - The
wp-includes/build/pages/site-editor/*files were removed in GB-74221 and the directory was renamed tosite-editor-v2(which does exist as expected). - GB-74412 removed the
editor.cssfiles for thetabsblock. These were added in [60758] were not removed because that came after [61476].
Other details:
- I'm unclear how, but I believe that something included in [61492] should have removed all of the
package.jsonbased on the commit message and this comment on PR-10718. - The
wp-includes/js/dist/react-refersh-*.jsfiles were removed in [61438]. They were re-added in [61488] (and fixed in [61543]), but they now reside withinwp-includes/js/dist/development/instead. I'm also not clear on why the move was required. It feels a bit backwards to have a "development" file within adistfolder, which is meant for distribution.
There are three remaining files that I'm not certain on just yet.
- The
wp-includes/css/dist/index.phpfile was last modified in [61491]. I have not found where this was removed to confirm it was intentional. wp-includes/build/routes/index.phpwp-includes/js/dist/script-modules/index.php
I am not 100% sure how to remove these, but without addressing this, they will be included in the final package that's generated and shipped to the world. This can be confirmed by downloading the nightly or beta2, which contains these files.
It's possible that reverting the svn:ignore changes, allow the build server to remove the files, then recommitting them could solve the issue. I am not going to play around with that right now because I don't want to introduce another variable and possibly cause issues with @dmsnell's plan.
#177
@
4 weeks ago
One more thought related to svn:ignore properties. I noticed that [61701] changed the ignore rules for src/wp-includes/assets and src/wp-includes/blocks from a list of specific files and patterns to a * wildcard.
I may be missing it (this ticket has gotten quite long), but I could not find any discussion around why * was used on the ticket.
My gut tells me that this is not the right way to handle this. Ignoring everything that's ever added to the directory is essentially a form of error suppression. If a new file is unexpectedly added to these directories, it would not be immediately obvious unless someone looked at the corresponding commit on the build server. By defining patterns that explicitly match the files that are expected to be built, it ensures that only the correct ones end up in the built package.
Based on what I'm seeing locally after running the build script as it stands in trunk currently, the expected files within each src/wp-includes/blocks/BLOCKNAME directory are as follows:
block.json(all)style-rtl.css/'style-rtl.min.css` (some)style.css/style.min.css(some)editor.css/editor.min.css(some)editor-rtl.css/editor-rtl.min.css(some)theme.css/theme.min.css(some)theme-rtl.css/theme-rtl.min.css(some)navigation-link/shared/item-should-render.phpandnavigation-link/shared/render-submenu-icon.php
That said, maintaining this is likely going to be a pain because of how SVN's ignore properties work.
- To my knowledge, SVN does not support wild card patterns such as
src/wp-includes/blocks/*/block.json. - With the exception of the
block.jsonfile, the files above are not included required for every block. - Since these are added and removed upstream in
gutenberg, the block directories would need to be updated any time files are added or removed.
This could likely be scripted, but would take a bit of research to confirm first. A few things to note:
- If files are added, the directory will be in a mixed state with unversioned files so changes can be verified.
- When files are removed, nothing would seem out of place because the files were previously expected and ignored.
The flow I have in mind is:
- Gutenberg hash is bumped in
package.json. - Build script is run.
- New files can be verified because they show as unversioned.
- After those changes are verified, a script (a Grunt task added to
grunt precommitfeels best at the moment) could be run that changes the property for each directory appropriately. - Only the expected changes should be present now.
I typically commit directly using SVN, but some committers use a git-svn flow. Whatever approach is used would need to be tested in multiple commit workflows. The Core Committer Workflows page in the handbook could be a great starting point.
To be clear, I don't think having a script for this is a blocker for 7.0. However, I do think that the wildcard ignore properties should be changed as soon as @dmsnell is hopefully able to work his magic and restore the change history.
#178
@
4 weeks ago
Actually, I just realized that the ignores would likely not be required at all should @dmsnell be successful and history is restored. But still good to have that documented for the future in case it comes up again, or there is a compelling reason to keep a subset of the ignore rules.
@desrosj commented on PR #10971:
4 weeks ago
#179
This did not really warrant the results I was hoping for. @westonruter did have one more idea, but I much prefer the solution I'm working on in #11019. The improvement there is significantly greater than what caching the gutenberg directory or the dependencies there would provide.
@desrosj commented on PR #11019:
4 weeks ago
#180
I still need to go through and polish this, but the PR for the Gutenberg repository is ready. That should be merged first anyway, so I'm hoping to clean this up on Monday.
#181
follow-up:
↓ 183
@
4 weeks ago
Not relevant to Dennis' PRs, but the SVN ignore wildcard is exactly the same as all the folder "wildcards" we have in gitignore for many years for many folders? Everything in those folders was correctly meant to be ignored including new files.
@desrosj commented on PR #11019:
4 weeks ago
#182
did you by any chance try to measure the time it takes to run the dir hashing? I would think that could have some unexpected lag to it.
My initial rough measurements were showing that the hashing takes around 150-200ms with the downloaded pre-built plugin files, and ~2.5 seconds with the full repository checkout.
#183
in reply to:
↑ 181
@
4 weeks ago
Replying to ellatrix:
Not relevant to Dennis' PRs, but the SVN ignore wildcard is exactly the same as all the folder "wildcards" we have in gitignore for many years for many folders? Everything in those folders was correctly meant to be ignored including new files.
There are only two wildcards I can see in place besides /src/wp-includes/assets/* and /src/wp-includes/blocks/*:
.cache/*/src/wp-content/themes/*
For ./src/wp-content/themes/, there are rules after that negating specific directories (twentyten, twentyeleven, etc.).
The issue is not so much adding a wildcard, it's that a wildcard was added when files in that directory were under version control and they were not deleted in a separate commit prior to the wildcard being added.
It seems the build server will not delete a file unless it's explicitly told to do so by SVN through a commit deleting the file. Otherwise the files will just remain forever.
@desrosj commented on PR #11019:
3 weeks ago
#185
I talked through this PR with @dmsnell today, and we arrived at the decision that it was best to leave all aspects of using the git repository out for this iteration.
While it's possible someone would could be using the gutenberg directory to test changes to the wp-build package, the script currently only performs a git clone at a --depth=1, which is not particularly useful for any meaningful contributions. We can always add this back later if it turns out this is an actual flow contributors will want to use.
We also decided that for the time being, there's no need to hash the entire gutenberg directory to try and detect changes to the files. If the files are re-added to version control, any unexpected changes to them would appear after running build anyway.
I've gone and rebased all of the changes onto the latest from trunk and removed all of the commits that re-added the Git-related functionality. I think this is good to get reviewed and tested.
This ticket was mentioned in PR #11182 on WordPress/wordpress-develop by @desrosj.
3 weeks ago
#186
It seems that the schema URL is not being transformed during the build process currently. This fixes that.
Trac ticket: Core-64393
## Use of AI Tools
None.
@desrosj commented on PR #11019:
3 weeks ago
#187
I also realized that a good portion of the custom code in the tools/gutenberg is performing tasks that are already handled within grunt.
I've made adjustments so that as much as possible (where other complex modifications are not being made), the Gutenberg-related files are now copied by grunt-contrib-copy instead. Also, grunt-contrib-replace already was replacing sourceMappingURL in ~50 files. It now replaces the pattern for all Gutenberg related files that are also moved into place in the new build script.
#188
@
3 weeks ago
After more simulation and experimentation, I’d like to perhaps try this svn commit to restore the version history, and I would really appreciate any help to review the plan before I do it.
Noting:
- There are no current merge commits in
trunkin thegitmirror. This means that the merge will be a first. - This is a normal thing to do in an
svnrepository, but branching is heavy and we haven’t done it, so I don’t know if there are any things I am not anticipating that could go wrong. Do any tools rely on an assumption that history is linear only? - Can we create a backup of the
svnmaster repo just in case? - Can anyone confirm whether the post-commit hook in
svnleads to agit svn fetchon thegitside before performing agit svn rebase, or whatever the code is that runs?
@dd32 I know you’ve been wearing a lot of hats recently, but is there any way we can review the scripts or the code that pushes changes to git so can we have an appropriate expectation of what happens if we do this?
Plan:
- create an
svnbranch from before the build change on Jan 5. - copy the built files from
trunkinto that branch on the files that were deleted - update the
.gitignoreto add them back in - merge that branch back into
trunk, thus establishing continuity with their histories.
If we don’t have any better assessments then I think at some point we just need to try this and deal with the consequences if something doesn’t go right. The thing I’m most concerned about is that if we don’t get updates on the git side that pull in the svn branch, then git svn is going to flatten the changes when we merge the branch and that will be for a waste because it won’t restore the history.
@dmsnell commented on PR #11019:
3 weeks ago
#190
@desrosj in 3050cd1b0a0d439c86d4110814b718aadfdecb74 I replaced the string/RegExp-based PHP parser with php, then in f588cf5dfe78b50e2bae53b09b61b647820824bf I moved it up near the top of the file so GitHub’s diff viewer wouldn’t show it interspersed with the now-removed parsePHPArray(). this new version could be slower due to the startup time involved in calling php in a new process, but I think that will be paid for in not having to maintain that code, and the code was already full of latent parsing issues.
as I’ve reviewed this today I keep finding places we’re doing the lots of repetitive work which doesn’t even need to be done, but it’s scattered through the files. for example, a lot of these PHP files are generated in Gutenberg from JSON, and then parsed as string in here in wordpress-develop, and then JSON-stringified, and then JSON-parsed. the whole thing is kind of mind boggling and I think we could probably eliminate a lot more code just by doing const data = require( 'manifest.json' ) instead of what’s happening now.
so I apologize, but this last statement isn’t fully actionable; just a little venting and one contributing factor to why it’s taken me so long to review this or the original change that introduced the code modified here.
#191
@
3 weeks ago
dd32 I know you’ve been wearing a lot of hats recently, but is there any way we can review the scripts or the code that pushes changes to git so can we have an appropriate expectation of what happens if we do this?
The one that will cause the most problems is the develop.svn -> core.svn sync I'd say, that's fairly custom. (Although https://build.trac.wordpress.org/changeset/61150 seems to have worked fine, I don't think it'll sync file copies/moves as anything other than delete/recreate, the branch copy is a dedicated step)
I'm not 100% sure how the SVN -> git sync works, I think it's a cron running git svn fetch; git push in a loop-type situation.
@dmsnell Please sync up with someone from systems (ie. not me) to discuss the steps forward.. It's likely that you're going to need to disable sync, and/or maybe make some manual commits to core.svn as well.
@desrosj commented on PR #11182:
3 weeks ago
#193
Merged in r61867.
@desrosj commented on PR #10971:
3 weeks ago
#195
Closing in favor of #11019, which is just about ready for merge.
@desrosj commented on PR #11019:
3 weeks ago
#197
Thanks for the feedback, @dmsnell! I am going to open a follow up ticket for these after this gets in for improvements. But I don't think any of the feedback is a blocker currently.
@desrosj commented on PR #11019:
3 weeks ago
#199
Merged in r61873.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
3 weeks ago
#202
@
3 weeks ago
The work in PR#11064 to restore the removed files is prepped in git and appears to be working as expected. Some updates:
- the goal of the restore PR is now more or less “revert the decision to remove all of the files” so it has changed in structure and scope.
- instead of a single restoration commit, it now comprises one commit per instance of when the repo was synchronized with Gutenberg. you can
git logthe deleted files and the changes that occurred between syncs is now visible (easier seen than described). - instead of only restoring the deleted files, this also includes additions such as the SVG icons.
ddeea0126e Restore: Sync to Gutenberg revision b79bbe25a1b00ce640bfab363f182c678181082f 61667fc4a4 Restore: Sync to Gutenberg revision 59a08c5496008ca88f4b6b86f38838c3612d88c8 7f996cd195 Restore: Sync to Gutenberg revision 7a11a53377a95cba4d3786d71cadd4c2f0c5ac52 7184f2a9dd Restore: Sync to Gutenberg revision b441348bb7e05af351c250b74283f253acaf9138 30095a4938 Restore: Sync to Gutenberg revision 23b566c72e9c4a36219ef5d6e62890f05551f6cb f83a4e49d7 Restore: Sync to Gutenberg revision 022d8dd3d461f91b15c1f0410649d3ebb027207f 5bebac321d Restore: Sync to Gutenberg revision e499abfb843a43ac88455ca319220c5f181e1cf3 ec268db962 Restore: Sync to Gutenberg revision e7b8c0c8a34bd62374ebf6f9cbfdeab4f822234c 327427f54d Restore: Sync Gutenberg to revision f4d8a5803aa2fbe26e7d9af4d17e80a622b7bab8 efc89d8519 Restore: Sync Gutenberg to revision 7b7fa2bc97a8029a302bd6511cf0d206b5953172
What remains:
- commit the ten intermediate restore changes into the
svnbranch^branches/fixes-64393-restore-version-history - run
svn merge ^branches/fixes-64393-restore-version-history --accept postponeon^trunkto create a merge commit insvntying the history together, recreate the conflict resolution as appears in the PR, reset the appropriatesvn:ignoreprops, verify, then commit.
This ticket was mentioned in PR #11221 on WordPress/wordpress-develop by @desrosj.
2 weeks ago
#203
This PR aims to prevent a failure when npm install is run when either the directory already exists or the the gutenberg/.gutenberg-hash file is missing (which means the script cannot currently confirm the correct version of the Gutenberg plugin is present).
- The
postinstallcommand is changed fromgutenberg:downloadtogutenberg:verify. - The
--forceoption has been removed.gutenberg:downloadwill now download a fresh copy every time it's run. - The
gutenberg:verifyscript is now the preferred entry point for managing the files within thegutenbergdirectory. It will only trigger a downoad if the hashes do not match, or the folder is missing entirely.
Trac ticket: Core-64393.
## Use of AI Tools
Claude was used to create the initial draft for this PR.
@dmsnell commented on PR #11024:
2 weeks ago
#204
Closing in favor of #11064
This ticket was mentioned in PR #11233 on WordPress/wordpress-develop by @desrosj.
2 weeks ago
#205
This adjusts the Gutenberg-related grunt copy configurations to also copy over the .js files in the gutenberg/build/pages and gutenberg/build/routes directories.
Trac ticket: Core-64393.
## Use of AI Tools
Claude was used to confirm that this was causing the differences.
@desrosj commented on PR #11233:
2 weeks ago
#207
Merged in r61798.
This ticket was mentioned in Slack in #hosting by amykamala. View the logs.
2 weeks ago
@desrosj commented on PR #11221:
2 weeks ago
#210
Merged in r62021.
This ticket was mentioned in Slack in #hosting by ramonfincken. View the logs.
12 days ago
This ticket was mentioned in PR #11294 on WordPress/wordpress-develop by @desrosj.
10 days ago
#212
A side effect of the changes from Core-64393 has surfaced that is potentially affecting anyone looking to run WordPress from src/.
Previously, the relevant block editor-related files were subject to version control within the src/ directory. They were always present, and they were updated from the dependencies installed through npm when npm run build:dev was run. Because of this, someone could clone wordpress-develop and immediately run the PHPUnit tests without having to run a build script (the files were already in the right spots).
The copy:block-json Grunt task was also responsible for generating the src/wp-includes/blocks/blocks-json.php file and was run before any grunt copy commands were run. This ensured it was present in src/ before copying all of the files over to build on npm run build.
The side effect of this is that when the PHPUnit test suite is run from src/, there are unexpected failures whenever anyone:
- Runs
npm run buildand notnpm run build:dev - Does not run any
buildscript at all assuming the files are in place.
This issue surfaced in https://github.com/WordPress/phpunit-test-runner/issues/307 and there are many Hosting Test reporters failing with the same errors currently. This is due to the fact that the phpunit-test-runner always runs npm run build as a part of the setup. I haven't found specific reasoning why. But if I had to guess, it's because npm run build:dev was not necessary previously. So running build ensured both ways of running the test suite would work.
On one hand, the test runner is _doing_it_wrong() because regardless of whether the tests are being run from build or src, npm run build is always the command that's run. But given that the files were removed from version control within src/, I think it makes sense to always run build:gutenberg -- --dev to ensure that the expected source files are present in the src/ directory.
Also worth noting is that #11064 aims to restore the files that were removed from version control along with the associated history. Once this happens, this likely wouldn't be necessary anymore (the files will again always at least exist because they are versioned). But because these files have gone missing unexpectedly, this is what I recommend as a fix until #11064 is merged.
Trac ticket: Core-64393.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude
Model(s): Sonnet 4.6
Used Claude to investigate the root cause, and recommend a possible way to fix the issue. An approach different than the one suggested was ultimately used.
@desrosj commented on PR #11294:
10 days ago
#215
Merged in r62052.
This ticket was mentioned in PR #11306 on WordPress/wordpress-develop by @desrosj.
10 days ago
#217
This addresses an issue where files persist in the src directory of the build server checkout despite having been removed. Because the files are not under version control and there's no explicit delete command, they are not removed from src as expected.
This is a short-term solution that's meant to run once and be removed. The long-term solution is the work being done in #11064, which will re-add these files to version control, which solves this problem naturally through changes to the files themselves.
Trac ticket: Core-64393.
## Use of AI Tools
@desrosj commented on PR #11306:
9 days ago
#219
Merged in r62069.
This ticket was mentioned in PR #11312 on WordPress/wordpress-develop by @desrosj.
9 days ago
#220
This removes several groups from COPY_CONFIG that were moved to Grunt tasks for consistency. The copyDirectory() function is also being removed because it's never called.
Trac ticket: Core-64393.
## Use of AI Tools
This ticket was mentioned in PR #11313 on WordPress/wordpress-develop by @desrosj.
9 days ago
#221
The Icon Library image files are images, so they should live in the wp-includes/images/ directory.
This updates the build script to:
- copy the icon library SVG files to
wp-includes/images/icon-libraryinstead ofwp-includes/icons - Moves the
manifest.phpfile to thewp-includes/assets/directory. - Gives the
manifest.phpfile a more specific name:icon-library-manifest.php - Remove
gutenberg-from the correspondinggrunt copytasks. - Removes an unnecessary
trailingslashit()call in the icon registry class.
Trac ticket: Core-64393
## Use of AI Tools
This ticket was mentioned in PR #11315 on WordPress/wordpress-develop by @desrosj.
9 days ago
#222
The *.asset.min.php files and router.php in the main script modules directory should not be copied in the build step.
The contents of these files are already included in the built file within wp-includes/assets/.
Trac ticket: Core-64393.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Sonnet 4.6
Used for: Used Claude to analyze the code base to confirm that the PHP files being removed had no use within wordpress-develop.
-->
@peterwilsoncc commented on PR #11312:
9 days ago
#223
Testing notes:
rm -rf gutenberg .gutenberg-hash; npm ci; npm run grunt clean:qunit; npm run build:dev- Opened network tools to disable caching
- Visited post editor, site editor, fonts library
- Created a gallery in a post of publicity stills from the hit Broadway musical Dear Evan Hansen (not the movie, ugh)
- Visited post on front end to ensure the block styles loaded as expected.
@desrosj commented on PR #11312:
9 days ago
#225
Merged in r62071.
@desrosj commented on PR #11315:
9 days ago
#226
@peterwilsoncc thanks! I revised the PR to skip the PHP files for the scripts, not the modules. I was crossed up because they are called scripts and script modules, but are different.
The PHP files copied into js/dist/script-modules are still required because they're consumed directly by the individual pages in order to register a script properly with dependencies.
I'd like to look at adjusting that for 7.1 so that the wp_register_script() calls are extracted out of these pages and into script-loader.php, but limiting the other PHP files that are unnecessary currently is good for now.
@desrosj commented on PR #11313:
9 days ago
#228
I accidentally referenced the wrong ticket in r62073.
#229
@
9 days ago
- Resolution fixed deleted
- Status changed from closed to reopened
Accidentally incuded "Fixes" instead of "See" in [62073].
@mcsf commented on PR #11313:
9 days ago
#230
IIRC, I originally set svn:ignore to reflect the .gitignore settings for icons. Something for you to keep in mind when you commit. :)
@desrosj commented on PR #11313:
9 days ago
#231
Thank you @mcsf! Added @t-hamano for a second set of eyes.
This ticket was mentioned in PR #11318 on WordPress/wordpress-develop by @desrosj.
8 days ago
#232
The built gutenberg asset currently includes the files for all routes in the Gutenberg plugin, even experimental ones. However, the registry.php file only references the routes that should be bundled in Core correctly.
This collects all of the grunt copy logic related to routes into a new copy:routes task and adjusts the src list to only include files related to the routes specified in the registry.php file to avoid unintentionally including files that were not meant to be considered stable.
Trac ticket: Core-64393.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Sonnet 4.6
Used for: Created the initial first draft of the PR based a description of the problem and desired outcome.
-->
@desrosj commented on PR #11313:
8 days ago
#233
I'm going to commit this so that it's included in the silent beta 6 later today.
@desrosj commented on PR #11313:
8 days ago
#235
Merged in r62077.
@desrosj commented on PR #11318:
8 days ago
#237
Merged in r62079.
@wildworks commented on PR #11313:
6 days ago
#238
Sorry for the late reply! All changes look good to me👍
This ticket was mentioned in PR #11064 on WordPress/wordpress-develop by @dmsnell.
4 days ago
#239
Trac ticket: Core-64393
The build change in #10638 removed many required files and added them to .gitignore. This led to WordPress crashing when loading wp-load.php until the npm run build:dev command was run.
Deleting these files had a number of deleterious side-effects:
- They no longer appear in
wordpress-develop, making it hard to see what code is going to run and hard to reference the files in places like Github wheregitCLI tools and the Gutenberg repo are not colocated and handy. - Any version-history for the files is lost, making it particularly difficult to examine how those files changed with the Gutenberg sync and update cycle, or how to track what ran at what version of WordPress.
- Because of the missing history, this severely impacts the ability to debug issues or
git bisectto find root cause and changes containing the root cause. - Any changes which are made to these files is untracked, making edits at high risk for being lost without any way to undo or revert. Previously, changes might be missed because they need to sync to Gutenberg, but after the build change, those edits never appear to warn someone that they edited the file in the wrong repo.
- Benchmarking, debugging, and analysis flows which scan through the source control revisions now take 10x, 100x, 1000x longer to run because
npm cihas to run, download more than 1 GB ofnpmpackages, and rebuild all of the files on everygit checkout. this practically eliminates the practicality of running workflows which assess the project over time.
This PR brings back those files and connects them to their pre-build-change version history by branching from a point in trunk immediately before the build change.
Although they were deleted in trunk, this patch, when applied _as a merge commit_ will provide two parents which will allow any and all git tooling to reconstruct the history of the files without any special options or flags.
## Status
This is ready for a full review.
As of the time of posting this, running npm run grunt gutenberg:download -- --force with this branch checked out leaves no changes or ignored changes in git, which suggests that every file which was changed has been accounted for.
dmsnell@Maximo ~/c/fix-the-build (build/restore-deleted-files-preserving-history)> git status --ignored On branch build/restore-deleted-files-preserving-history Your branch is up to date with 'origin/build/restore-deleted-files-preserving-history'. Ignored files: (use "git add -f <file>..." to include in what will be committed) gutenberg/ node_modules/ packagehash.txt
---
Note:
- This _must_ merge as a non-squashed commit, because the commit parent must be accessible to preserve history.
- This needs to also come through in the
svn-to-gitconversion. If the merge is lost in that process then it won’t matter if this PR restores the history, because it will be lost when back-writing from the Subversion source.
## Testing
Here is a sequence of events that simulates this operation. The script creates an SVN repo, adds some files and makes a few meaningless commits, then deletes a test.txt file.
There is a git svn mirror tracking the SVN repo.
From the SVN side, a new branch is created to restore the files. It is forked from before they were deleted and then the test.txt file is modified in a neutral way so that it will create a merge conflict. This is important because otherwise svn and git will automatically accept the deleted files as the truth.
That branch is merged in which tracks the version history for the files, because it maintains metadata pointing to the commits before they were deleted.
On the git svn side though it’s critical to first git svn fetch --all to retrieve the new branch (otherwise it will not have the metadata and therefore linearize the merge), and then to run git rebase --merge --rebase-merges so that it avoid linearizing the merge.
This has been confirmed and the
svnbranchfixes-64393-restore-version-historyhas already come over fromsvninto thegitmirror — this should work as expected.
#!/usr/bin/env bash
ROOT=$(pwd)
REPO=file://${ROOT}/svn-repo
printf "\e[90mStep \e[33m1\e[90m — create SVN repo\e[m\n"
svnadmin create svn-repo
svn mkdir ${REPO}/trunk -m "Create trunk"
svn mkdir ${REPO}/branches -m "Create branches"
printf "\e[90mStep \e[33m2\e[90m — load content into SVN checkout\e[m\n"
svn co ${REPO} svn-checkout
pushd svn-checkout/trunk
svn up ..
echo "Testing instructions" > test.txt
echo "1 2 3 4" > data.dat
echo "" > .gitignore
svn add test.txt data.dat .gitignore
svn commit -m "Initial commit"
popd
printf "\e[90mStep \e[33m3\e[90m — establish \e[2mgit\e[0;90m mirror\e[m\n"
git svn clone -T trunk -b branches ${REPO} git-repo
printf "\e[90mStep \e[33m4\e[90m — make noisy change\e[m\n"
pushd svn-checkout/trunk
echo "\n\n1. look at it" >> test.txt
echo " 5" >> data.dat
svn commit -m 'Expand testing'
echo "<?php\n\ndie();" > feature.php
svn add feature.php
svn commit -m 'Add feature skeleton'
svn up
popd
pushd git-repo
git svn rebase
popd
printf "\e[90mStep \e[33m5\e[90m — remove test.txt\e[m\n"
pushd svn-checkout/trunk
svn rm test.txt
echo "/test.txt" > .gitignore
svn add .gitignore
svn propset svn:ignore -F .gitignore .
svn commit -m 'Remote tests'
popd
printf "\e[90mStep \e[33m6\e[90m — make more noisy commits\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(1);" > feature.php
svn commit -m 'Die abnormally'
echo "Are you lost?" > directions.html
svn add directions.html
svn commit -m 'Add directions'
svn up
popd
pushd git-repo
git svn rebase
printf "\n\n\e[90mState of the git repo after deleting the tests\e[m\n"
git log --graph --topo-order --oneline
popd
printf "\n\e[90mPausing for review of the post-delete repo states.\e[m\n"
read
printf "\e[90mStep \e[33m7\e[90m — Branch before removal to restore\e[m\n"
pushd svn-checkout/trunk
svn copy -r5 ${REPO}/trunk ${REPO}/branches/restore-tests -m 'Restore tests (branch from f5 before removal)'
svn up ../
popd
pushd svn-checkout/branches/restore-tests
echo "\n" >> test.txt
echo "" > .gitignore
svn propset svn:ignore -F .gitignore .
svn commit -m 'Introduce merge conflict to force restoration'
svn up ../
popd
pushd git-repo
git svn rebase
popd
printf "\e[90mStep \e[33m8\e[90m — Noisy commit\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(0);" > feature.php
svn commit -m 'Die normally'
svn up
popd
pushd git-repo
git svn rebase
printf "\n\n\e[90mState of the git repo after noisy commits\e[m\n"
git log --graph --topo-order --oneline
popd
printf "\e[90mStep \e[33m9\e[90m — Merge branch and restore tests\e[m\n"
pushd svn-checkout/trunk
svn merge ${REPO}/branches/restore-tests --accept postpone
svn propdel svn:ignore test.txt
ls ../branches/restore-tests
cp ../branches/restore-tests/test.txt .
svn resolve --accept working test.txt
svn add test.txt
svn commit -m 'Merge and restore tests'
svn up ../
popd
pushd git-repo
git svn fetch --all
git svn rebase
printf "\n\n\e[90mState of the git repo after the merge\e[m\n"
git log --graph --topo-order --oneline --pretty='format:%h %cs %s'
popd
pushd git-repo
git svn fetch --all
git svn rebase
printf "\n\n\e[90m ...and after calling propset to adjust the date\e[m\n"
git log --graph --topo-order --oneline --pretty='format:%h %cs %s'
popd
This ticket was mentioned in Slack in #core-committers by jorbin. View the logs.
4 days ago
@dmsnell commented on PR #11064:
2 days ago
#242
Merged in [62143]
08be2764e3f46f2729e5a4f0af34a6d3150d640b
This ticket was mentioned in PR #11361 on WordPress/wordpress-develop by @desrosj.
2 days ago
#243
This performs a build:dev after the commit that restored history for files that were removed from version control.
Trac ticket: Core-64393.
## Use of AI Tools
AI assistance: None
@dmsnell commented on PR #11361:
2 days ago
#245
Merged in [62144]
4318f220de0787f589513937fe524df6533ba7e4
#252
@
32 hours ago
- Resolution set to fixed
- Status changed from reopened to closed
Just wanted to share a high-level summary here, but I think this can be closed out in favor of new, more specific tickets:
- The build script is stable and bundles the appropriate files from the Gutenberg repository into this repository without the performance degradation experienced by using a checkout of the actual
gutenbergrepository. - Running the
gutenbergbuild script locally before buildingwordpress-developis no longer required. - A number of changes have been made to remove nearly 900 files that were incorrectly being included in the built package accounting for nearly 6MB.
- The change history of the files that were originally removed has successfully been restored and those files are subject to version control again. Follow up discussions can be had so that these file can be removed when safe and appropriate.
Here is a collection of tickets that are related to this one and may or may not have been noted already above for easy reference: #64925, #64909, #64884, #64176, #64734, #64933.
There is one related ticket still opened that should be addressed before 7.0: #64874. And I have a long list of notes for follow up tickets to create.
Notably, the spirit of the original goals is intact, and collectively are solved:
- Making it easier to include PHP code from the Gutenberg repository.
- Consuming files built through the
wp-buildpackage in Core.
There's a lot of work that can be done, but this is a giant step forward in many ways. Thank you to everyone who contributed to this the last 4 months!
Did someone say monorepo? :D
I suggest bringing this up in dev chat to discuss the best path forward with evaluating this. Dedicated Slack meetings or even video calls, followed by a concrete make/core proposal would be helpful. We can come back to Trac once there's a plan :)