Problem/Motivation
Betas have added one update hook per config change: display_builder_update_11101 to 11106 and display_builder_post_update_1 to _9. All of them only migrate beta data. From RC1, config schema and APIs are frozen, so these hooks are dead code for any site already past the betas.
Keeping them costs more than dead code:
- Each schema change can break an older hook that saves a profile. #3624051: Failed: InvalidArgumentException after update (drush updatedb) to 1.0.0-beta7: post update 3 saved profiles while
state.revertstill had its beta6 shape, and the update failed on every beta6 site. - Post updates run in string order, so
_10runs before_2. That already happened: the two post updates numbered_10and_11in 1.0.x-dev sorted between_1and_2, andProfileUpgradePathTestwent red. They were renamed after what they do, which is how every post update is named from now on. - Helpers like
ProfileIslandsUpdater::splitControlsIsland()and::removeValueOnlyKeys()exist only for one beta hook.
Deleting the hooks without declaring them is not an option: a site jumping from an early beta to RC1 would skip them silently and keep invalid config.
Proposed resolution
Make 1.0.0-beta10 the required stepping stone. It ships every beta hook, including the #3624051: Failed: InvalidArgumentException after update (drush updatedb) to 1.0.0-beta7 fix, and ProfileUpgradePathTest runs the default profile of beta4 to beta7 through them.
At RC1:
- Delete
display_builder_update_11101to11106and declarehook_update_last_removed():
function display_builder_update_last_removed(): int { return 11106; } - Delete
display_builder_post_update_1to_9and declare them inhook_removed_post_updates(), all mapped to1.0.0-beta10, the release a site must install first:
function display_builder_removed_post_updates(): array { return array_fill_keys([ 'display_builder_post_update_1', // ... 'display_builder_post_update_9', ], '1.0.0-beta10'); }Core then blocks
update.phpanddrush updbon a site that skipped beta10, with a message naming it. Nothing is skipped silently. - Keep the named post updates: they are the 1.x ones.
- Remove
ProfileIslandsUpdater::splitControlsIsland()and::removeValueOnlyKeys(). KeepProfileIslandsUpdateritself: it is the tool for 1.x updates. - Replace the beta fixtures of
ProfileUpgradePathTestwith the RC1 default profile, so the test guards every 1.x update the same way.
From RC1 on:
- Never reset update hooks again in 1.x.
- Name post updates after what they do, for example
display_builder_post_update_add_scaffold_options(), not by number. - Start new
hook_update_Nat11200. - Convert stored data the schema no longer accepts in a
hook_update_N, which always runs before post updates, not in a post update.
Remaining tasks
- Tag 1.0.0-beta9 with the #3624051: Failed: InvalidArgumentException after update (drush updatedb) to 1.0.0-beta7 fix, then 1.0.0-beta10 as the last beta.
- Release notes for beta10 and RC1: update to beta10 and run
drush updbbefore RC1. - Implement the removal at RC1.
- Check on a beta7 site that
drush updbon RC1 fails with the beta10 message, and passes after beta10.
User interface changes
None.
API changes
Removed: ProfileIslandsUpdater::splitControlsIsland(), ProfileIslandsUpdater::removeValueOnlyKeys().
Data model changes
None. Sites must be on 1.0.0-beta10 before updating to RC1.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 3624068 after.png | 312.71 KB | csakiistvan |
| #15 | 3624068 before.png | 500.04 KB | csakiistvan |
Issue fork display_builder-3624068
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mogtofu33 commentedComment #3
mogtofu33 commentedComment #5
mogtofu33 commentedComment #8
mogtofu33 commentedComment #9
mogtofu33 commentedComment #10
pdureau commentedChanging the tag to have the full remaining scope in a single query: https://www.drupal.org/project/issues/search/display_builder?text=&assig...
Comment #11
pdureau commentedIt seems similar work has already been done in e4af481f (not covered by a ticket 😔):
So, I have rebased and repush, and now the MR is empty.
What is left to do here? Just removing stuff from
display_builder.install,display_builder.post_update.php...)?Comment #13
pdureau commentedI have created a new MR : https://git.drupalcode.org/project/display_builder/-/merge_requests/531/
With the expected removals, as far as I understood them:
display_builder.installdisplay_builder.post_update.php(⚠️ the named post update hooks stayed, do they need to be removed too?)::splitControlsIsland()and::removeValueOnlyKeys()fromProfileIslandsUpdaterProfileTest::testHiddenRevertStaysHidden()default.1.0.0-beta4,default.1.0.0-beta5,default.1.0.0-beta6and the related logic inProfileUpgradePathTesthave been replaced bydefault.1.0.0-rc1.yml(⚠️ knowing we may have a beta10 before the rc1, as suggested by the previous tag on the ticket)custom.1.0.0-beta6anddefault.1.0.0-beta7as advised by my LLM, but I have to admit I don't understand why the removal of those 2 caused so many additional changes inProfileUpgradePathTest✅ It is comforting no other tests have been updated and the pipeline is still green. ⚠️ But I guess the pipeline is not testing updates from release to release, so we must be careful.
✅ None of the 40 issues tagged with 1.0.0-beta has a current merge request that touches
display_builder.installordisplay_builder.post_update.php. ⚠️ But this may change in the near future, so let's be careful.Comment #14
csakiistvanComment #15
csakiistvan✅ Tested and works — applied MR !531, rebuilt caches and opened
/update.phpon a site set to schema 11103 withdisplay_builder_post_update_7to_9not run: before the MR it offered 11104–11106 and the beta post updates, after it the requirements step stops with "Unsupported schema version: Display Builder … last removed version: 11106, installed version: 11103", anddrush updbruns nothing. A site skipping the betas is now blocked instead of silently missing updates.Comment #16
pdureau commentedThanks a lot. I will do an extra manual check before sending to Jean.
Comment #17
pdureau commentedCareful, #3548769: Security: availability of endpoints according to islands is adding a hook in
display_builder.post_update.php