Problem/Motivation
As discussed in #3363222: Update to Symfony 6.3:
The current requirement in 10.0.x for example allows symfony/console 6.3 to install. Which then results in this issue:
https://www.drupal.org/project/drupal/issues/3364801
Shouldn't we backport or lock the version to 6.2 then?
Re Backport:
no that will stay on Symfony 6.2, and then 10.0.x goes out of support the end of this year.
So locking it is, let's do that in this issue.
| Comment | File | Size | Author |
|---|---|---|---|
| #33 | 3365567-33.patch | 19.6 KB | longwave |
Issue fork drupal-3365567
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3365567-lock-drupal-10.0
changes, plain diff MR !4132
Comments
Comment #2
spokjeComment #3
chi commentedComment #4
catchIs this an issue when using drupal/core-recommended?
Comment #6
chi commentedRe #4 I don't think so. However, not all people use
drupal/core-recommended. Personally, I think it is wrong pattern to fix dependency problems with this package.Comment #7
spokjeAh....apparantly not...
https://packagist.org/packages/drupal/core-recommended#10.0.x-dev
Comment #8
spokjeSo I think we can close this issue as a won't fix?
Or is it a problem that drupal/core without using drupal/core-recommended can still be updated to SF 6.3?
Comment #9
cilefen commented@Chi You are arguing that core should set known working dependency ranges regardless of the specific pinned versions in core-recommended. Am I correct?
It makes sense to me.
Comment #10
spokjeI would argue that setting known working dependency ranges is the essence of core-recommended and that if you want to "go rogue" on drupal/core you can, but at your own risk.
But let's let a core committer chime in on that, putting this on NR for that reason (and the fact that TestBot turned green).
Comment #11
chi commentedcore-recommended is optional package. It pins dependencies to those that are used for testing Drupal core on drupal.org CI.
So that it may potentially help to avoid issues when third-party packages accidentally break BC promises in non-major release. If I am not mistaken, it was originally created by webflow and then moved to core repository as part of Composer initiative.
Anyway, given it is just an option,
drupal/coreshould always clearly declare supported dependencies.Comment #12
andypostThe only related left open to detect regressions but other issues are closed like #2976407: Use drupalci.yml for composer dependency min/max testing - DO NOT COMMIT
Comment #13
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
nod_bot is checking against 10.1.x so excluding this one from the list of issues
Comment #15
jonathan1055 commentedI am using D10.0-dev and have just hit this problem. From #10 I have no intention to "go rogue", I was just following the normal process of setting up a site using the steps that have been in place for several years. From https://packagist.org/packages/drupal/core-recommended#10.0.x-dev are you saying I need to add
composer require drupal/core-recommendedinto the set-up steps?Comment #16
cilefen commentedThe constraints should be 6.2.*, so that of course, any site can install a lower version of SF 6.2 components if desired.
Comment #17
cilefen commented@jonathan1055 The official install instructions lead to having core-recommended by dint of using
composer create-project drupal/recommended-project my_site_name_dir. As far as I know those have been the instructions for several years.I am making this NW based on #16.
Comment #19
darren ohComment #20
cilefen commented~6.2 is an incorrect constraint. It allows 6.3 to install.
Comment #21
darren ohComment #22
smustgrave commentedVersion appears locked.
Comment #23
longwaveDo we need to lock all of Symfony on 6.2, or just the problematic components?
Comment #24
longwaveAnother thought: the "updated deps" CI run should catch this, no? https://www.drupal.org/pift-ci-job/2692265 implies that PHPStan has picked up on this - if we fix the "updated deps" run with the minimum amount of fixes and constraints, is that better than locking Symfony entirely to 6.2?
Comment #25
darren ohThe policy is that only Symfony 6.2 is supported on Drupal 10.0, so it makes sense to lock all of Symfony to that version.
Comment #26
longwaveIf we lock
symfony/validatorto ~6.2.0 and fix the PHPStan warnings, we should be able to get Drupal 10.0 running on Symfony 6.3 except for the validator component - let's see...Comment #27
longwaveWhere is this policy?
Symfony 6.3 should be backward compatible with 6.2, *except* in the case where we used internal Symfony APIs, which we have done in the typed data validator, and hence the issues that we are seeing here.
Comment #28
longwaveLet's also lock
symfony/serializer, and backport some return type fix comments from 10.1.x.Comment #29
longwaveA lesson for the future is that we should backport as many fixes as possible when we update to a new Symfony version, a lot of this is taken from #3338328: Update to Symfony 6.3
Comment #30
darren ohThe policy was stated by Nathaniel Catchpole and quoted in the description of this issue. This issue was opened because the core committers refused to accept a backport.
Comment #31
catch@Darren Oh we won't backport the actual update to Symfony 6.3, this doesn't mean we can't backport compatibility fixes if they're otherwise fine for a patch release - these are two different things.
Comment #32
longwaveMore void return declarations.
Comment #33
longwaveComment #34
longwaveUpdating the title for the suggested new approach.
The updated deps test in #33 proves that Drupal 10.0 runs on Symfony 6.3, except these two components, if we update phpstan-drupal and add some comments to tell Symfony our future plans.
Comment #36
darren ohComment #37
longwaveAnother reason for doing it this way: Symfony 6.4 will be released in November, when Drupal 10.0 goes out of support. There's a good chance that Symfony 6.4 will bring along some similar issues; users who update their components to Symfony 6.4 will run into these issues, and really we want them to update to Drupal 10.1 or 10.2 - whereas if we locked on Symfony 6.2, users could stay on Drupal 10.0 for longer.
Comment #38
smustgrave commented#37 makes complete sense.
Comment #40
catchOK I think this is the best we can do - pin the ones where we're subclassing internal stuff (not by our own choice, we tried to get upstream changes so we wouldn't have to do that and they were rejected), and suppress phpcs for the rest.
Committed 70853dd and pushed to 10.0.x. Thanks!