@diosmosis opened this Pull Request on September 7th 2022 Member

Fixes #16905
Refs https://github.com/matomo-org/matomo/issues/8568

In environments when Matomo runs alongside other PHP software, such as in Matomo for WordPress, hard to debug issues caused by conflicting dependencies can arise. Given there is no way to know exactly which other versions of dependencies might
end up being used in a user's setup, dealing with them on a version-by-version basis is undesirable (if at all possible).

This PR provides the ability to prefix dependencies in core and in plugins as part of the composer workflow, making sure the
dependencies we use do not conflict with different versions in other software.

twig & monolog are currently configured to be prefixed in this PR. An example of prefixing composer dependencies in a plugin can be found here: https://github.com/matomo-org/plugin-GoogleAnalyticsImporter/pull/279

Adding support for dependency prefixing

Prefixing is accomplished via a new composer:prefix-dependency command. It is added as a post install and post update script in composer.json files, eg:

    "scripts": {
        "post-install-cmd": [
            "<a class='mention' href='https://github.com/php'>@php</a> ./console composer:prefix-dependency"
        ],
        "post-update-cmd": [
            "<a class='mention' href='https://github.com/php'>@php</a> ./console composer:prefix-dependency"
        ]
    }

or

    "scripts": {
        "post-install-cmd": [
            "<a class='mention' href='https://github.com/php'>@php</a> ../../console composer:prefix-dependency --plugin=MyPlugin"
        ],
        "post-update-cmd": [
            "<a class='mention' href='https://github.com/php'>@php</a> ../../console composer:prefix-dependency --plugin=MyPlugin"
        ]
    }

For plugins, the dependencies that you want prefixed are listed in the plugin.json file, in the prefixedDependencies property. For core, the list is hardcoded in the Prefixer class.

Prefixing a dependency

Once the command is added to composer.json, you can add dependencies to prefixedDependencies. However, this alone may not be enough. If the dependency you want to prefix dynamically references classes and functions, php-scoper, the tool used to prefix the namespaces, may not be able to detect and change those uses. In that case, you need to manually replace code via a patcher that you define in scoper.inc.php. An example of this is the extra replaces done for twig in the scoper.inc.php file for Matomo core, and https://github.com/matomo-org/plugin-GoogleAnalyticsImporter/blob/a7f6b48bec018338bb815bdf9bfa2e17223aa5f7/scoper.inc.php for google's compiled PHP protobuf code.

How prefixing is accomplished

Prefixing is accomplished using the php-scoper project. php-scoper processes a folder, adding a prefix to all namespace/use
statements, and anything else it recognizes. It works pretty well, but doesn't replace more dynamic usages. Those can be
replaced manually, however, by adding code to a scoper.inc.php file (as mentioned above).

The entire process, encapsulated by the composer:prefix-dependency command and the Piwik\Dependency\Prefixer class is as follows:

  • if php-scoper does not exist locally and a custom path was not given, the php-scoper phar is downloaded from a hardcoded link
  • determine what dependencies should be scoped. for core, it's hardcoded in a constant. for plugins, it's in the prefixedDependencies property in the plugin's plugin.json file.
  • child dependencies are recursively gathered. child dependencies are prefixed as well to avoid dependency conflicts with them in addition to the root dependency.
    • at this point we also read the composer.json file of every dependency to prefix and gather the namespaces of those dependencies (via the psr-4 autoload configuration).
  • if prefixing for a plugin and the plugin does not have a scoper.inc.php file, an empty one is generated
  • php-scoper is run to prefix the main namespaces of the dependency. The prefix is either Matomo\Dependencies for core or Matomo\Dependencies\{PluginName} for a plugin.
    • the dependencies to prefix are made available to the scoper.inc.php file via the MATOMO_DEPENDENCIES_TO_PREFIX environment variable. scoper.inc.php uses a Finder to limit the processed files to those dependencies' directories.
    • the namespaces to prefix are made available to the scoper.inc.php file via the MATOMO_NAMESPACES_TO_PREFIX environment variable. scoper.inc.php specifies those namespaces in a include_namespaces configuration for php-scoper (NOTE: this option is actually added via a fork on my personal github, more info below.) This lets us limit prefixing to just the dependencies we want to prefix.
    • the prefixed files are output to a vendor/prefixed directory
  • if prefixing for a plugin, a second php-scoper pass is done, this time with Matomo\Dependencies as the prefix and the namespace just for prefixed core dependencies. So in core, since we prefix monolog, this part of the process will replace uses of Monolog in plugin dependencies with Matomo\Dependencies\Monolog, for instance.
    • the output here goes to a prefixed2 directory, which is then swapped w/ prefixed so the result of the second pass is in the prefixed/ folder.
  • the prefix-dependency command now uses composer to dump an autoload file for the prefixed/ directory, if it exists.
  • the dependencies that were prefixed are now removed from the vendor/ directory.
  • then we run the dump-autoload command again for the root vendor/ directory, so it no longer references dependencies that are now in prefixed/.
  • finally, a new autoload.php file is generated which loads the vendor/ autoloader, and, if it exists, the vendor/prefixed/ autoloader.

The process is slightly different if prefixing for a plugin that has an empty prefixedDependencies property. In this case, since we don't need to prefix anything, but still need to replace references to prefixed core dependencies, only the second php-scoper pass is done.

include_namespaces

Currently php-scoper only has exclude_*** configuration options. This is pretty inconvenient when we want to prefix X and exclude everything else, so I added an include_namespaces feature in https://github.com/diosmosis/php-scoper/tree/included-namespaces. An official fork in the matomo-org organization will need to be made eventually. No other changes to php-scoper are needed, and we probably won't need to ever update it. I'll try to get the changes accepted upstream after this is reviewed.

Review

@diosmosis commented on September 7th 2022 Member

FYI @tsteur. Not sure if there's anyone else I should ping, apologies if I missed someone.

@tsteur commented on September 8th 2022 Member

Nice, I just checked out the branch and ran the command and it renamed things nicely.

Ideally, when running composer in the future then it would indeed need to run php-scoper directly as part of the composer command.

It also allows using no prefixed dependencies, so Matomo can be distributed normally and any side effects of prefixing
can be limited to Matomo for WordPress, where the benefits of prefixing outweigh any potential issues.

Not fully understanding what is meant by this one @diosmosis ?

Would that mean that if someone uses Matomo for WordPress, and they install another Matomo plugin from the Marketplace in their WordPress, and this marketplace doesn't use the prefixed version, then it may cause issues that it could use a wrong vendor version from a completely different WordPress plugin?

I guess if we were to only replace Twig and Monolog this could be fine potentially as it's maybe usually not expected that these libraries would be used too often directly.

I guess from my perspective I just want to keep On-Premise and Matomo for WordPress as similar as possible to not have any unnoticed possible regressions in Matomo for WordPress because they run different code. Looking at the PR I think that doesn't seem to be the case though if I understand it right and would only apply to plugins.

@diosmosis commented on September 8th 2022 Member

@tsteur

Would that mean that if someone uses Matomo for WordPress, and they install another Matomo plugin from the Marketplace in their WordPress, and this marketplace doesn't use the prefixed version, then it may cause issues that it could use a wrong vendor version from a completely different WordPress plugin?

Yes, that could be an issue. If scoping in the composer workflow, then I guess the plugin developer would be forced to use the prefixed namespace otherwise it wouldn't work for a normal Matomo, so this wouldn't happen.

I guess from my perspective I just want to keep On-Premise and Matomo for WordPress as similar as possible to not have any unnoticed possible regressions in Matomo for WordPress because they run different code. Looking at the PR I think that doesn't seem to be the case though if I understand it right and would only apply to plugins.

I understand. For a proof of concept I wanted to provide a solution the was as minimal as possible and show that if it turns out to be more complicated to prefix a dependency correctly and predictably, there was an easy way to go back. I also wanted to show its possible to externalize the mechanism and not have it in core if desired. All of this mostly because there was a lot of manual replacing required for twig, though it's probably an exception given it defines so many functions and generates php code that uses them.

Regardless, I can make it part of the composer workflow if desired, should I move forward with that? (I don't have a big opinion here, I think it's workable either way.)

@diosmosis commented on September 8th 2022 Member

@tsteur I'm going to assume adding to the compost workflow is required. Can you tell me what release this is intended to be a part of or point me to someone who can?

@sgiehl commented on September 8th 2022 Member

We shouldn't do that before Matomo 5. As plugin developers should adjust their code, that imho shouldn't be done in a minor release. Also using the scoped classes in plugins would make those incompatible with older Matomo releases...

@diosmosis commented on September 12th 2022 Member

Clarified on slack this should be targeting 5.x-dev.

@tsteur commented on September 13th 2022 Member

Sorry about the late reply

Regardless, I can make it part of the composer workflow if desired, should I move forward with that? (I don't have a big opinion here, I think it's workable either way.)

