Allow for installation on Symfony 4.4

[seamuslee001]

Overview

Work in progress PR for running CiviCRM using Symfony 4.4 as is required by Drupal 9

Before

Symfony 2.8.x and 3.x are used

After

Symfony 3.x and 4.4.x are used

Technical Details

Main things are as of Symfony 3.3 all services are private by default, also the ContainerAwareEventDisapatcher is removed in 4.0 and so there is a bit of tape to support the various versions

ping @totten @MikeyMJCO @KarinG

[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).

[KarinG]

Jenkins is happy! :-)

[mattwire]

@seamuslee001 Quick question - Do we need to continue supporting 2.8?

[seamuslee001]

@mattwire I'm not sure but it seemed the simpler way out but others like @totten who have used Symfony more may have more of an opinion

[homotechsual]

I'd vote to drop 2.8 at this point if we can. It's pretty long in the tooth now and I don't see where we'd "need" it for anything backwards compatibility wise.

[seamuslee001]

I've removed 2.8.50 now and the standard install will be Symfony 3.4.x now lets see what Jenkins thinks of it

[seamuslee001]

I have removed WIP as Jenkins seems to be ok with this PR I have submitted a PR for flexmailer civicrm/org.civicrm.flexmailer#57 that would need to be merged with this PR as going to v3.4 will break things in flexmailer

[totten]

I'm also +1 for removing 2.8 on general simplification grounds.

IMHO, the initial/main reason for keeping 2.8 was general backwards-compatibility/risk-aversion on traditional build types (D7/WP/J). But now that we've had people using Symfony 3.x on D8 builds, that risk is mitigated. As a general rule, I don't think any published build processes require Symfony 2.x compat.

Another smaller, historical consideration was that cv.phar includes some Symfony libraries, and so there was some potential for mismatched versions. That's not an issue now that cv.phar uses PHP-Scoper to isolate its dependencies.

Note: Off the cuff, I would expect civicrm-core to continue needing to support the choice -- 3.x or 4.x -- for a long period... like several years... since D8 specifically expects 3.x.

Part of me thinks that maybe ~4.4 is overly precise (e.g. if it works with 3.0 and 4.4, shouldn't it also work with 4.0?). OTOH, that basically affects nobody, and we're not going to test 4.0. Also, there's an upside to setting ~4.4... in a few years, when it's time to remove 3.x, our baseline will be that much further ahead.

[seamuslee001]

@totten apart from the fact services are private by default going forward the removal of the container aware dispatcher i think is the only other significant thing which is a thing Flexmailer ran into (not sure if Mosaico worries either)

[totten]

Aah, this is a BC break that removes some functions - eg getContainer(), addListenerService(), addSubscriberService().

Personally, losing getContainer() is fine. (If someone really needed a container reference, they'd be more likely to use Civi::container() than Civi::disptacher()->getContainer()...) But losing addListenerService() feels a bit more dephell-y. Both flexmailer+mosaico use addListenerService().

I suspect that anyone on D7/WP/J + Mosaico would be looking at WSOD under normal upgrade procedures:

• If you upgrade civicrm-core first, then you'd get WSOD (i.e. because addListenerService() is used by the older/pre-upgrade ext) - so you don't get a chance to update flexmailer+mosaico.
• If you upgrade flexmailer+mosaico first, then you'd get WSOD (i.e. because ServiceClosureArgument isn't a thing in Symfony 2.8) before you can update civicrm-core.

Perhaps we can just re-create addListenerService() directly in CiviEventDispatcher (since we can't inherit it)? Then it doesn't matter how you mix/match the versions - b/c civicrm-core supports the same contract either way.

I've pushed up a commit which does that. flexmailer (both current master and latest v1.1.1) passes its unit-tests run with this.

(In principle, re-creating addSubscriberService() would make sense too... but I don't think anyone in universe uses it. The code in \Civi\FlexMailer\Services::registerListeners() doesn't matter because that function never runs, and it hasn't since v0.1.-alpha1...)

[seamuslee001]

Yeh that is ok by me @totten

[seamuslee001]

just added the needs documentation flag so we can make sure to remember to update dev docs

[totten]

Flagged as mergeable - I think the main BC break is mitigated, and no one seemed to think of any other problems.

The specific dev-doc that needs updating is here: https://docs.civicrm.org/dev/en/latest/hooks/usage/symfony/#methods

[totten]

jenkins, test this please

[homotechsual]

Has a docs PR civicrm/civicrm-dev-docs/pull/810 which is approved (by me at least!) for merge with the core release.

[seamuslee001]

I'm going to merge this now given no obvious objections coming through

[KarinG]

Thank you @seamuslee001 👍