Add Composer plugin to correctly autoload PHP CSS Parser dependency #6464

pierlon · 2021-07-20T01:49:38Z

Summary

The PHP CSS Parser dependency currently will not autoload due to Composer using the psr-4 namespace path specified in the master branch of that repository. Possible ways of resolving the issue include forking the repository to update the psr-4 namespace, or waiting for the package maintainer to release an update with the updated namespace.

This PR provides an alternative way to solve the issue, which I think would be easiest to use and maintain in the short term. By making use of a Composer plugin, we can programmatically update the psr-4 namespace of the package in the composer.lock file and the instance of the package Composer uses just before the autoload files are generated.

Checklist

My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

pierlon · 2021-07-20T01:57:16Z

php-css-parser-install-composer-plugin/src/Plugin.php

+		$package_locker   = $event->getComposer()->getLocker();
+		$local_repository = $event->getComposer()->getRepositoryManager()->getLocalRepository();
+
+		self::patchComposerLockFile( $package_locker );


There may be an event earlier than this that we could hook into so that we only would have to update the package instance once and have Composer generate the correct composer.lock but I didn't spend much time seeing if that was possible.

… without error

github-actions · 2021-07-20T02:28:40Z

Plugin builds for 51b85ba are ready 🛎️!

Download development build
Download production build

westonruter

How can the issue that this PR fixes be reproduced? I tried running composer update on develop and it seemed to work.

php-css-parser-install-composer-plugin/src/Plugin.php

codecov · 2021-07-20T06:40:38Z

Codecov Report

Merging #6464 (d99bb2e) into develop (5244315) will increase coverage by 0.33%.
The diff coverage is n/a.

❗ Current head d99bb2e differs from pull request most recent head 9c49f6e. Consider uploading reports for the commit 9c49f6e to get more accurate results

@@              Coverage Diff              @@
##             develop    #6464      +/-   ##
=============================================
+ Coverage      75.12%   75.46%   +0.33%     
  Complexity      5908     5908              
=============================================
  Files            188      237      +49     
  Lines          17769    17887     +118     
=============================================
+ Hits           13349    13498     +149     
+ Misses          4420     4389      -31