Yes, that be great

@github-actions[bot] commented on October 3rd 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@diosmosis commented on October 11th 2022 Member

@sgiehl

Do we need to increase the PHP requirement to PHP 7.4? Looks like php scoper requires at least 7.4

Just for development/git installs.

Also I was wondering if we should prefix all usage of vendor libs. We could let the autoloader remove the prefix if it can't be found prefixed. That would allow us to easily prefix other libs later if they cause trouble, without having another breaking change thinking

Only issue I can think of regarding this is that it would break PHPStorm's ability to explore the code. Otherwise it seems doable (assuming it will be ok to prefix libraries in minor releases). @tsteur can you comment on this?

@tsteur commented on October 11th 2022 Member

Also I was wondering if we should prefix all usage of vendor libs. We could let the autoloader remove the prefix if it can't be found prefixed. That would allow us to easily prefix other libs later if they cause trouble, without having another breaking change thinking

Only issue I can think of regarding this is that it would break PHPStorm's ability to explore the code. Otherwise it seems doable (assuming it will be ok to prefix libraries in minor releases). @tsteur can you comment on this?

I don't have any thoughts on this. What be the benefits and value of autoloading it for all libs, and what would be the risks and possible problems in the future? @sgiehl

We could let the autoloader remove the prefix if it can't be found prefixed.

Would that mean it could still cause incompatible plugins on WordPress? Like other plugins in WordPress wouldn't use the prefix so checking if that would then trigger the autoloading of our libs with a different version instead of their lib?

@diosmosis commented on October 11th 2022 Member

@tsteur

Would that mean it could still cause incompatible plugins on WordPress? Like other plugins in WordPress wouldn't use the prefix so checking if that would then trigger the autoloading of our libs with a different version instead of their lib?

I think in this case it's just doing a class alias of Matomo\Dependencies\A\B\C => A\B\C, but ONLY if Matomo\Dependencies\A\B\C isn't found. wordpress plugins wouldn't use the Matomo\Dependencies\... format.

@tsteur commented on October 11th 2022 Member

I see 👍 All good

@diosmosis commented on October 12th 2022 Member

@sgiehl @tsteur I implemented this locally, and it definitely makes PHPStorm useless in this area:

image

Can I confirm it's worth the trade off? (To reiterate, if we prefix all references to dependencies (not the vendor files themselves), then require in matomo docs that plugins always use Matomo\Dependencies\ even if the dependency itself isn't prefixed, then we can prefix vendor files in the future w/o a BC break. Though if a plugin didn't use Matomo\Dependencies\ it would still work even though our docs tell them not to.)

@diosmosis commented on October 16th 2022 Member
@tsteur commented on October 17th 2022 Member

As long as we don't use them it should be fine 👍 It could reduce possible upgrade issues if we were to prefix more libraries in the future maybe?

@tsteur commented on October 17th 2022 Member

@sgiehl any thoughts otherwise?

@sgiehl commented on October 17th 2022 Member

That was actually just a random thought that came to my mind. I didn't think about code completion...
Not having class auto completion for dependencies might make the development a bit harder / slower for some cases.
Guess the only way to circumvent that would be to create class aliases for development at least for those classes we are using, so PHPStorm would be able to find them.
Nevertheless we would need to ensure that we can't accidentally use the original class names, as this could even cause more trouble.

Another random thought:

Not being able to develop and run our tests on PHP 7.2/7.3 anymore will make it hard to detect possible regressions on older PHP versions early. So I would suggest not do this for core, unless we increase the requirements. Renaming the dependencies in the build process only and maybe some tests, would change that maybe...

Overall I'm still not convinced that renaming composer package namespaces will solve more issues for wordpress users than we might end up having issues for on premise users caused by that in the end.

@diosmosis commented on October 17th 2022 Member

Not having class auto completion for dependencies might make the development a bit harder / slower for some cases.
Guess the only way to circumvent that would be to create class aliases for development at least for those classes we are using, so PHPStorm would be able to find them

I'll see if I can find a solution here.

Not being able to develop and run our tests on PHP 7.2/7.3 anymore will make it hard to detect possible regressions on older PHP versions early. So I would suggest not do this for core, unless we increase the requirements. Renaming the dependencies in the build process only and maybe some tests, would change that maybe...

We're currently using a fork of php-scoper. It's possible we might be able to just downgrade the minimum required version there.

