Generalise typo3/phar-stream-wrapper so CiviCRM can be installed on D8.7

[seamuslee001]

Overview

CiviCRM v5.24.3 cannot be installed onto a Drupal 8.7 (or earlier) site using straight composer because there is a conflict between CiviCRM's requirement of ^3 and drupal's ^2.1.1.

This problem does not affect Drupal 8.8 (or later).

Before

CiviCRM cannot be installed on drupal 8.7 using composer from v5.24.3 onwards

After

CiviCRM can be installed ontop of Drupal8.7 using composer

ping @totten

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

This sounds pretty sensible. If you test it and the bits of fb37219 are working, then 👍 from me.

[seamuslee001]

A d8 build using this seems to work fine, I think other builds should be fine as well but would appreciate some additional r-run from others

[pfigel]

This exposes a weird edge-case.

There's a small difference in the signature for TYPO3\PharStreamWrapper\Assertable::assert between 2.x and 3.x:

2.x:

public function assert($path, $command);

3.x:

public function assert(string $path, string $command): bool;

In core, we implement the interface in Civi\Core\Security\PharExtensionInterceptor like this:

public function assert(string $path, string $command): bool {

This would throw a fatal against the 2.x version as the signature is not compatible. However, since Drupal already ships with with its own PharExtensionInterceptor, this code should never be reached on Drupal. In other words, this would only be a problem on WordPress or Backdrop if there's some kind of installation scenario where composer resolves to the 2.x version. This does seem somewhat brittle to rely on.

We could also decide to lock to the 2.x version and implement that signature in Civi\Core\Security\PharExtensionInterceptor, but then we might run into issues once Drupal decides to bump its version of typo3/phar-stream-wrapper.

Not really sure what to do here.

[seamuslee001]

yeh so @MikeyMJCO reported no issues with Drupal 8.8 and that seems to be because 8.8 is locked to 3.1.3 https://packagist.org/packages/drupal/core#8.8.x-dev where as 8.7 is 2.1.1 https://packagist.org/packages/drupal/core#8.7.x-dev. 8.7 is still security supported but not for much longer so not sure what to do here

[totten]

If we could determine the version number programmatically, then it should be possible to make an adapter. Pseudocode:

• PharExtensionInterceptor portability - Anonymous class approach
• PharExtensionInterceptor portability - Trait approach

The trick is determining the version. There are are some composer plugins which can log package versions...

[jackrabbithanna]

I just want to mention that Drupal 8.7 has it's end of life here in a few weeks when 8.9 is released. Once that happens 8.7 will not receive official security support, and sites should upgrade to 8.8 or preferably 8.9

[seamuslee001]

@jackrabbithanna yeh that is a good point, I ran into this with a specific client so figure may as well try a PR but don't have a problem if this gets declined

[jackrabbithanna]

I'm sure a few will want latest Civi to work with 8.7 .. I just wouldn't go through too many backflips on something that should be irrelevant very soon.

[totten]

So, the signals so far indicate that this is better (certainly no worse) than before, and we've got a plausible theory for this will work-out (even in the case where the code gets matched-up with an incompatible revision of Assertable). We even have a possible contingency in case it really needs to be compatible with both revisions of Assertable.

Some QA should be done on diff composer situations, but there's not much we can do about that before merging.

Let's go ahead with merge and do some builds and E2E tests on the merged dev branch.