Possibility for multiple Credis versions to conflict

[justinbeaty]

Preconditions (*)

1. OpenMage 20.12 (but I think it will be reproducible in any version)
2. colinmollenhour/cache-backend-redis installed, and using the version of Cm_RedisSession in this repo.

Steps to reproduce (*)

1. Install preconditions as noted above, before Fix undefined offset on redis session #1254 was merged, it was impossible to log in

Expected result (*)

1. Log in in should work as normal

Actual result (*)

1. With colinmollenhour/cache-backend-redis installed, credis was being loaded from vendor instead of lib/Credis

Discussion

Since #1254 is now merged, I think the more urgent issue is resolved. I've been running the patch from that PR for the past week, and things are okay. However, I am running as headless Magento, and only the admin session creates a redis session. It's possible there are other incompatibilities between the different versions of Credis, but I haven't run into any as far as I know.

Here's the possible different versions that could be installed as I understand it:

• The version in this repo has a copyright of 2011, so pretty old
• The version (as a git submodule) in the external Cm_RedisSession is from about 4 years ago
• The version required in Cm_Cache_Backend_Redis (through composer.json) is set to "*", so the latest.

So this issue is more of a discussion on how small things like this should be prevented. Perhaps instead of lib/Credis, you would need to install Credis via composer, and the Cm_RedisSession in this repo is updated to support the latest version? Or maybe Cm_RedisSession is removed from this repo and we only should use from composer? Unfortunately, Magento 1.x comes from a time before when composer was popular, and we also have to support people who do not use composer. But, I don't think composer could even have solved this problem, except that it might just warn that you are trying to use two different versions.

[justinbeaty]

I also just re-read Flyingmana's comment in #1705 where it would be best to not involve composer in fixing this, likely because there are still many non-composer users. I think the best fix then is probably updating this repo's version of Cm_Cache_Backend_Redis to work with the latest Credis, and also updating lib/Credis to the latest as well. That way, if one did install Credis via composer (via Cm_Cache_Backend_Redis), then there is no incompatibility. But we would have to wonder if any 3rd party modules rely on the old Credis lib, and updating it would break those modules? I don't have a really good answer yet to all of this so I'm interested to hear other opinions.

[Flyingmana]

related to #524
So updating our code, and marking the CM_Redis repository as included in our composer.json may be the easiest.

[justinbeaty]

@Flyingmana I think that can work. We should target 20.x with the updated lib/Credis, since there is the breaking change in the Credis lib. I do not know if other modules may have used Credis directly as well.

I am still not too sure in what case someone would use the Cm_RedisSession in this repo vs the external one. I was using the external one for a while until I decided to use the built in one. Does the external one just have different features?

[justinbeaty]

This should be fixed with v19.5.0-rc1 and v20.1.0-rc1