Skip to content

[Cache] Fix compatibility with Redis 6.1.0 pre-releases #58169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 6, 2024
Merged

[Cache] Fix compatibility with Redis 6.1.0 pre-releases #58169

merged 1 commit into from
Sep 6, 2024

Conversation

cedric-anne
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #57884
License MIT

The solution provided in #57885 will fix the compatibility with the PHPRedis 6.1.0 release, but the issue persists on the pre-releases (for instance the 6.1.0RC1 version).
I propose to use the new signatures for any 6.1.0 version, including its pre-releases.

version_compare('6.1.0RC1', '6.1.0', '>='); // false
version_compare('6.1.0RC1', '6.1.0-dev', '>='); // true

@derrabus
Copy link
Member

derrabus commented Sep 4, 2024

Version 6.1.0 hasn't been released yet, which basically means that the maintainers could decide to release an RC2 which will again break the logic that you're proposing. Is that correct?

In that case, I wouldn't do anything here. The problem will resolve itself with the next release of ext-redis.

@derrabus derrabus added the Cache label Sep 4, 2024
@carsonbot carsonbot changed the title Fix compatibility with Redis 6.1.0 pre-releases [Cache] Fix compatibility with Redis 6.1.0 pre-releases Sep 4, 2024
@cedric-anne
Copy link
Contributor Author

Version 6.1.0 hasn't been released yet, which basically means that the maintainers could decide to release an RC2 which will again break the logic that you're proposing. Is that correct?

An RC2 would still use the new signature. Comparing the version with >= 6.1.0-dev would include any 6.1.0 pre-release, and every future releases.

In that case, I wouldn't do anything here. The problem will resolve itself with the next release of ext-redis.

@derrabus
Copy link
Member

derrabus commented Sep 4, 2024

Oh right. I misunderstood your initial message. LGTM then.

@cedric-anne
Copy link
Contributor Author

Oh right. I misunderstood your initial message. LGTM then.

It was not really clear. My bad.

@cedric-anne
Copy link
Contributor Author

I pushed a new commit as the signature was not correct. I had the following error: Compile Error: Declaration of Symfony\Component\Cache\Traits\Redis6ProxyTrait::hSet($key, $fields_and_vals): Redis|int|false must be compatible with Redis::hSet(string $key, mixed ...$fields_and_vals): Redis|int|false

After applying the patch, this error is gone.

@fabpot
Copy link
Member

fabpot commented Sep 6, 2024

Thank you @cedric-anne.

@fabpot fabpot merged commit 76df329 into symfony:6.4 Sep 6, 2024
9 of 10 checks passed
@cedric-anne cedric-anne deleted the patch-1 branch September 6, 2024 07:49
This was referenced Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants