Closed (fixed)
Project:
Drupal core
Version:
9.0.x-dev
Component:
base system
Priority:
Major
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Oct 2019 at 08:32 UTC
Updated:
10 Dec 2019 at 16:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
mikelutzComment #3
mikelutzI added symfony/mime and symfony/var-dumper as explicit dependencies (They are already dependencies or dependencies of a dependency of http-kernel). If we don't add them explicitly, they are installed at v5.0.0. This might be fine, http-kernel allows them at ^4.4|^5.0, but because the 5.0 versions lose security support after 8 months, we would be constantly updating them to new minors in the middle of one of our minors, so I prefer to pin them to the LTS version. I also updated the composer.json files to ^4.4.0 from ^4.4 for no particular reason other than aesthetics. My OCD likes having the minimum patch version explicitly stated, even when it is 0.
Comment #4
alexpottFor me I think we should only include the patch version when it is significant. In an ideal world we'd be doing min / max testing and trying to be a compatible with PHP ecosystem as possible.
Listing indirect dependencies in our composer.json is also pretty interesting - I get the rationale - but also doesn't that mean we should be doing this for all our dependencies. Listing things we're not dependent on can backfire. What happens if Symfony remove a dependency (unlikely though that is).
Comment #6
andypostComment #7
mikelutzAlright, removing the patch from the composer.json.
Rather than explicitly requiring var-dumper and mime at ^4, we now conflict with >=5, so if symfony removes one, composer will be able to uninstall it.
Also https://github.com/symfony/symfony/pull/34366 added single quotes around RedirectResponse's meta refresh tag, so there's a regex update to the tests to take that into account.
Comment #8
alexpottYep but we don't do we... it feels as though composer should be able to say prefer-lts
Comment #9
mikelutzAs I see it, we can
Talk amongst yourselves, and tell me what to roll into the next patch. :-)
Comment #10
catchSo for this issue, I think either 1 or 3 is fine and don't have a strong preference. However we should open a beta blocking issue to decide which one we go with for release (or option 4 if it materialises).
Comment #11
mikelutzThe patch in #7 currently implements option #9.3
Comment #12
krzysztof domańskiI have attached this issue to CR Drupal 9 now requires Symfony 4.4.
Comment #13
alexpottI've not got another option atm. I think I dislike the conflicts more than explicitly requiring var-dumper and mime at ^4.4 - because we really don't conflict with them. My fear of option 1 would be that we add one of these as a dep during the 9 cycle and then we have to downgrade or do
^4|^5in our own composer.json. So perhaps I'm back to the approach taken in #3. As the least worst outcome whilst we ship a lock file and recommended versions.Comment #14
eric_a commentedHow about declaring conflicts with all of 5.0, 5.1, 5.2, 5.3? Symfony has planned 5.4 (and 6.4) to be long-term support, and last minor.
Comment #15
eric_a commentedAh, right.
Comment #16
mikelutzNew patch removing all conflicts and just allowing the 5.0 packages to be installed.
Comment #17
alexpottThis looks great. Thanks @mikelutz for bearing with us while we came to a decision. Or in this case a punt - I created #3096781: symfony/mime and symfony/var-dumper versions are on 5.1.7 (not an LTS) and therefore have gaps in security coverage relative to Drupal minors
Comment #18
alexpottCrediting people who contributed to the issue discussion and D9readiness meeting discussion.
Comment #19
alexpottCommitted this because it helps everyone having the composer stuff at the correct versions.
Committed 35bbac2 and pushed to 9.0.x. Thanks!
Comment #21
krzysztof domański