Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Follow-up to #3087644: Remove Drupal 8 updates up to and including 88**.
Blocks Drupal 9 beta.
Proposed resolution
1. Remove the update
2. Add a workspace-specific message to inform any workspaces 8.8.0 users trying to go direct to 9.0.0, that they need to update to 8.8.2 or higher first.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Existing Drupal 8 sites using the workspaces module must update to at least Drupal 8.8.2 prior to updating to Drupal 9.
Comment | File | Size | Author |
---|---|---|---|
#19 | 3108416-interdiff.txt | 2.43 KB | catch |
#19 | 3108416-19.patch | 2.84 KB | catch |
#16 | Screenshot from 2020-02-27 11-29-08.png | 100.01 KB | catch |
#15 | 3108416-interdiff.txt | 1.84 KB | catch |
#15 | 3108416-15.patch | 3.15 KB | catch |
Comments
Comment #2
catchComment #3
jibranComment #4
DamienMcKennaTagging as a requirement for Drupal 9.0-beta1.
Comment #5
DamienMcKennaWhen will the decision be made on what direction to take?
Comment #6
catchGood question, tagging so this doesn't drop off the radar - we only moved it to a follow-up so the other issue could get committed as quickly as possible, but nothing blocks this one being decided and resolved.
For me I would remove it:
1. If we keep it in, we need to support a fairly complex update all the way from 8.8 to 9.4+ and there's a good chance that will cause us problems later.
2. Workspaces is still experimental, so I think it's OK to expect people to update to the latest patch release of 8.x prior to 9.x.
3. We can add some explicit messaging following #3098475: Add more strict checking of hook_update_last_removed() and better explanation to inform people exactly what they need to do if they try to update from an older database with workspaces installed.
4. The only reason we have the update post-8.8.0 was because it was fixing a critical bug in the pre-8.8.0 upgrade path, so it is not really a new update added after 8.8.0's release as such, but an old one split into two.
The main reason to keep it would be so that workspaces users don't have any different steps to do from any other Drupal 9 user however:
- other sites might also have to update to later patch releases to have other critical upgrade path bugfixes applied
- being prevented from updating via hook_update_last_removed() is better than us allowing someone to update against 9.3 or 9.4 and hitting a compatibility issue with a later update that we didn't catch with automated tests for some reason.
Comment #7
catchHere's a patch for the update removal.
Removing the post update will need to happen in #3106666: Remove post updates added prior to 8.8.0 still.
If we agree to remove the update, we'll need a follow-up here to address point #2 of the issue summary.
Comment #8
catchAnd the patch...
Comment #9
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI definitely agree that we should remove this update function in 9.0.x, it makes everything much simpler for most (99% ?) Drupal installs, and the relatively small number of sites that are using core Workspaces can easily update to the latest 8.8.x release before going to 9.0.x.
Regarding the second point form the issue summary, I've read the patch from #3098475-38: Add more strict checking of hook_update_last_removed() and better explanation and, as far as I can tell, it already doesn't allow a site to upgrade if the installed schema version is lower than the value returned by
hook_update_last_removed()
, so I'm not sure why we'd need a workspace-specific message to that requirements check..Comment #10
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedJust discussed this issue with @Berdir and now I see the usefulness of having a workspace-specific message for upgrades from 8.x to 9.x.
We agreed that either this patch or the one from #3098475: Add more strict checking of hook_update_last_removed() and better explanation should be updated, depending on which issue lands first :)
Comment #12
BerdirUnrelated PHPunit deprecation test fail.
Comment #13
alexpottThis will need a reroll - also perhaps we need to wait to see how #3098427-77: Manipulating the revision metadata keys before running the BC layer breaks the BC layer plays out.
Comment #14
xjmI guess I am very reluctantly OK with removing this given the complexities of maintaining it, including #3098427: Manipulating the revision metadata keys before running the BC layer breaks the BC layer and the fact that we've had to fix it like three times already before that. I don't totally love it because beta experimental modules are supposed to have the same upgrade path continuity requirements as stable modules, but mentioning it in the release notes (especially once #3098475: Add more strict checking of hook_update_last_removed() and better explanation is fixed) is probably an OK tradeoff given the complexities of maintaining it.
Comment #15
catchNow that #3098475: Add more strict checking of hook_update_last_removed() and better explanation has landed we can customize the error message here.
To start, here's one using exactly the same message as for system module, just with the version number changed.
Comment #16
catchComment #17
alexpottI think we can have less duplication by doing
Less duplicate strings to maintain.
Comment #18
catchThere are three cases to worry about here,, #17 breaks one of them I think.
1. You do not have workspaces installed, and are updating from a version prior to 8.8.0 to 9.x
2. You do have workspaces installed, and are updating from a version prior to 8.8.0 to 9.x
3. You do have workspaces installed, and are updating from 8.8.0 or 8.8.1 to 9.x
In the third scenario, workspaces will be in the list of modules, but system will not be, so we would not show the message.
Comment #19
catchDiscussed briefly in slack, this should reduce the duplication but also cover all cases.
Comment #20
xjmComment #21
catchComment #22
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented#19 looks great to me!
Comment #23
catchNote that the actual post update removal happens in #3106666: Remove post updates added prior to 8.8.0, which depends on #3066801: Add hook_removed_post_updates(), and also deals with workspaces' other post updates.
Comment #24
alexpottPost updates are being handled by another issue.
Comment #25
alexpottCommitted 6cad09c and pushed to 9.0.x. Thanks!
Fixed coding standards on commit.
Comment #27
alexpott