Skip to content

Drupal 12 Support - More BC Alternative - #127

Merged
demeritcowboy merged 3 commits into
civicrm:masterfrom
jackrabbithanna:drupal12-proto2
May 16, 2026
Merged

Drupal 12 Support - More BC Alternative#127
demeritcowboy merged 3 commits into
civicrm:masterfrom
jackrabbithanna:drupal12-proto2

Conversation

@jackrabbithanna

Copy link
Copy Markdown
Contributor

Alternative PR to #126

Instead of using the OO Install / Update object/plugins, use hooks in civicrm.install.

Requires that civicrm.install be required in civicrm.module.

Adds an implementation of hook_runtime_requirements() to show requirements as before the Drupal status page.

This is less good that the other IMO, but #126 will require a separate branch/repo because it will not be backward compatible with Drupal 9/10

@jackrabbithanna
jackrabbithanna marked this pull request as draft May 14, 2026 22:30
@jackrabbithanna

Copy link
Copy Markdown
Contributor Author

The change record necessitating the require in the civicrm.module of civicrm.install https://www.drupal.org/node/3554900

Comment thread civicrm.install Outdated
];


foreach ($setup->checkRequirements()->getMessages() as $msg) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this block supposed to be here? In the original hook_requirements this was copied from, it says only do this block if $phase !== 'runtime', but this new hook_runtime_requirements is the equivalent where $phase == 'runtime'.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it was intention, without this bit then the list of requirements in the Drupal status page doesn't show. Stuff like:

CiviCRM: Database: Can successfully lock and unlock tables
CiviCRM: Database: Connected
CiviCRM: Database: Database supports MySQL triggers

It can be made to be gone though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it came from here: #100.

An alternative would be something more fine-grained so could control externally what exactly runs when, but is it worth the effort? shrug

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I swear those things used to be there, maybe not recently. I was just testing fresh stock 6.14 D10 install and they're not there now. Ok, well then we can also decide if we need the hook_runtime_requirements at all if we don't want those to show on the status page.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this is a good point :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep 08cad4c
well, I guess somebody didn't want the requirements listed. its fine, I'll update this MR

@jackrabbithanna

Copy link
Copy Markdown
Contributor Author

Removed hook_runtime_requirements() implementation because we had removed runtime requirements in the past :) Now this PR is real simple. I'm doing installation tests on various 10/11 versions before I undraft it.

@jackrabbithanna
jackrabbithanna marked this pull request as ready for review May 15, 2026 22:24
@jackrabbithanna

Copy link
Copy Markdown
Contributor Author

Tested install with 10.3, 11.3 and 12.x-dev .. working good for me. After successful install additional modules can be installed/uninstalled without issue.

@demeritcowboy

Copy link
Copy Markdown
Contributor

Works for me.

@demeritcowboy
demeritcowboy merged commit 9a74e53 into civicrm:master May 16, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants