phpstan dev constraints make it difficult for modules to support 11.2 and below

📌 Update PHPStan to 2.1.17 Active is where the 1.x version was removed by accident

Major, I think. Maybe not quite critical?

This is where if we had a tooling topic maintainer I would ask them for signoff. In lieu of that, I guess framework managers?

Do we need to validate the 1.x constraint with an affected contrib module somehow, since core won't run pipelines with it?

I don't think there's a way to test this without committing it, but also nothing can go very wrong.

Not sure how much this is going to help though if phpstan 1 and 2 fail on each other's config but perhaps I'm misunderstanding.

Dumb questions... why do we have to put phpstan/phpstan as a dev dependency in composer.json, and therefore lock it for install, at all?

Can't we just omit that and let CI jobs do their own composer require when they need the tool, for whatever version they care for? And no metapackage locks either. Also, quicker composer installs and less data - only get package when needed.

Then maybe OK, we may want to specify conflict versions in composer.json to avoid running with obsoleted baseline, but that makes a lot of difference than always forcing a download.

My 0.02$.

MR!12525 has #7 in practice.

It would mean you can't composer install core and run phpstan on it, you'd always have to composer require it first, I don't see why we can't specify our own dev dependencies as actual dependencies really.

✨ Create metapackage of (loose) contrib test dependencies Active touches on the comments from #7 and #10 in that we ultimately don't have a good package for "developing contrib" vs "developing core" to pull dependencies. For contrib the phpstan constraint really is not needed.

Long term I do suggest Core consider if they even need a package for "developing core" package. The core composer.json should be able to load everything a developer needs to test core meaning there really is only a need for listing what is needed by contrib to use Core as a test platform.

To me: Visually MR!12510 change looks fine.

Biggest risk of doing this is some downstream dependency lists PHPStan (or related modules) as a regular dependency instead of a dev dependency forcing back to ^1, or some other sub-dependency of PHPStan (or related modules) is somehow forced back to a version only supported on older releases. Both of those will however likely fail the jobs (baseline not matching) so will be know very quickly, and if that does occur the ^2 constraint alone would fail the Composer stage meaning both scenarios require interaction by the Core team to resolve. MR!12525 would suffer the same risk.

Essentially the risk to core is minimal, and would generally just show up at a different stage.

Linking 🐛 Can't update to 11.2.0 Active for cross-issue visibility (on Slack there was originally some discussion of using that issue to handle this concern).

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily 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.

Bot is only testing MR!12525.

That said it may be legitimate tooling failure as it would change the tooling requirements.

Back to NR for MR!12510, if it weren’t for the required FM sign off I would call it RTBC.

mondrake → changed the visibility of the branch 3532698-no-composer-dev to hidden.

Closed MR!12525.

Yeah 12525 is interesting but way out of scope; let's explore more in @cmlara's related issue.

@cmlara, issues can be RTBC with the "Needs $foo review" tags when $foo is a committer role, especially if the issue is otherwise RTBC, because the RTBC queue is the list of issues committers review. :) So setting RTBC based on @cmlara's comment (not my own knowledge).

I think relaxing the constraint is fine, it's not entirely clear to me from the issue summary whether phpstan 10 and 11 have mutually exclusive configurations or whether it's only the constraint that's the problem - but we can't do much about the mutually exclusive configurations if that's a problem anyway.

mondrake → closed merge request !12525

Alright then.

We probably want a change record then though?

I don't think it needs a change record - we're widening a constraint that was only in a couple of patch releases - everything will automatically start to work for modules it was broken on, modules that already updated to phpstan 2 don't need to downgrade again.

OK, let's split the difference at a small release note in the patch release, which I've added to the IS.

Also confirmed the issue with the original more open constraint had no CR.

Thanks all, committed to 11.x and backported to 11.2.x

larowlan → closed merge request !12510