Overall I'm still not convinced that renaming composer package namespaces will solve more issues for wordpress users than we might end up having issues for on premise users caused by that in the end.

I suspect the value here is in lowering churn and increasing adoption, not (entirely) in reducing bug reports.

@diosmosis commented on October 18th 2022 Member

tests running on 7.2 again, haven't figured out the other item yet

@tsteur commented on October 18th 2022 Member

I suspect the value here is in lowering churn and increasing adoption, not (entirely) in reducing bug reports.

Indeed . Ideally it also reduces additionally the bug reports. If we're wanting to get few million WordPress installs for example then it will be only possible if we're compatible with most plugins. Currently, we're for example not compatible with WP-Rocket meaning a few million possible installs are already lost as an opportunity. And that's only one incompatibility with one plugin. There may be many more. The more we reduce these, the more installs we can get.

Not having class auto completion for dependencies might make the development a bit harder / slower for some cases.
Guess the only way to circumvent that would be to create class aliases for development at least for those classes we are using, so PHPStorm would be able to find them

I assumed we have auto completion for the renamed classes, but not for classes that aren't prefixed anyway. In that case we would still use the regular class name. Having the class names work for other namespaces too may be helpful few years into the future if we decide to prefix more libraries and then it can reduce issues when someone upgrades maybe. We could also remove it though if we don't find value in it and only have the prefixed classnames that we are actually going to use.

Overall I'm still not convinced that renaming composer package namespaces will solve more issues for wordpress users than we might end up having issues for on premise users caused by that in the end.

Not sure if I missed it. It's great you're bringing this up. We definitely don't want to make other things too worse but make a balanced decision. Can you outline the possible issues/risks again and when they might occur? So we can make a decision if we are going to merge this or not.

@sgiehl commented on October 18th 2022 Member

Not sure if I missed it. It's great you're bringing this up. We definitely don't want to make other things too worse but make a balanced decision. Can you outline the possible issues/risks again and when they might occur? So we can make a decision if we are going to merge this or not.

  • Prefixing dependencies will always contain the risk, that we might not be able to update the dependency in the future, if a newer version contains code, that can't be processed correctly by php-scoper.
  • I'm not able to say if renaming such basic dependencies might cause any issues during an update
  • Requiring Matomo in another composer projects might no longer be possible, if the project shares the (renamed) dependencies

