Make Sylius compatible with PHP 7.2 #8771

stefandoorn · 2017-10-04T17:51:01Z

Q	A
Branch?	1.0
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
License	MIT

Run test suite already against PHP 7.2 to pro-actively make adjustments before PHP 7.2 is released (if possible). Not sure if this should be against 1.0 or master though ;-)

stefandoorn · 2017-10-04T17:56:39Z

First issue spotted: friendsofphp/php-cs-fixer needs to be at 2.6.1 at least to support PHP 7.2.

stefandoorn · 2017-10-04T20:29:40Z

I'm not sure how to figure out what the Behat errors are. Output seems scarce.

gabiudrescu · 2017-10-06T16:31:00Z

@stefandoorn if you check this etc/travis/run-suite after_failure "${SYLIUS_SUITE}" the build log, you can reach this:

http://termbin.com/uw75

which is pretty verbose.

stefandoorn · 2017-10-07T11:36:59Z

Ok..

idn_to_ascii(): INTL_IDNA_VARIANT_2003 is deprecated in /home/travis/build/Sylius/Sylius/vendor/league/uri/src/Components/HostnameTrait.php line 68

league/uri is required by Payum at ~4.0. Not sure about it, but issue might be fixed in league/uri 5.x. Doesn't seem Payum already supports this, so this might be a hard path to get fixed.

stefandoorn · 2017-10-07T12:52:47Z

Anyone also knows the URL on which the imgur uploaded screenshots can be found?

pamil · 2017-10-09T12:35:19Z

@stefandoorn looks like our imgur API key is not valid so uploading screenshots has stopped working.

pamil · 2017-10-10T09:14:33Z

.travis.yml

+                - memcached
+        -
+            sudo: required
+            php: nightly


I'd skip the nightly PHP version for now as there's still some time until PHP 7.3 gets released & builds will take more time to finish.

Instead, running packages build on PHP 7.2 is a must :)

Mainly added nightly for now as it might discover some issues now already that we can tackle directly. Will take it out before finalising PR.

pamil · 2017-10-10T09:17:13Z

composer.json

@@ -88,7 +88,7 @@
        "matthiasnoback/symfony-dependency-injection-test": "^1.0",
        "mikey179/vfsStream": "^1.6",
        "pamil/prophecy-common": "^0.1",
-        "phpspec/phpspec": "^4.0",
+        "phpspec/phpspec": "^4.0@dev",


Same change applies for every Sylius package using phpspec (packages build on PHP 7.2 would fail without this one).

Alright. Need to see if this change is required at all before finalising PR.

pamil · 2017-10-10T09:22:24Z

Not sure if this should be against 1.0 or master though ;-)

IMO it should go for 1.0 branch as 1.0 requires PHP ^7.1, so PHP 7.2 should also be supported there. (rebase needed though then)

pamil · 2017-10-10T09:54:46Z

As for that Payum-related errors:

Payum itself requires PHP 5.5+
Payum requires thephpleague/uri v4:
- thephpleague/uri v4 requires PHP 5.5+
- thephpleague/uri v4 uses the deprecated function call
- thephpleague/uri v4 requires jeremykendall/php-domain-parser v3:
  - jeremykendall/php-domain-parser v3 uses the deprecated function call
If Payum requires thephpleague/uri v5:
- thephpleague/uri v5 requires PHP 7.0+
- thephpleague/uri v5 requires jeremykendall/php-domain-parser v4.0.3-alpha:
  - jeremykendall/php-domain-parser v4.0.3-alpha does not use the deprecated function call

Conclusion:

Getting Payum to use the newer thephpleague/uri would require to drop PHP 5.5 / 5.6 support for them (and probably a lot of work)
We can try to send bugfixes to thephpleague/uri v4 and jeremykendall/php-domain-parser v3 to use that not deprecated variant in idn_to_* functions (like done here - https://github.com/jeremykendall/php-domain-parser/blob/4.0.3-alpha/src/Pdp/Parser.php#L318-L322)

pamil · 2017-10-10T10:17:49Z

jeremykendall/php-domain-parser#204 and thephpleague/uri-src#41 have been opened, let's wait for the replies.

stefandoorn · 2017-10-17T11:22:04Z

@pamil Ready to review. Had to explicitly require the updated dependencies to get the proper versions installed.

Build on PHP 7.3 fails due to PHP-CS-Fixer not allowing 7.3. I'm not sure we should disallow installation on PHP 7.3 due to this.

pamil

Awesome! Can you add packages build for PHP 7.2? It's like the one for PHP 7.1 but with docs removed from $SYLIUS_SUITE 🎉

pamil · 2017-10-17T13:39:39Z

Thanks Stefan, great job! 🥇

stefandoorn changed the title ~~Test suite against PHP 7.2~~ [WIP] Test suite against PHP 7.2 Oct 4, 2017

stefandoorn mentioned this pull request Oct 7, 2017

idn_to_ascii(): INTL_IDNA_VARIANT_2003 is deprecated in PHP 7.2 Payum/Payum#693

Closed

stefandoorn mentioned this pull request Oct 9, 2017

Update methods to match symfony generated class #8758

Closed

pamil suggested changes Oct 10, 2017

View reviewed changes

pamil changed the base branch from master to 1.0 October 10, 2017 09:22

This was referenced Oct 10, 2017

Support for PHP 7.2 in 3.x branch jeremykendall/php-domain-parser#204

Closed

Support for PHP 7.2 in 4.x branch thephpleague/uri-src#41

Closed

stefandoorn force-pushed the travis-php7.2 branch from fe8ad37 to feea952 Compare October 17, 2017 10:20

pamil added this to the 1.0 milestone Oct 17, 2017

pamil added Maintenance CI configurations, READMEs, releases, etc. PHP 7.2 labels Oct 17, 2017

stefandoorn force-pushed the travis-php7.2 branch from 3e62bb6 to 0323fdf Compare October 17, 2017 11:18

stefandoorn changed the title ~~[WIP] Test suite against PHP 7.2~~ Make Sylius compatible with PHP 7.2 Oct 17, 2017

Make Sylius compatible with PHP 7.2

b904900

stefandoorn force-pushed the travis-php7.2 branch from 0323fdf to b904900 Compare October 17, 2017 11:21

pamil reviewed Oct 17, 2017

View reviewed changes

Run packages build also against PHP 7.2

7253d82

pamil approved these changes Oct 17, 2017

View reviewed changes

pamil merged commit 153e380 into Sylius:1.0 Oct 17, 2017

This was referenced Oct 17, 2017

[Travis] Run application & packages suites in one job #8866

Closed

[Travis] Allow failures for PHP 7.2 builds #8868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Sylius compatible with PHP 7.2 #8771

Make Sylius compatible with PHP 7.2 #8771

stefandoorn commented Oct 4, 2017 •

edited

Loading

stefandoorn commented Oct 4, 2017

stefandoorn commented Oct 4, 2017

gabiudrescu commented Oct 6, 2017

stefandoorn commented Oct 7, 2017

stefandoorn commented Oct 7, 2017

pamil commented Oct 9, 2017

pamil Oct 10, 2017

pamil Oct 10, 2017

stefandoorn Oct 10, 2017

pamil Oct 10, 2017

stefandoorn Oct 10, 2017

pamil commented Oct 10, 2017 •

edited

Loading

pamil commented Oct 10, 2017 •

edited

Loading

pamil commented Oct 10, 2017

stefandoorn commented Oct 17, 2017

pamil left a comment

pamil commented Oct 17, 2017

Make Sylius compatible with PHP 7.2 #8771

Make Sylius compatible with PHP 7.2 #8771

Conversation

stefandoorn commented Oct 4, 2017 • edited Loading

stefandoorn commented Oct 4, 2017

stefandoorn commented Oct 4, 2017

gabiudrescu commented Oct 6, 2017

stefandoorn commented Oct 7, 2017

stefandoorn commented Oct 7, 2017

pamil commented Oct 9, 2017

pamil Oct 10, 2017

Choose a reason for hiding this comment

pamil Oct 10, 2017

Choose a reason for hiding this comment

stefandoorn Oct 10, 2017

Choose a reason for hiding this comment

pamil Oct 10, 2017

Choose a reason for hiding this comment

stefandoorn Oct 10, 2017

Choose a reason for hiding this comment

pamil commented Oct 10, 2017 • edited Loading

pamil commented Oct 10, 2017 • edited Loading

pamil commented Oct 10, 2017

stefandoorn commented Oct 17, 2017

pamil left a comment

Choose a reason for hiding this comment

pamil commented Oct 17, 2017

stefandoorn commented Oct 4, 2017 •

edited

Loading

pamil commented Oct 10, 2017 •

edited

Loading

pamil commented Oct 10, 2017 •

edited

Loading