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

Extensions - Setup <classloader> during installation #20091

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

totten
Copy link
Member

@totten totten commented Apr 17, 2021

Overview

Extensions may define <classloader> rules, such as:

  <classloader>
    <psr0 prefix="CRM_" path="" />
    <psr4 prefix="Civi\" path="Civi"/>
  </classloader>

Any classloaders should take effect promptly during installation, to allow code like this:

function whizbang_civicrm_install() {
  \Civi\Whizbang\Setup::doStuff();
  \CRM_Whizbang_Setup::doMoreStuff();
}

For a more complete example, see: https://gist.github.com/totten/02e6e15c8a50aaca929b2206f2492de4

This is an extract from #20090.

Before

Classloaders are not available during installation.

The example doesn't work -- because hook_civicrm_install fires before ./Civi has been added to the classloader.

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm] cv en whizbang
Enabling extension "whizbang"


  [Symfony\Component\Debug\Exception\FatalThrowableError]
  Class 'Civi\Whizbang\Setup' not found


ext:enable [-r|--refresh] [--ignore-missing] [--level LEVEL] [-t|--test] [-U|--user USER] [--] [<key-or-name>]...

After

The classloader is updated before running install logic.

The example works:

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm/ext/whizbang] cv en whizbang
Enabling extension "whizbang"
Hello from /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/ext/whizbang/Civi/Whizbang/Setup.php@7 circa 2021-04-17 04:47:13
Hello from /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/ext/whizbang/CRM/Whizbang/Setup.php@6 circa 2021-04-17 04:47:13

Technical Details

In theory, CRM_Extension_ClassLoader has a property $this->loader which (purported to) track the real/live loader. We need access to $this->loader to register extra classloading rules. Alas,$this->loader was null. So the patch fixes that.

@civibot
Copy link

civibot bot commented Apr 17, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

This was pretty easy to test with the information provided & agree it's helpful

@eileenmcnaughton eileenmcnaughton merged commit 6c9b26d into civicrm:master Apr 19, 2021
@totten totten deleted the master-ext-instclass branch April 19, 2021 19:38
totten added a commit to totten/civicrm-core that referenced this pull request Apr 21, 2021
This is a somewhat counter-intuitive analog to civicrm#20091
which allows you to use classes during the uninstall procedure.

Why is this necessary? Recall that a typical admin will go through this lifecycle:

1. Enable extension $x
2. Disable extension $x
3. Uninstall extension $x

Step `#2` disables the classloader for purposes of regular page-loading. However, in step `civicrm#3`,
you need the classloader again
totten added a commit to totten/civicrm-core that referenced this pull request Apr 21, 2021
This is an analog/follow-up to civicrm#20091

Why is this necessary? Recall that a typical admin will go through this lifecycle:

1. Enable extension $x
2. Disable extension $x
3. Either:
   * (a) Re-enable extension $x
   * (b) Uninstall extension $x

Step `#2` disables the classloader for purposes of regular page-loading. However, when you
get to step `#3a` or `#3b`, then you need the classloader again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants