Update cv library. Respect CIVICRM_BOOT or CIVICRM_SETTINGS.

[totten]

This update aims to resolve an issue identified in the discussion of civicrm/cv#166 - i.e. civix doesn't support some of bootstrap (initialization) options that cv does, which limits compatibility with some environments.

Before

• When civix needs to acces CiviCRM, it uses cv's Bootstrap.php.
• Bootstrap.php is included by way of a special/single-file download step.
• Bootstrap.php only supports the traditional (Civi-first / CIVICRM_SETTINGS) protocol. It doesn't support the newer (CMS-first / CIVICRM_BOOT) protocol.

After

• When civix needs to acces CiviCRM, it uses cv's Bootstrap.php or CmsBootstrap.php.
• All cv helper classes are included as a composer dependency (civicrm/cv-lib).
• civix's boot behavior should now match cv's behavior, as described in https://github.com/civicrm/cv#bootstrap

[jensschuppe]

Thanks, @totten, that's great work! Unfortunately, there's now a Composer dependency mismatch causing a

PHP Fatal error: Declaration of Drupal\dblog\Logger\DbLog::emergency(Stringable|string $message, array $context = []): void must be compatible with Psr\Log\LoggerInterface::emergency($message, array $context = []) in /path/to/drupal/web/core/lib/Drupal/Core/Logger/RfcLoggerTrait.php on line 20.

For civix:

$ composer depends psr/log
civicrm/cv-lib  v0.3.44 requires  psr/log (~1.1 || ~2.0 || ~3.0) 
symfony/console v4.4.34 conflicts psr/log (>=3)

while the Drupal Composer project has psr/log:3.0.0 (but at least 2.0.0 is compatible).

$ composer why-not psr/log 2.0.0
psr/log 2.0.0 requires php (>=8.0.0 but 7.3.0 is installed)

Civix uses the older version due to its config.platform setting for php being set to 7.3.0. Is that on purpose?

With civix not locking the PHP version via config.platform, this works well. Installing Composer dependencies for civix with --ignore-platform-reqs is a workaround. I'm not sure the packaged civix phar does this or not, as I'm using the repo.

[totten]

Civix uses the older version due to its config.platform setting for php being set to 7.3.0. Is that on purpose?

Yup. It matches the declarations on civicrm-core and cv. The aim is to work anywhere that CiviCRM does.

The config.platform ensures that we do our testing/QA/release with versions that are suitable for use in the PHARs. Without it, we'd quickly get packages that deviate from the minimum requirements.

(Key difference - PHARs must have packages that are compatible with the full range of environments, i.e. php73-php82. But when you locally run composer on civix.git, it only needs a version that's compatible with the local system.)

With civix not locking the PHP version via config.platform, this works well. Installing Composer dependencies for civix with --ignore-platform-reqs is a workaround. I'm not sure the packaged civix phar does this or not, as I'm using the repo.

I haven't yet reproduced the problem-scenario -- I tried a bit with D9. What's your UF? D10?

In principle, the LoggerInterface problem shouldn't affect civix.phar because it uses namespace prefixing. But actually there's a bug where namespace prefixing is de-facto off (so I wager you'll have the same problem right now). cv had a similar issue a few months back. I'm expect we can adapt civicrm/cv#158 for a fix on civix.

But parallel to that, psr/log has become a bit of a hassle since they started bumping major-versions. I dunno -cv-lib is a pretty early/low-level thing. Maybe it use an internal interface instead of psr/log...

[demeritcowboy]

I'm seeing that error with drupal 10. php is 8.1 (drupal 10 won't run with less). psr/log is 3.0.0. Happens during core:install.

PHP Fatal error: Declaration of Drupal\dblog\Logger\DbLog::emergency(Stringable|string $message, array $context = []): void must be compatible with Psr\Log\LoggerInterface::emergency($message, array $context = []) in .../web/core/lib/Drupal/Core/Logger/RfcLoggerTrait.php on line 20

Since it sounds like the problem is that cv is getting built with an older psr/log, and in my case it's a build of cv from git, I can probably just tell it to use 3.0.

[jensschuppe]

With civix not locking the PHP version via config.platform, this works well. Installing Composer dependencies for civix with --ignore-platform-reqs is a workaround. I'm not sure the packaged civix phar does this or not, as I'm using the repo.

I haven't yet reproduced the problem-scenario -- I tried a bit with D9. What's your UF? D10?

Drupal 10 with PHP 8.1.

[totten]

Dave mentioned some success after dropping psr/log requirement.

I've been trying to port civicrm/cv#158 (but it's not working yet)