Assuming we find a way to fix the auto-completion, when using all vendors prefixed (even if they aren't). Couldn't we only do the prefixing for the wordpress version then?
This would have the benefit, that we could even manually apply adjustments to the vendor libs if needed, as those libs are even commited to git there.
And it would fully remove any risk for on premise users.

@tsteur commented on October 19th 2022 Member

@sgiehl could think about only prefixing them in Matomo for WordPress.

If we were to prefix libraries in WordPress, how would we deal with plugins that access these properties? Maybe if it's only twig and monolog then it would work as maybe most plugins wouldn't use it directly but maybe our plugins would or someone else's. Would this be fixable?

If we were to prefix PHP-DI then for sure I would expect some issues as most plugins use it for example.

Although it could have side effects if we only prefix it in Matomo for WordPress meaning we would need to maintain a fork of Matomo and run Matomo + all plugin tests each also on forks to guarantee Matomo works nicely on Matomo for WordPress when they are prefixed there?

@tsteur commented on October 24th 2022 Member

Any thoughts @sgiehl ? If we could prefix in the WordPress library generally that be great since we ideally also need to prefix PHP-DI as we get reports about incompatibility more regularly recently.

Generally, to increase adoption and reach our targets we need ideally some solution to vendor conflicts problem.

@github-actions[bot] commented on October 31st 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@diosmosis commented on November 3rd 2022 Member

@tsteur @sgiehl fyi, updated to detect if being installed as a composer dependency, and if so, skips prefixing

@tsteur commented on November 4th 2022 Member

@sgiehl assuming installing Matomo via composer, do you still see any issue there?

There's obviously the problem that if php-scoper can't replace it automatically that we need to patch composer.json or php-scoper to make this work. This we would need to do though either way whether we do it only for Matomo for WordPress or also for Matomo.

Any further thoughts, risks, problems or requirements we should consider? Or any other solutions in mind we should consider?

From my perspective the only other vendor lib we should eventually look at also would be PHP-DI. This could potentially wait though for Matomo 6. Abstracting DI to use \Matomo\DI or something can already be a first start towards this maybe.

@diosmosis commented on November 4th 2022 Member

There's obviously the problem that if php-scoper can't replace it automatically that we need to patch composer.json or php-scoper to make this work. This we would need to do though either way whether we do it only for Matomo for WordPress or also for Matomo.

I'm not sure exactly what this means, but I think it's not worth worrying about whether php-scoper won't work with a package. In this case, you would define a patcher in scoper.inc.php, and all those do is take a file's string contents and modify them. Essentially, if you can fork a repo and change the namespace by hand, php-scoper will work as all it does is automate that process. The only 'downside' is that you'd have to have at least some knowledge of the internals of a dependency, and users of php-scoper not being the authors of the dependencies they want to prefix, that knowledge would not be perfect. Which is why I did the GoogleAnalyticsImporter proof of concept.

@TheRealMisterFix commented on November 4th 2022

Not sure if this is related, but I'm assuming so... I have WP-Matomo installed on a wordpress site, and had it configured as "Self-Hosted (PHP API)", worked fine, connected to our local Matomo instance.

I installed the WP-Rocket plugin, and the site stopped working, with the following error:

[04-Nov-2022 14:31:39 UTC] PHP Fatal error:  Declaration of Piwik\Plugins\Monolog\Handler\FileHandler::write(array $record) must be compatible with Monolog\Handler\StreamHandler::write(array $record): void in /var/www/analytics.SITENAME.com/html/plugins/Monolog/Handler/FileHandler.php on line 22

I removed the WP-Rocket plugin manually, and switched WP-Matomo to "Self-Hosted (HTTP API), default", reinstalled the WP-Rocket plugin, and it worked fine.

@github-actions[bot] commented on November 12th 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@diosmosis commented on November 17th 2022 Member

@tsteur @sgiehl prefixed php-di as well now. Also added a UI test for a composer installed matomo (similar to LatestStableInstall + the OneClickUpdate test), and fixed a couple issues to get it to work.

Regarding updating, updating from 4.X will work for some versions as long as it has the one click updater code that initiates CliMulti right after updating the downloaded files (there's a hack in CliMulti's constructor that gets this to work). For all other versions however, the one click update will probably fail. In these cases, the update path would be manual install via core:update, or updating to the latest 4.x release, then updating to 5.0. Can the latter be something that's forced via release channels?

@tsteur commented on November 17th 2022 Member

however, the one click update will probably fail. In these cases, the update path would be manual install via core:update, or updating to the latest 4.x release, then updating to 5.0. Can the latter be something that's forced via release channels?

That's something that we should be able to enforce. If they are eg currently on a 2.X release, then we could force them to update first to a 4.X release. I've never even thought about this but that may be in the future even a great strategy that allows us to make more changes there when people are like 2 versions behind.
fyi 67% are already on Matomo 4. 92% are on Matomo 3 or newer.
In 3.13 we added added this https://github.com/matomo-org/matomo/pull/15770 with the redirect so I'm thinking anyone on that version (without testing, just a guess) would be maybe fine and not run into any problems which is 78%. Others we could force to update to Matomo 4.x first.

@sgiehl can you review this PR and test it?

@sgiehl commented on November 24th 2022 Member

@tsteur I've added the review to my todo list. But it might take some time till I'm able to do it. I've got a lot other tasks to do atm.

@sgiehl commented on December 1st 2022 Member

@diosmosis how much effort would it be to get php scoper supporting PHP 7.2?
Currently it's not possible to checkout Matomo and run a composer install on PHP 7.2.
As we still officially support this version, I think this might be a mandatory requirement. Otherwise even developing with PHP 7.2 would no longer be possible.
@tsteur what are your thoughts on that?

@diosmosis commented on December 1st 2022 Member

@sgiehl

how much effort would it be to get php scoper supporting PHP 7.2?

Too much. It would be a major fork, especially considering their main branch has already moved on from php 7 altogether.

You can still develop on php 7.2, you just have to run php-scoper w/ php 7.4. It's not easy but it's do-able. travis-ci does this.

@sgiehl commented on December 1st 2022 Member

But this won't allow to install matomo from a git checkout on PHP 7.2 anymore.
If that isn't possible anymore we might need to discuss with @tsteur how to proceed with this.

@diosmosis commented on December 1st 2022 Member

But this won't allow to install matomo from a git checkout on PHP 7.2 anymore.

AFAIK this isn't a supported workflow for production installs.

@tsteur commented on December 2nd 2022 Member

We could always remove support for this specific feature for PHP 7.2 and PHP 7.3 if that helps? Meaning people will need PHP 7.4+ if they want to install from git.

Or is there a chance to run PHP-scoper only for PHP 7.4 and newer similar to as if it was installed as a composer package?

@diosmosis commented on December 2nd 2022 Member

@tsteur

Or is there a chance to run PHP-scoper only for PHP 7.4 and newer similar to as if it was installed as a composer package?

this is doable I think. Is this the preferred solution?

@tsteur commented on December 2nd 2022 Member

any thoughts @sgiehl ?

@sgiehl commented on December 2nd 2022 Member

Doing the renaming depending on the PHP version doesn't sound like a good solution.
If we officially support using Matomo prefixed and unprefixed, we are again back to the problem, that we would actually need to run all our tests with both solutions. Same applies for all plugins, as we otherwise can't ensure that everything runs smoothly.
If we gonna do that, we can also let Matomo by default not rename the dependencies and only do that for wordpress.

The other solution we could go with is imho drop support for PHP <7.4 (at least for git checkout) and also drop support for using Matomo as a composer package. Meaning that we always prefix everything. Everything else might imho be too much effort or too risky.

@tsteur commented on December 6th 2022 Member

We can remove the support of installing Matomo via Git for PHP 7.2 and PHP 7.3. We only need to make sure to mention this clearly in the changelog and also we can update https://matomo.org/faq/how-to-install/faq_18271/ . If people do try to install it via Git, ideally they get a good error message mentioning the problem and how they can fix it (by upgrading PHP) if possible. If it's not possible then please let me know and I'll create an FAQ with the error message people would get in such a case so that people can at least easily find the solution to the problem.

Regarding installing Matomo via composer: When someone is trying to install Matomo via composer is it only a problem if they also use one of our dependencies that we prefix? Or is it generally a problem and not working?

@sgiehl commented on December 6th 2022 Member

For composer it should only be an issue if the same packages would be used. But our composer.json would actually be "broken" for PHP <7.4 with the changes on this branch. On the one side we define to require PHP > 7.2, but then try to execute a command on install that requires a library that needs PHP 7.4. So that would fail on PHP <7.4.
But changing the requirement to PHP 7.4 in composer.json also isn't possible, as Matomo should still run with PHP 7.2 and otherwise other dependencies might get updated and require PHP 7.4.

Installing via git on PHP <7.4 would result in a PHP Scoper error message when running a composer install.

It will also no longer be possible to easily develop with PHP 7.2, as you would also require PHP 7.4, which is kind of useless.

In general we could maybe announce that Matomo 5 will require PHP 7.4, but provides a limited support for PHP 7.2 & PHP 7.3.

@tsteur commented on December 7th 2022 Member

Is there a chance we can hook into one of the events and check for the PHP version and accordingly adjust the error message for certain PHP installs? https://getcomposer.org/doc/articles/scripts.md#command-events

Generally, we still want to fully support PHP 7.2+ for people who install it via our build.zip. Limiting support could otherwise break it for 19% of our users. Or I'm not sure what would change if we "limit support" because to support it we'd still need to effectively run the tests on Travis for 7.2 and we'd need to be able to test it locally if needed (eg by running composer with PHP 7.4 and then switching to PHP 7.2).

People installing it via git/composer and using PHP 7.2/7.3 should be a very small minority (<= 1%) and it be acceptable to break it/ removing support for it. We only need to make the experience as nice as we can and have 1 or 2 FAQs for it and we can mention that for these people we effectively remove the support 👍 .

We can also break it for installs where they embed Matomo in a project via composer and they have one of the same dependencies that we are prefixing. That should be a very small number where this happens.

A completely different way could be to push dependencies into vendor directory but I'm assuming that's not something we want :-)

@sgiehl commented on December 7th 2022 Member

With limited support I didn't mean to change anything in the code. We should still support 7.2 and run tests on it.
But we could maybe list PHP 7.4 as requirement (or at least as recommendation) for Matomo 5 e.g. here: https://matomo.org/faq/on-premise/matomo-requirements/
And maybe add a note, that Matomo 5 is still working and tested on PHP 7.2 & PHP 7.3 (when installed with the package), but that we might drop support for it in the near future.

@tsteur commented on December 8th 2022 Member

👍 we could add a sentence that for installing via git/composer certain special requirements apply and can also mention it on the dedicated pages for installing via git etc 💯

Could also generally add a sentence that we don't recommend using PHP versions that no longer receive security updates 👍

@github-actions[bot] commented on December 16th 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

Powered by GitHub Issue Mirror