The Needs Review Queue Bot β tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to Postponed: needs info
over 1 year ago 6:28am 21 March 2023 - π³πΏNew Zealand DanielVeza Brisbane, AU
The code in question in
ModuleDependencyMessageTrait
has changed in the last three years since this issue was raised. Is this still an issue? Can we get some steps to reproduce? - π¬π§United Kingdom jonathan1055
I've updated the IS. It still is a problem, but will not be apparent until we get to core version 9.8.x. I will upload a new patch as the problem code has been moved again.
Using git_deploy I can simulate the core version being moved on to 9.8 and can then reproduce the problem. I was also looking a adding a test to the 'system' module to demonstrate what happens when the core version is 9.8.x or 10.8.x instead of 9.8.0, 10.8.0 etc.
- Status changed to Needs review
over 1 year ago 12:03pm 24 March 2023 - π¬π§United Kingdom jonathan1055
New patch for 10.1, which also applies at 9.5
- Status changed to Needs work
over 1 year ago 11:19pm 25 March 2023 - πΊπΈUnited States smustgrave
Hello @jonathan1055 reading the issue summary thanks for the update!
Adding some tasks for "Remaining tasks" section.
This will need a test case to show the problem.
Thanks!
- Status changed to Needs review
over 1 year ago 5:05pm 26 March 2023 - π¬π§United Kingdom jonathan1055
Thanks @smustgrave
I have created a new test module to simulate the version change required, and added test coverage. The test module can be moved or renamed it required. The initial push to 10.1 mr is only for the test, so it should fail.Also for speed and reduced resource usage I have temporarily limited the tests to just one class.
- @jonathan1055 opened merge request.
- Status changed to Needs work
over 1 year ago 1:11am 28 March 2023 - πΊπΈUnited States smustgrave
Test looks good!
Can you revert the one test change and think I can mark it RTBC.
- π¬π§United Kingdom jonathan1055
Thanks @smustgrave, will do.
For manual checking, the problem can be seen via UI. Install the new module (which will require local settings
$settings['extension_discovery_scan_tests'] = TRUE;
Then try to install the which now requires View >=9.2 but it is greyed out and fails the dependency check
On question I had was is it OK to hardcode 9.2 and 9.8.x in the test modules? I used 9.x as an example because I am hoping this fix can be back-ported to Drupal 9. We will be hit by the bug in core 9.8.x sooner than we reach 10.8.x.
- Status changed to Needs review
over 1 year ago 11:05am 28 March 2023 - π¬π§United Kingdom jonathan1055
Branch has a random failure so I've clicked 'do not wait for branch to pass' :-)
- π¬π§United Kingdom jonathan1055
To answer my own question from #19 about hardcoding 9.2 and 9.8.x in the tests, there are existing tests in Drupal 10 that reference core 8.x etc, and use fixtures set up for Drupal 8, so I think this is fine to have 9.2 and 9.8.x in the new test.
- Status changed to RTBC
over 1 year ago 2:04pm 29 March 2023 - πΊπΈUnited States smustgrave
Thanks for all the work on this @jonathan1055
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Crediting smustgrave for an issue summary update
-
larowlan β
committed cc05ac89 on 10.0.x
Issue #3086845 by jonathan1055, smustgrave: Module constraint checks...
-
larowlan β
committed cc05ac89 on 10.0.x
-
larowlan β
committed 1b4abdb0 on 10.1.x
Issue #3086845 by jonathan1055, smustgrave: Module constraint checks...
-
larowlan β
committed 1b4abdb0 on 10.1.x
-
larowlan β
committed d9ef1091 on 9.5.x
Issue #3086845 by jonathan1055, smustgrave: Module constraint checks...
-
larowlan β
committed d9ef1091 on 9.5.x
- Status changed to Fixed
over 1 year ago 2:26am 31 March 2023 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Committed to 10.1.x and backported to 10.0.x and 9.5.x - thanks!
- π¬π§United Kingdom jonathan1055
Thanks @larowlan for the commit and for backporting to 9.5.x
The users of git_deploy β (me included) will be grateful for this when we reach 9.8.x Automatically closed - issue fixed for 2 weeks with no activity.