Flag	Coverage Δ
javascript	`79.15% <ø> (?)`
php	`75.29% <ø> (+0.16%)`	⬆️
unit	`75.29% <ø> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
includes/sanitizers/class-amp-embed-sanitizer.php	`62.50% <0.00%> (-7.50%)`	⬇️
includes/embeds/class-amp-meetup-embed-handler.php	`22.22% <0.00%> (-5.06%)`	⬇️
src/BackgroundTask/BackgroundTaskDeactivator.php	`55.00% <0.00%> (-4.10%)`	⬇️
includes/embeds/class-amp-issuu-embed-handler.php	`11.76% <0.00%> (-4.03%)`	⬇️
includes/embeds/class-amp-reddit-embed-handler.php	`11.76% <0.00%> (-4.03%)`	⬇️
includes/admin/class-amp-admin-pointers.php	`13.63% <0.00%> (-3.04%)`	⬇️
src/AmpSlugCustomizationWatcher.php	`84.61% <0.00%> (-2.89%)`	⬇️
...c/BackgroundTask/SingleScheduledBackgroundTask.php	`83.33% <0.00%> (-2.39%)`	⬇️
...s/sanitizers/class-amp-accessibility-sanitizer.php	`80.00% <0.00%> (-2.36%)`	⬇️
...rc/Admin/ReenableCssTransientCachingAjaxAction.php	`12.00% <0.00%> (-2.29%)`	⬇️
... and 189 more

pierlon · 2021-07-20T22:29:03Z

How can the issue that this PR fixes be reproduced?

If you were to run composer update on the develop branch you'd notice the following set of changes:

diff --git a/composer.lock b/composer.lock
--- a/composer.lock	(revision c6d7c8ab38f51066f7585b20567bc8c2537baa3c)
+++ b/composer.lock	(date 1626819370118)
@@ -196,18 +196,22 @@
                 "reference": "bfdd976",
                 "shasum": ""
             },
-			"require": {
-			  "php": ">=5.3.2"
-			},
+            "require": {
+                "ext-iconv": "*",
+                "php": ">=5.6.20"
+            },
             "require-dev": {
                 "codacy/coverage": "^1.4",
-				"phpunit/phpunit": "~4.8"
-			},
+                "phpunit/phpunit": "^4.8.36"
+            },
+            "suggest": {
+                "ext-mbstring": "for parsing UTF-8 CSS"
+            },
             "default-branch": true,
             "type": "library",
             "autoload": {
                 "psr-4": {
-                    "Sabberworm\\CSS\\": "lib/Sabberworm/CSS/"
+                    "Sabberworm\\CSS\\": "src/"
                 }
             },

Amongst the changes, the one that we're most concerned with is the update to the psr-4 namespace, which is incorrect.

# Conflicts: # composer.lock

westonruter · 2021-07-20T23:48:07Z

I tried switching to this branch and I get an error when I run composer update:

$ composer update
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 0 updates, 0 removals
Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
PHP Fatal error:  Uncaught Error: Call to undefined method Composer\Package\CompletePackage::getAliasOf() in /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php:155
Stack trace:
#0 /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php(101): AmpProject\AmpWP\PhpCssParserInstall\Plugin::patchPackage(Object(Composer\Repository\InstalledFilesystemRepository))
#1 [internal function]: AmpProject\AmpWP\PhpCssParserInstall\Plugin::onPreAutoloadDump(Object(Composer\Script\Event))
#2 phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php(190): call_user_func(Array, Object(Composer\Script\Event))
#3 phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php(117): Composer\EventDispatcher\EventDispatcher->doDispatch(Object(Composer\Script\Event))
#4 phar:///usr/local/bin/composer/src/Composer/Autoload/AutoloadGenerator.php(170): Composer\EventDispatcher\EventDispatcher->dispatchScript('pre-autoload-du...', true, Array, Array)
#5 in /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php on line 155

Fatal error: Uncaught Error: Call to undefined method Composer\Package\CompletePackage::getAliasOf() in /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php:155
Stack trace:
#0 /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php(101): AmpProject\AmpWP\PhpCssParserInstall\Plugin::patchPackage(Object(Composer\Repository\InstalledFilesystemRepository))
#1 [internal function]: AmpProject\AmpWP\PhpCssParserInstall\Plugin::onPreAutoloadDump(Object(Composer\Script\Event))
#2 phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php(190): call_user_func(Array, Object(Composer\Script\Event))
#3 phar:///usr/local/bin/composer/src/Composer/EventDispatcher/EventDispatcher.php(117): Composer\EventDispatcher\EventDispatcher->doDispatch(Object(Composer\Script\Event))
#4 phar:///usr/local/bin/composer/src/Composer/Autoload/AutoloadGenerator.php(170): Composer\EventDispatcher\EventDispatcher->dispatchScript('pre-autoload-du...', true, Array, Array)
#5 in /app/public/content/plugins/amp/php-css-parser-install-composer-plugin/src/Plugin.php on line 155

pierlon · 2021-07-21T00:13:59Z

I tried switching to this branch and I get an error when I run composer update:

Hmm that's weird, I'm using composer v2.1.3 to test the plugin. I think b347c39 should solve the error you're getting.

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter · 2021-07-21T00:42:03Z

Hmm that's weird, I'm using composer v2.1.3 to test the plugin. I think b347c39 should solve the error you're getting.

Yes, I don't get any error now. And composer update completes without dirtying the composer.lock.

westonruter · 2021-07-21T00:42:46Z

I'm using Composer v2.1.3 also, by the way.

westonruter

This is way beyond my knowledge of Composer's inner-workings, but LGTM!

Add Composer plugin to correctly autoload PHP CSS Parser dependency

6d3270d

pierlon commented Jul 20, 2021

View reviewed changes

pierlon added 2 commits July 19, 2021 21:59

Normalize composer.json

31de5f0

Add docblock for method

c02d7d0

pierlon mentioned this pull request Jul 20, 2021

Update Composer dependencies #6446

Merged

2 tasks

pierlon marked this pull request as ready for review July 20, 2021 02:03

pierlon requested a review from westonruter July 20, 2021 02:03

pierlon self-assigned this Jul 20, 2021

pierlon added dependencies Pull requests that update a dependency file php Pull requests that update Php code labels Jul 20, 2021

pierlon added this to the v2.2 milestone Jul 20, 2021

Copy Composer plugin to build folder so dependencies can be installed…

af17d7e

… without error

Update variable name to provide better context

d99bb2e

westonruter reviewed Jul 20, 2021

View reviewed changes

php-css-parser-install-composer-plugin/src/Plugin.php Outdated Show resolved Hide resolved

pierlon added 3 commits July 20, 2021 18:32

Merge branch 'develop' into fix/php-css-parser-install

51c835f

# Conflicts: # composer.lock

Update composer.lock

9c49f6e

Update composer.json PHP 8 patch

3146a45

pierlon added 2 commits July 20, 2021 20:01

Revert composer.json changes

324d5b0

Get package alias if needed

b347c39

Fix formatting

51b85ba

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter approved these changes Jul 21, 2021

View reviewed changes

pierlon merged commit f4beb71 into develop Jul 21, 2021

pierlon deleted the fix/php-css-parser-install branch July 21, 2021 05:16

pierlon mentioned this pull request Jul 21, 2021

[2.1] Add Composer plugin to correctly autoload PHP CSS Parser dependency #6472

Merged

2 tasks

westonruter mentioned this pull request Aug 5, 2021

Update Gutenberg packages after v11.2.1 release #6511

Merged

westonruter modified the milestones: v2.2, v2.1.4 Aug 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Composer plugin to correctly autoload PHP CSS Parser dependency #6464

Add Composer plugin to correctly autoload PHP CSS Parser dependency #6464

pierlon commented Jul 20, 2021

pierlon Jul 20, 2021

github-actions bot commented Jul 20, 2021 •

edited

Loading

westonruter left a comment

codecov bot commented Jul 20, 2021 •

edited

Loading

pierlon commented Jul 20, 2021

westonruter commented Jul 20, 2021

pierlon commented Jul 21, 2021

westonruter commented Jul 21, 2021

westonruter commented Jul 21, 2021

westonruter left a comment

Add Composer plugin to correctly autoload PHP CSS Parser dependency #6464

Add Composer plugin to correctly autoload PHP CSS Parser dependency #6464

Conversation

pierlon commented Jul 20, 2021

Summary

Checklist

pierlon Jul 20, 2021

Choose a reason for hiding this comment

github-actions bot commented Jul 20, 2021 • edited Loading

westonruter left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 20, 2021 • edited Loading

Codecov Report

pierlon commented Jul 20, 2021

westonruter commented Jul 20, 2021

pierlon commented Jul 21, 2021

westonruter commented Jul 21, 2021

westonruter commented Jul 21, 2021

westonruter left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 20, 2021 •

edited

Loading

codecov bot commented Jul 20, 2021 •

edited

Loading