Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component-Extensions - Ensure the initial activation list matches #26499

Merged
merged 4 commits into from
Jun 9, 2023

Conversation

totten
Copy link
Member

@totten totten commented Jun 9, 2023

Overview

Fixes a regression from #26036 (merged circa 5.63-dev) where old-style installation procedures may produce inconsistent data.

This problem was introduced by 5.63, but it became symptomatic in 5.64 because #26208 makes the system reliant on the extension-data.

Steps to Reproduce

Perform an installation using any of the old-style installation routines, such as:

  • setup.sh
  • drush civicrm-install
  • Joomla Web Installer
  • Manual loading of civicrm.mysql, civicrm_data.mysql, or similar

For example, I used this procedure:

./bin/setup.sh -Dgsdf           ## With basic data
#./bin/setup.sh -Dgsef          ## With example data

cv flush --level=cms-full

cv vget enable_components -v
cv ext:list -Li

Note: For newer install procedures (civicrm-setup), it seems to be OK (*based on very cursory testing of cv core:install and civibuild reinstall), but I haven't stress-tested. I expect these commands worked because civicrm-setup has additional logic for initializing components+extensions.

Before

After installation, the list of "Components" and "Extensions" is out-of-sync.

After

After installation, the list of "Components" and "Extensions" match.

Technical Details

There is a structural asymmetry - when dealing with an empty/default dataset, "settings"/"components" will inherit a default activation-list; but "extensions" start with an empty activation-list. You must have a record to show the extension as active.

This patch makes the live-setting-default (for enable_components) and the stored default (for civicrm_extension) match. To ensure that they continue matching, they will need to be updated in tandem. I added comments to both sides to point this out.

totten added 4 commits June 9, 2023 14:32
The issue is that `civicrm_setting[enable_components]` and
`civicrm_extension` need to be sync'd (as of 5.63+).

But there's a structural asymmetry - when dealing with an empty/default
arrangement, "settings"/"components" will inherit a default activation-list;
but "extensions" start with an empty activation-list.

With this patch, we retain the local-active list of component-extensions.
Hopefully, that matches the default setting...
@civibot
Copy link

civibot bot commented Jun 9, 2023

(Standard links)

@civibot civibot bot added the 5.63 label Jun 9, 2023
@colemanw
Copy link
Member

colemanw commented Jun 9, 2023

@totten it looks like this will work great for now because it adds to sql/civicrm_data/civicrm_extension.sqldata.php - but that system comes with a pretty big caveat:

// Note this is a limited interim technique for installing core extensions -  the goal is that core extensions would be installed
// in the setup routine based on their tags & using the standard extension install api.
// do not try this at home folks.

We probably ought to add this to the comments, because it's not explicitly stated but "do not try at home" actually translates to "this doesn't work with extensions that have any installer logic". Namely, it doesn't work with SearchKit, so we're still going to have that problem. But at least this solves a problem so based on that, MOP.

PS: If we really can't solve the searchKit install issue, I guess a better-than-nothing hack might be to include SK tables in civicrm_generated?

@totten
Copy link
Member Author

totten commented Jun 9, 2023

@colemanw Yeah, I used the low-tech approach here because 5.63 is in RC now (want keep the RC stable; and the data isn't gonna change within 5.63.x). But I agree it's not ideal long-term -- we want something more structural/automatic.

@colemanw colemanw merged commit 8eecaf9 into civicrm:5.63 Jun 9, 2023
@totten totten deleted the 5.63-init-ext branch June 13, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants