Problem/Motivation
Packages were held back because of the older version of PHP should now be upgradable since we use PHP 7.2.
Proposed resolution
Upgrade all the packages we can.
Remaining tasks
Test all the things.
User interface changes
None.
API changes
None (hopefully).
Data model changes
None.
Release notes snippet
+-------------------------------+---------+---------+
| Production Changes | From | To |
+-------------------------------+---------+---------+
| guzzlehttp/guzzle | 6.3.3 | 6.5.2 |
| symfony/class-loader | v3.4.35 | v3.4.36 |
| symfony/console | v4.4.0 | v4.4.2 |
| symfony/debug | v4.4.0 | v4.4.2 |
| symfony/dependency-injection | v4.4.0 | v4.4.2 |
| symfony/error-handler | v4.4.0 | v4.4.2 |
| symfony/event-dispatcher | v4.4.0 | v4.4.2 |
| symfony/http-foundation | v4.4.0 | v4.4.2 |
| symfony/http-kernel | v4.4.0 | v4.4.2 |
| symfony/mime | v5.0.0 | v5.0.2 |
| symfony/polyfill-ctype | v1.12.0 | v1.13.1 |
| symfony/polyfill-iconv | v1.12.0 | v1.13.1 |
| symfony/polyfill-intl-idn | v1.12.0 | v1.13.1 |
| symfony/polyfill-mbstring | v1.12.0 | v1.13.1 |
| symfony/polyfill-php72 | v1.12.0 | v1.13.1 |
| symfony/polyfill-php73 | v1.12.0 | v1.13.1 |
| symfony/process | v4.4.0 | v4.4.2 |
| symfony/routing | v4.4.0 | v4.4.2 |
| symfony/serializer | v4.4.0 | v4.4.2 |
| symfony/service-contracts | v2.0.0 | v2.0.1 |
| symfony/translation | v4.4.0 | v4.4.2 |
| symfony/translation-contracts | v2.0.0 | v2.0.1 |
| symfony/validator | v4.4.0 | v4.4.2 |
| symfony/var-dumper | v5.0.0 | v5.0.2 |
| symfony/yaml | v4.4.0 | v4.4.2 |
+-------------------------------+---------+---------+
+------------------------+--------+--------+
| Dev Changes | From | To |
+------------------------+--------+--------+
| drupal/coder | 8.3.6 | 8.3.7 |
| symfony/browser-kit | v4.4.0 | v4.4.2 |
| symfony/css-selector | v4.4.0 | v4.4.2 |
| symfony/dom-crawler | v4.4.0 | v4.4.2 |
| symfony/filesystem | v4.4.0 | v4.4.2 |
| symfony/finder | v4.4.0 | v4.4.2 |
| symfony/lock | v4.4.0 | v4.4.2 |
| symfony/phpunit-bridge | v4.4.0 | v4.4.2 |
+------------------------+--------+--------+| Comment | File | Size | Author |
|---|---|---|---|
| #42 | 3091418-42.patch | 43.18 KB | traviscarden |
| #31 | 3091418-31.patch | 71.5 KB | tedbow |
| #25 | 3091418-25.patch | 49.15 KB | manuel garcia |
| #25 | interdiff-3091418-22-25.txt | 954 bytes | manuel garcia |
| #22 | 3091418-22.patch | 48.43 KB | jibran |
Comments
Comment #2
manuel garcia commentedSteps taken to generate this patch:
1. checked out the
9.0.xbranch (last commit125a331dc300abb5d383ed2b3dfe1e6c08c14e9d)2. removed the
vendordirectory3. Ensured that
composer.lockis checked out (not modified).4. Run
composer updateComment #3
manuel garcia commentedOn #3079791: Bump Drupal's minimum PHP version to 7.2 as soon as 9.0.x is branched (a higher version may be required later) comment #26 @hussainweb commented:
This patch was uploaded there however it no longer applies on 9.0.x:
Uploading it here for transparency.
Comment #4
manuel garcia commentedComment #5
manuel garcia commentedLet's try this again.
Comment #7
manuel garcia commentedHere is the composer lock diff for #5:
Comment #8
manuel garcia commentedI went back in time to 3ed75506489dc2cd8bfd62d6a2ce01ff9ce81cf3 and applied the patch on #3 to have a look at the composer lock diff, here it is:
Comment #9
manuel garcia commentedLooks like the failures are due to deprecation notices:
"Symfony\Component\DomCrawler\Crawler::text()" will normalize whitespaces by default in Symfony 5.0, set the second "$normalizeWhitespace" argument to false to retrieve the non-normalized version of the text.Seems to be triggered by calls to
WebAssert::pageTextContains()Comment #10
manuel garcia commentedI've noticed that the behat packages we require-dev are archived repositories:
These repos is actually forks of
Which do seem to be alive still.
In any case this is probably something we should address as this is what we've got on our
composer.jsonat this moment:Comment #11
manuel garcia commentedOK ends up that those are package alias for minkphp packages so we're ok on that side.
That said, we've apparently been chasing a new release for years: https://github.com/minkphp/Mink/issues/787
See #3078671: Pin behat/mink and behat/mink-selenium2-driver to use resolvable release
Comment #12
xjmComment #14
xjmAlso adding credit for Hussain.
Comment #15
xjmAnd crossposting with myself apparently.
Comment #16
jibranThis is not that stright forward. I think we need to take the following steps:
core/composer.jsonto be PHP 7.2 compatible.composer.jsonandcomposer.lockchanges.Comment #17
gábor hojtsy@jibran: re #16: I think for #3080837: [meta] PHP (and JS) dependencies to consider decoupling or removing we got as far as we could in Drupal 8.8? #3009213: [META] Update / reconsider PHP dependencies for Drupal 9 links back to here as one of its steps, so I don't think we should wait around in a circle :)
I don't think there is harm in updating a dependency that may turn out to be removable later. Unless there is a lot of work to make it work with Drupal 9, but until we try we'll not know. So let's try?
Comment #18
jibranThat would be fine as long as we remove them before beta because after beta it would be BC break.
Yeah, let's not block this issue then. We can also skip the new deprecation errors to make the patch green and unblock the alpha. We'll keep removing them till beta.
Do we have to make sure there is no BC break during these updates? I'd say yes.
Comment #19
gábor hojtsy@jibran: IMHO now would be the time to break BC, this is about Drupal 9. When to break BC if not now? Also for skipped deprecations, I think we should cross the bridge (ie. discuss) when we get there :)
Comment #20
gábor hojtsyDiscussed mechanics of how to roll this patch. @jibran pointed out he has steps outlined at https://gist.github.com/jibran/dd8ac842a239d6a2dc7af80bd69eb19f from earlier. For Drupal 9: php 5.5.9 should be changed to 7.2.x. And each composer command should be prefixed by
COMPOSER_ROOT_VERSION=9.0.x-devapparently.Comment #21
jibranOn PHP 7.2.3 we have
We can't update
behat/mink-selenium2-driveras stated in previous comments. We can ignoredrupal/core-*packages. We need a dedicated issue forphpunit/phpunit8andsymfony/phpunit-bridge5, if we don't already have and it would be non-alpha blocker as it is a dev dependency and can be updated after alpha.We can update
drupal/coder symfony/browser-kit symfony/css-selector symfony/debug symfony/dom-crawler symfony/filesystem symfony/finder symfony/lockthough.Comment #22
jibranI had to revert
symfony/mime,symfony/var-dumperto biring in PHP 7.2.3 compataible version. Here is the change details:Comment #23
jibranFeel free to work on this.
Comment #25
manuel garcia commentedSo nice to see progress on this, thanks!
I had a look at the failing test, I believe it's due to this change in Guzzle https://github.com/guzzle/guzzle/pull/2081
So all we have to do is update our test to assert for the updated message.
Comment #26
effulgentsia commentedDoes the Guzzle update from 6.3 to 6.5 require PHP 7.2? If not, let's update it in a separate issue that we can cherry pick to 8.9. Same for any other library that doesn't require the PHP version bump.
Comment #27
jibranI agree with #26 but it is just one testing change so let's update it here. Let's open new issue for 8.9.x.
Comment #28
catchAgreed with #27, there's no way to cherry-pick commits to composer.json/composer.lock between 9.x and 8.x, so we're always going to have to do a manually backported patch. They definitely need their own separate 8.9.x issues though.
Comment #29
gábor hojtsyGiven we only had that single test fix required and no other changes needed, what is left here to do?
Comment #30
longwaveGuzzle 6.5.0 is causing issues for some PHP installs in #3100785: Use of undefined constant INTL_IDNA_VARIANT_UTS46 in Guzzle 6.5.0 so I wonder if we should wait for Guzzle 6.5.1, which is hopefully soon as per https://github.com/guzzle/guzzle/issues/2448
Comment #31
tedbowThis is reroll but also added the new guzzle release 6.5.2
Generally looks good. Lets see about tests
Comment #32
tedbowHere is how I made #31
Then I applied the update to change to
\Drupal\Tests\migrate\Functional\process\DownloadFunctionalTest::testExceptionThrow()from #25 after confirming the test will fail without it.Comment #33
greg.1.anderson commentedThis all looks good to me, and I'm pretty much ready to RTBC it. I just have one question, though. I noticed this:
Sure enough, drupal/core's composer.json still requires ~3.4.0 of symfony/class-loader. Is there any reason why this Symfony component alone is still being pinned to 3.x, or could we bump this dependency up to 4.x like the others?
Comment #34
xjmI'd also like to find out about #33. I think we can file a followup for it since there aren't any changes to
core/composer.jsonin this patch yet.We probably also want a followup for 8.9.x, though that's really just something we address every minor and not directly needed here.
Comment #35
mile23It looks like symfony/class-loader doesn't have a release after 3.4. In fact:
We should probably remove it from drupal/core if symfony 4 is our baseline.
Comment #36
greg.1.anderson commentedCreated #3103008: Update or remove symfony/class-loader version on 9.0.x as a follow-up for symfony/class-loader, although that issue still needs to be fleshed out a bit.
RTBC'ed.
Comment #37
greg.1.anderson commented#35: Ah yes, that component is in fact deprecated. Changed the follow-up issue to "update or remove" it.
Comment #38
berdirSee #3020296: Remove Symfony's classloader as it does not exist in Symfony 4, that and related issues have been around for a while.
Comment #39
mile23Comment #40
catchThe Doctrine website is showing several of the versions updated to here as unsupported, with newer versions in development (but not released yet) and only older versions still supported (some still more recent than the versions we're on).
Went through a few.
I have no idea why they have essentially 'dead end' versions with releases from two months ago, it could possibly be their website not updated fully yet, but I think we need to investigate a bit more.
I don't think we can go to doctrine/annotations 1.8
1.8 is unsupported, 2.0 is upcoming, 1.7 is supported, according to
https://www.doctrine-project.org/projects/annotations.html
Similarly doctrine/cache 1.10 is unmaintained, only 1.8 is
https://www.doctrine-project.org/projects/cache.html
This is OK.
https://www.doctrine-project.org/projects/collections.html
This is also OK.
https://www.doctrine-project.org/projects/common.html
And this is OK too.
https://www.doctrine-project.org/projects/inflector.html
Can't update this one for the same reason
https://www.doctrine-project.org/projects/lexer.html
This one is OK. https://www.doctrine-project.org/projects/persistence.html
Comment #41
catchOK I might have found part of the problem here, see this announcement:
https://www.doctrine-project.org/2018/07/12/common-2-9-and-dbal-2-8-and-...
So doctrine/lexer is explained by this:edit: no it's not I misread.
We may have to spin these ones off into dedicated issues.
Comment #42
traviscarden commentedCreated #3104265: Update Composer dependencies on Doctrine components in 9.0.x. Here's a patch without the
doctrine/*updates. Changes:p.s. - @jibran, thanks for turning me on to davidrjonas/composer-lock-diff. What a great tool!
Comment #43
xjmThanks @TravisCarden!
I've been going back and forth about what should be updated in
composer.jsonand when. Given the fact that the Guzzle update is a bugfix and we're changing a test for it, I'm wondering if we should be updating thecore/composer.jsonrequirements for Guzzle to^6.5.2? Thoughts?Comment #44
xjmComment #45
greg.1.anderson commentedI am always of a mind that, for any given project, if you run
composer update --prefer-lowest(this is the "min" part of "min/max testing), then you should get a release that is usable. In order for this to work, each dependency in composer.json must list the minimum version of that component that will work.It is an aspirational goal to someday do some form of min/max testing on Drupal. In support of that, I would be +1 on pinning the guzzle version as suggested in #43.
Comment #46
xjmThanks Greg! Based on that, NW to bump the requirement for Guzzle.
Comment #47
greg.1.anderson commentedWhen we are bumping the Composer dependencies for a major Drupal version release, perhaps we should also make a policy that we increase all of the version constraints for all components listed explicitly in core/composer.json to whatever minimum version they happen to resolve to in composer.lock at that time. Folks will already be adjusting their code to work with the new version of Drupal; it seems like the need for supporting anything older than what is in composer.lock would therefore be minimal. While this is not required, doing this reduces the number of versions of dependencies supported by Drupal, and reduces the number of valid versions of dependencies that are potential solutions from the Composer resolver. (This does not help the current default algorithm, which tries all permutations, but we already have some Composer plugins which optimize this, and such a reduction might help future versions of these do even better.)
Overall, I care 2/10, but it seems like a good idea.
Comment #48
mile23New child, based on #46 and #24/25 to preserve scope here: #3104473: Use version of guzzlehttp/guzzle ^6.5.2
Added #3104474: D9, PHP 7.2 vendor min Testing issue - DO NOT COMMIT to do min testing on D9.
Patch in #42 still applies, and tests are green so LGTM.
Comment #49
catch#47 is a good idea, opened #3091418: Update composer dependencies on 9.0.x following PHP 7.2 requirement.
Comment #51
catchCommitted/pushed to 9.0.x, thanks!
Comment #52
wim leers#49 that is this issue 😅
Comment #53
longwave#49 should refer to #3104596: [policy, no patch] set miminum composer.json version constraints based on first minor release !
Comment #54
xjm🎉
Comment #56
gábor hojtsyI cloned this into #3109261: Update composer dependencies on 9.0.x following PHP 7.3 requirement following #3106075: Bump minimum PHP for Drupal 9 to PHP 7.3. No idea what that will entail, it would be great if someone could try :) Retitling this for specificity.
Comment #57
xjm