[demeritcowboy]

If the port is related to symfony I did notice there was a symfony6 incompatibility when I tried to get cv to use log 3.0, since that set off a chain of requiring symfony6 and then ran into function signature difference for Civi\Cv\Application::getDefaultInputDefinition.

[totten]

@demeritcowboy Yeah, I wasn't aware of that specific conflict, but that's the kind of thing that namespace-prefixing should address. (i.e. There's nothing innately broken with Symfony4, except that Symfony4 and Symfony6 don't naturally coexist.)

Anyway, I made some progress - and this PR produces a PHAR that looks plausibly prefixed. I' can do some basic commands on my local D10+php81. (However, I'm not sure my local environment is hostile enough for testing it.)

• php-scoper - Fix prefixing #302

[totten]

Well, those bits are merged (dropping the psr/log requirement and re-enabling the namespace-prefixes on civix.phar). As far as my quick/manual inspection goes, they seem to work...

So then I tried to setup the test-matrix to include D9+D10. This is where it's a bit ucky.

• D9 is failing on several tests with failures like this:

  TypeError: htmlentities(): Argument #1 ($string) must be of type string, array given
  
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Utils/System.php:284
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Core/Menu.php:445
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Core/Menu.php:265
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Core/Menu.php:296
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php:389
  /home/homer/buildkit/build/build-2/vendor/civicrm/civicrm-core/CRM/Extension/Manager.php:319
  

• D10 is failing with a Symfony class-contract error.

  ...
  Test 'E2E\MultiExtensionTest::testAddPage' started
  Test 'E2E\MultiExtensionTest::testAddPage' ended
  ...
  Test 'E2E\SnapshotUpgradeTest::testSnapshot with data set "civixsnapshot-HEAD-empty" ('org.example.civixsnapshot-HEAD-empty')' started
  
  Fatal error: Declaration of Symfony\Component\Console\Style\OutputStyle::write(iterable|string $messages, bool $newline = false, int $type = self::OUTPUT_NORMAL) must be compatible with Symfony\Component\Console\Output\OutputInterface::write($messages, $newline = false, $options = 0) in /home/homer/buildkit/build/build-4/vendor/symfony/console/Style/OutputStyle.php on line 49
  

I haven't really looked into the D9 one. I tried the D10 first since it sounded thematically similar to the issue @demeritcowboy had before (and I was hoping to show the test passing after the merges). But the test+failure seem a bit gnarly:

1. The failure doesn't occur when running SnapshotUpgradeTest by itself. It requires some interaction with earlier tests.
2. I suspect the issue is roughly like this: the test is based on Symfony CommandTester, which does everything in one process - and this process doesn't benefit from PHAR namespace-prefixing. So it invites a sort of race between two copies of Symfony.

The first solution that comes to mind is to rearrange this test to use civix.phar, which doesn't see easy. And the thing is that this is separate from substantive compatibility issues -- it's an issue in just the test harness.

[demeritcowboy]

That first one I have come across before, and I remember just hacking it and then thinking I'd come back to it later. Let me see if I have any notes somewhere.

[demeritcowboy]

What I came across was similar but different. I did come back to it and had a series of PRs, these being the closest, but are different:

• [php 8.1 compat] Avoid CRM_Utils_System::url null for $query param civicrm/civicrm-core#24176
• [php 8.1 compat] Avoid CRM_Utils_System::url null for $query param civicrm/civicrm-core#24192

So as you've noted for the drupal 10 part this might be specific to the e2e tests.