Skip to content

Conversation

@pierlon
Copy link
Contributor

@pierlon pierlon commented Jul 6, 2021

Summary

Updates:

  • cweagans/composer-patches to ~1.0 (>=1.0.0 <2.0.0) (as requested by @swissspidy in Bump cweagans/composer-patches from 1.7.0 to 1.7.1 #6363 (comment))

  • roave/security-advisories to dev-latest instead of dev-master as recommended by the maintainer

  • Emulated PHP version (config.platform.php) to v5.6.20 to allow sabberworm/php-css-parser to be installed, otherwise when attempting install it would produce the error:

      Problem 1
    - Root composer.json requires sabberworm/php-css-parser dev-master#bfdd976 -> satisfiable by sabberworm/php-css-parser[dev-master].
    - sabberworm/php-css-parser dev-master requires php >=5.6.20 -> your php version (5.6; overridden via config.platform, actual: 7.2.34) does not satisfy that requirement.
    

    Update: Turns out this is because the composer config of the master branch is being locked and not the config in the bfdd976 commit. I think it might be a composer bug so I'll file a report.

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 pierlon added dependencies Pull requests that update a dependency file php Pull requests that update Php code labels Jul 6, 2021
@pierlon pierlon added this to the v2.2 milestone Jul 6, 2021
@pierlon pierlon requested review from schlessera and swissspidy July 6, 2021 19:32
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2021

Plugin builds for 51036ad are ready 🛎️!

@pierlon
Copy link
Contributor Author

pierlon commented Jul 6, 2021

Checking what's causing the PHP tests to fail...

"cweagans/composer-patches": "1.7.1",
"cweagans/composer-patches": "~1.0",
"fasterimage/fasterimage": "1.5.0",
"sabberworm/php-css-parser": "dev-master#bfdd976"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like composer is locking the requirements of dev-master first, then checking out to bfdd976, causing a psr-4 namespace mismatch ("Sabberworm\\CSS\\": "src/" instead of "Sabberworm\\CSS\\": "lib/Sabberworm/CSS/") and thus not being able to autoload the library correctly.

@pierlon pierlon force-pushed the update/composer-deps branch from 87f5121 to 1c90850 Compare July 6, 2021 21:10
@pierlon
Copy link
Contributor Author

pierlon commented Jul 7, 2021

Tiktok embed test was failing earlier but all seems good now 😄.

Comment on lines +199 to +205
"require": {
"php": ">=5.3.2"
},
"require-dev": {
"codacy/coverage": "^1.4",
"phpunit/phpunit": "^4.8.36"
},
"phpunit/phpunit": "~4.8"
},
Copy link
Contributor Author

@pierlon pierlon Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've manually reverted the psr-4 path to lib/Sabberworm/CSS/ in 1c90850. This seems like a Composer bug so I'll report it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the behavior is as intended, as explained in composer/composer#9990 (comment). I'm not sure how to resolve this then since installing the dependency from the master branch is essentially broken now. Maybe we could fork it in the interim until a new release is made?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fork it in order to create a release tag to reference?

I hope that a new release of PHP-CSS-Parser will be made soon with all of the improvements being made recently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a Composer plugin as a method of patching the composer.lock file as well, but I'm not sure if we need to employ that as yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎 You're a composer whisperer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a PR to add it: #6464.

@westonruter westonruter merged commit c6d7c8a into develop Jul 19, 2021
@westonruter westonruter deleted the update/composer-deps branch July 19, 2021 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants