Incompatibility between zend-diactoros and psr-http-message-bridge versions: require symfony/psr-http-message-bridge >=1.1.2

I think we should try to fix this upstream. I think we need to change line from

$psrResponse = $psrResponse->withHeader('Set-Cookie', array());

to

$psrResponse = $psrResponse->withoutHeader('Set-Cookie');

https://github.com/symfony/psr-http-message-bridge/pull/64 . There's the started (not by me) PR
https://github.com/symfony/psr-http-message-bridge/commit/8592ca33533224... . There's a conversation about it

There is an upstream issue already - see https://github.com/symfony/psr-http-message-bridge/pull/64

@mikelutz got there first :)

We should update the issue summary to say that that is the preferred solution.

So https://github.com/symfony/psr-http-message-bridge/pull/64 is now merged upstream so we're waiting on a release.

Also a new release has been tagged too! https://github.com/symfony/psr-http-message-bridge/tree/v1.1.2 is out.

I think we should add a conflicts to core/composer.json for 1.1.1 and be done here.

#3039611-27: Update core PHP dependencies for 8.8.x is discussing the same issue. Thanks, for creating the issue.

#8: I think alternatively we can require ^1.1.2?

This patch does that. Interestingly, our composer.lock already lists 1.1.2 since #3045967: Update Symfony components (3.4.24) and Twig (1.38.4) to latest bugfix release! 🤔

@Wim -

There are a few additional discrepancies where our composer.lock has higher versions than the composer.json minimum, a few of which also cause problems if someone does a composer downgrade as shown in #3044175: [DO NOT COMMIT] Vendor minimum testing issue

I wonder if it would be best to create an issue to attempt to address all of them at once (including running the min/max testings on all 3 currently supported php versions and making sure we aren't missing anything else)

The examples above and the two min/max issues only test for full min/max vendor versions. This example in this thread requires one library at composer minimum and another not at composer minimum, and there isn't much we can do about that in drupal. The conflict here ideally should have been resolved with dependency management in either diactros or message bridge, but there isn't much we can do about it now other than set our minimums for both above the problem versions. We can't realistically test every allowed vendor version combination, unfortunately.

Thanks for that background information, @mikelutz! Very helpful :)

That being said, what you're describing is only tangentially related to this issue. The only reason I even brought it up is because core committers would and should expect that this patch also updates composer.lock, but it does not. Because that already happened elsewhere — be that by accident or not, it does not matter. What matters is that that means the scope of changes made by this patch is even smaller!

Any reason we shouldn't RTBC and commit this?

@alexpott indicated that @mikelutz would hopefully RTBC this, so assigning to him :)

Yes, this is an obvious oversight. It should be backported to 8.7 as well, but will need a new patch due to recent composer changes in 8.8.

Of course, done. The 8.7 composer.lock already lists 1.1.2 as well, so there too we only have to update composer.json.

We can also pin "zendframework/zend-diactoros": "<=1.7.2" see #3039611-36: Update core PHP dependencies for 8.8.x

Is this a duplicate of #2989631: Update zend-framework component dependencies?

#19: that would prevent future updates, which would move us to a less future-compatible place?

#20: AFAICT symfony/psr-http-message-bridge <1.1.2 is incompatible with zendframework/zend-diactoros >1.8.5 and #2989631: Update zend-framework component dependencies bumps the minimum to 1.8.6.

This issue is not about updating, it's only about fixing a compatibility issue. hence the tiny scope.

Committed c7ad1c7 and pushed to 8.8.x. Thanks!

I agree that we probably should backport this to 8.7.x as our dependencies are currently incorrect and if you install with prefer lowest you can end up with a broken site but I'm going to ping a release manager before backporting.

Discussed with @catch and agreed to backport.

Committed 321576f and pushed to 8.7.x. Thanks!

Cherry-picked to 8.7.x. Edit: now I see why the cherry pick gave strange messages. Sorry, I got pinged about this issue in a different channel and cross-posted. Harmless I think; just noise in the commit log.

I should have noticed this before, but... When we change dependency constraints we should mention it in the release notes, especially for patch releases where this is an unusual occurrence. I wrote a release note for this, but for the future, any changes to composer.json and comoposer.lock are worth considering for release notes. Especially when it's a minor update in a patch release as in this issue.