Problem/Motivation
See #3463225: [meta] Prevent 'downgrades' from maintenance minors to older minor releases for the next major version for background.
If we add a hook_update_N() to system module in 11.1, and backport this to 10.4 using the equivalent updates API, then a site that attempts to 10.4.0 to 11.0.0 will get a requirements error that they're trying to update to a release that is too old, and they should update to a newer release instead (e.g. 11.1).
We don't currently have such an update, so the idea here is to add an empty/no-op update purely to prevent unintentional downgrades like the above from happening.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3463226-nr-bot.txt | 91 bytes | needs-review-queue-bot |
Issue fork drupal-3463226
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:
- 3463226-use-the-new
changes, plain diff MR !8900
- 3463226-10.4.x
changes, plain diff MR !10038
Comments
Comment #4
quietone commentedComment #5
smustgrave commentedSo ran the test locally without the change to system.install and the test still passes. Shouldn't it fail?
Comment #6
quietone commentedNo, it should not fail, it is independent of the system.install. The test is relying on the downgrade_test module to provide the hook and that module is only installed in the test after proving the requirement failure. The test is a functional test of the concept. There is no similar test using the actual system_update_11101 because I don't see a way to prevent the update process from finding and executing that hook.
Comment #7
xjmI left some comments on the test. It could use some clarification since update tests are dark magic.
I think we need also need the MR that adds the equivalent system update to review this properly, no?
Comment #8
quietone commentedComment #9
xjmStill NW for a second MR for 10.4, I think.
Comment #11
quietone commentedMade some minor improvements and the 10.4.x MR as well.
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
quietone commentedComment #14
catchTwo minor nits on the MR but otherwise RTBC for me. Would be good to get this in before the 10.4 beta, although I also think it could go in during the beta if necessary.
Comment #15
dwwWorking on @catch's concerns for the 10.4 MR
Comment #16
dwwFixed the nits in the 10.4.x MR, then cherry-picked those commits into the 11.x MR. Also did some more cleanup in the 11.x MR. I think this is ready for re-review.
Comment #17
dwwFYI, I opened #3487637: Move all system_update_N() methods next to each other to cleanup a weirdness once this issue is committed to 11.x and 11.1.x.
Comment #18
quietone commentedI converted the version numbers to be specific for 10.4 and 11.
Comment #19
dwwSaving credits. So far, everyone has meaningfully contributed here.
Comment #20
godotislateBoth MRs lgtm.
Comment #25
catchCommitted/pushed the two MRs to 11.x/11.1.x and 10.5.x/10.4.x respectively. Thanks!
We'll need to decide if we do a similar thing for 10.5.x->11.1.x too.
Comment #29
damienmckennaShould the 10.4, 11.0 and 11.1 release notes be updated to more clearly state this requirement in the upgrade process? While people will run into the issue if they try upgrading from 10.4 to 11.0, the issue is obscured and many are not aware of it.
Comment #30
xjmAmending attribution.