Include the CMS Composer project's autoloader

[jensschuppe]

This avoids errors including e. g. PEAR's Log.php while loading CRM_Core_Config when using CiviCRM Core as symlinked Composer dependency in dev environments.

I'm not sure, however, whether this makes sense in all environments, but if that addition is trying to include the same autoloader again, require_once will take care of it, so it won't hurt. I'm not able to run certain cv or civix commands without it, but I'd be happy to explain my setup, if there's doubt it's necessary.

[civicrm-builder]

Can one of the admins verify this patch?

[colemanw]

add to whitelist

[totten]

civibot, test this please

[totten]

Before I forget -- in Bootstrap.php, there are functions for boot() and generate() - generally, a change in one needs a parallel change in the other. I'm pushing an update just to show what that might look like.

Also, it helps me understand the gist of the patch by omitting the conditionals/whitespace/etc -- it's basically saying:

/*  1 */ require_once $GLOBALS["civicrm_root"] . "/CRM/Core/ClassLoader.php";
/*  2 */ \CRM_Core_ClassLoader::singleton()->register();
/* +3 */ require_once '/home/totten/bknix/build/d9/vendor/autoload.php';
/*  4 */ \CRM_Core_Config::singleton();
/*  5 */ \CRM_Utils_System::loadBootStrap(array(), FALSE);

The patch adds step 3.

This avoids errors including e. g. PEAR's Log.php while loading CRM_Core_Config when using CiviCRM Core as symlinked Composer dependency in dev environments.

Well, that's fun a scenario. I've been trying to understand what makes that scenario problematic (and why this fixes it). Thinking out loud:

• CRM_Core_Config has an explicit (and very early) call to require_once 'Log.php';. Log.php was installed by composer, so you need composer's autoload.php to access any of its content.

  • The require_once statement looks a little old-school/weird, but I think that's a red-herring.

• Line #2 calls CRM_Core_ClassLoader. This ordinarily loads composer's autoload.php.

  • See: https://github.com/civicrm/civicrm-core/blob/5.63.0/CRM/Core/ClassLoader.php#L84-L99
  • Note that it tries to predict the location of composer's autoload.php using this formula:

    $top_path = dirname(dirname(dirname(dirname(dirname(__DIR__)))));

  • But in the symlinked use-case, that formula doesn't work. requireComposerAutoload() silently fails.

• Part of me doesn't want line #3 -- because we should fix requireComposerAutoload() to work as advertised.

  • But fixing requireComposerAutoload() is probably hard. I'm struggling to think of a good patch.
  • What you've got here is probably easier - because it can leverage cv's findCmsRoot(). Clever.
  • The placement of the patch (cv.git vs civicrm-core.git) has some functional differences, but it's sort of thin now-a-days. Other consumers (civicrm-core:bin/*.php, civicrm-core:extern/*.php) should have similar misbehavior due to the failure of requireComposerAutoload(). But on D8+, we've pushed people away from those. So cv is the only thing that really matters.

• This essentially sounds like an incompatibility-bug in the legacy boot protocol (aka "Civi-first boot" aka Bootstrap.php aka CIVICRM_SETTINGS)

  • I'd like to change the newer protocol (aka "CMS-first boot" aka CmsBootstrap.php aka CIVICRM_BOOT). Pending issue: Bootstrap - Switch default boot process. Update docs. #145
  • @jensschuppe Could you test your configuration with the newer protocol? My hope is that you'd see something like this:

    ## Old protocol
    $ unset CIVICRM_BOOT
    $ cv ev 'echo "OK\n";'
    Crashycrashybad for Log.php from CRM/Core/Config.php#1234 crashycrashybad
    
    ## New protocol
    $ export CIVICRM_BOOT="Auto://."
    $ cv ev 'echo "OK\n";'
    OK

(Note to self: Tested a bit with drupal9-clean and standalone-clean. They have similar file-structures and similar use-cases for composer-symlink. But findCmsRoot() yields slightly different results. The patch calls dirname() in a way that makes sense for D9's root but not standalone's root. That needs a think.)

[jensschuppe]

Thanks, @totten for elaborating!

@jensschuppe Could you test your configuration with the newer protocol? My hope is that you'd see something like this:

## Old protocol
$ unset CIVICRM_BOOT
$ cv ev 'echo "OK\n";'
Crashycrashybad for Log.php from CRM/Core/Config.php#1234 crashycrashybad

## New protocol
$ export CIVICRM_BOOT="Auto://."
$ cv ev 'echo "OK\n";'
OK

This makes cv commands successfully include the Log.php etc. (but still fails for civix commands - which uses a copy of cv's Bootstrap.php and thus still uses the legacy boot method, I guess ...?).

And:

Part of me doesn't want line #3 -- because we should fix requireComposerAutoload() to work as advertised.
But fixing requireComposerAutoload() is probably hard. I'm struggling to think of a good patch.
What you've got here is probably easier - because it can leverage cv's findCmsRoot(). Clever.

So why not just port findCmsRoot() to requireComposerAutoload() then?

[totten]

This makes cv commands successfully include the Log.php etc. (but still fails for civix commands - which uses a copy of cv's Bootstrap.php and thus still uses the legacy boot method, I guess ...?).

Cool, glad it worked for cv. It does make sense that civix needs an update -- and IMHO it needs that update regardless (since the goal is ultimately to replace Bootstrap with CmsBootstrap).

So why not just port findCmsRoot() to requireComposerAutoload() then?

Yeah, that theoretically could get your configuration to work.

I should note that there are currently two variations of findCmsRoot() (via Bootstrap.php and CmsBootstrap.php). This would create a third variant (in another repo). The logic doesn't change much, but it's hard to test, and it has been changing recently as we sort-out the file-structure for Standalone UF. Similarly, I suppose cv could find the CMS root and somehow pass it over to requireComposerAutoload() -- less duplication, but more thinking about version-interoperability.

I quite like the idea of updating civix to support CIVICRM_BOOT. It's a slightly heavier lift, but it needs to be done to support the long-term goal of standardizing on CmsBootstrap, and it should have decent compatibility.

(I'll poke around that a bit today...)

[totten]

@jensschuppe OK, based on #168 and totten/civix#300, I've published civix v23.07.1. It should support CIVICRM_BOOT.

[jensschuppe]

It does (with a minor thing to consider, see totten/civix#300 (comment)). I think this PR is now obsolete. Thanks a lot for being that quick!