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.

Issue fork drupal-3365567

Command icon 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:

Comments

Spokje created an issue. See original summary.

spokje’s picture

Issue summary: View changes
catch’s picture

Is this an issue when using drupal/core-recommended?

chi’s picture

Re #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.

spokje’s picture

Ah....apparantly not...

symfony/console: ~v6.2.5
symfony/dependency-injection: ~v6.2.6
symfony/deprecation-contracts: ~v3.2.0
symfony/error-handler: ~v6.2.5
symfony/event-dispatcher: ~v6.2.5
symfony/event-dispatcher-contracts: ~v3.2.0
symfony/http-foundation: ~v6.2.6
symfony/http-kernel: ~v6.2.6
symfony/mime: ~v6.2.5
symfony/polyfill-ctype: ~v1.27.0
symfony/polyfill-iconv: ~v1.27.0
symfony/polyfill-intl-grapheme: ~v1.27.0
symfony/polyfill-intl-idn: ~v1.27.0
symfony/polyfill-intl-normalizer: ~v1.27.0
symfony/polyfill-mbstring: ~v1.27.0
symfony/process: ~v6.2.5
symfony/psr-http-message-bridge: ~v2.1.4
symfony/routing: ~v6.2.5
symfony/serializer: ~v6.2.5
symfony/service-contracts: ~v3.2.0
symfony/string: ~v6.2.5
symfony/translation-contracts: ~v3.2.0
symfony/validator: ~v6.2.5
symfony/var-dumper: ~v6.2.5
symfony/var-exporter: ~v6.2.5
symfony/yaml: ~v6.2.5

https://packagist.org/packages/drupal/core-recommended#10.0.x-dev

spokje’s picture

So 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?

cilefen’s picture

@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.

spokje’s picture

Status: Active » Needs review

You are arguing that core should set known working dependency ranges regardless of the specific pinned versions in core-recommended.

I 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).

chi’s picture

core-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/core should always clearly declare supported dependencies.

andypost’s picture

The 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

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The 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.

nod_’s picture

Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

bot is checking against 10.1.x so excluding this one from the list of issues

jonathan1055’s picture

I 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-recommended into the set-up steps?

cilefen’s picture

The constraints should be 6.2.*, so that of course, any site can install a lower version of SF 6.2 components if desired.

cilefen’s picture

Status: Needs review » Needs work

@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.

Darren Oh made their first commit to this issue’s fork.

darren oh’s picture

Status: Needs work » Needs review
cilefen’s picture

Status: Needs review » Needs work

~6.2 is an incorrect constraint. It allows 6.3 to install.

darren oh’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Version appears locked.

longwave’s picture

Do we need to lock all of Symfony on 6.2, or just the problematic components?

longwave’s picture

Another 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?

darren oh’s picture

The 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.

longwave’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new3.53 KB

If we lock symfony/validator to ~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...

longwave’s picture

The policy is that only Symfony 6.2 is supported on Drupal 10.0

Where 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.

longwave’s picture

StatusFileSize
new8.47 KB

Let's also lock symfony/serializer, and backport some return type fix comments from 10.1.x.

longwave’s picture

StatusFileSize
new15.57 KB

A 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

darren oh’s picture

The 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.

catch’s picture

@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.

longwave’s picture

StatusFileSize
new19.53 KB

More void return declarations.

longwave’s picture

StatusFileSize
new19.6 KB
longwave’s picture

Title: Lock Drupal 10.0 on Symfony 6.2 » Lock Drupal 10.0 on symfony/serializer and symfony/validator 6.2

Updating 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.

darren oh’s picture

longwave’s picture

Another 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.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

#37 makes complete sense.

  • catch committed 70853dd7 on 10.0.x
    Issue #3365567 by longwave, Darren Oh, Spokje, cilefen, Chi: Lock Drupal...
catch’s picture

Status: Reviewed & tested by the community » Fixed

OK 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.