Prefix four dependencies during composer install/update (symfony/console, twig, monolog & php-di)

[diosmosis]

Fixes #16905
Refs #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: matomo-org/plugin-GoogleAnalyticsImporter#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": [
            "@php ./console composer:prefix-dependency"
        ],
        "post-update-cmd": [
            "@php ./console composer:prefix-dependency"
        ]
    }

or

    "scripts": {
        "post-install-cmd": [
            "@php ../../console composer:prefix-dependency --plugin=MyPlugin"
        ],
        "post-update-cmd": [
            "@php ../../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

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[diosmosis]

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

[tsteur]

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]

@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]

@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]

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]

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

[tsteur]

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

[diosmosis]

@sgiehl OneClickUpdate tests are passing. The other failures look unrelated, let me know if otherwise.

[github-actions]

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

[github-actions]

This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers

[github-actions]

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

[github-actions]

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

[github-actions]

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.