Skip to content
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

Windows compatibility patches #838

Closed
mehrdadn opened this issue Jun 27, 2020 · 4 comments
Closed

Windows compatibility patches #838

mehrdadn opened this issue Jun 27, 2020 · 4 comments

Comments

@mehrdadn
Copy link

mehrdadn commented Jun 27, 2020

Hello,

We've been using the native hiredis client from this repo to interface with tporadowski/redis on Windows, but to do this, we've had to apply a number of compatibility patches.

Would you be interested in incorporating the patches upstream?

You can find our patches here (the ones mentioning hiredis). We currently use Redis 5.0.9.

Some of them may not be important/desired upstream (e.g. for suppressing build output), but others are compatibility patches that I think should be relatively useful for other users and easy to incorporate upstream.

If you have any questions about them please let me know.

@michael-grunder
Copy link
Collaborator

michael-grunder commented Jun 27, 2020

It looks like we've already applied most of the patch logic to Hiredis since the version that was packaged with Redis 5.0.9.

If you clone current hiredis master you'll see for example that we've patched out SIGPIPE as well as have a pass-thru compatibility ioctl that gets used when we detect Windows.

Additionally your connect -> connect_ name conflict patch seems unneeded as that is now called do_connect.

That being said I am not a Windows developer so if we have some incorrect logic vis-a-vis Windows in current hiredis master I'd love a PR to fix those issues, as we do attempt to support Windows as best we can.

Edit: We presently run tests in Windows but I do have plans on properly fixing all of those type errors at some point.

@mehrdadn
Copy link
Author

Oh wow, sorry about that, I didn't realize! I'll try to pull the updates and see if there's anything left.

By the way, can I ask what the difference is between the this repo and https://github.com/antirez/redis/tree/HEAD/deps/hiredis? When should users use that one vs. this one?

Thank you!

@michael-grunder
Copy link
Collaborator

By the way, can I ask what the difference is between the this repo and https://github.com/antirez/redis/tree/HEAD/deps/hiredis?

It's the same repository only the one packaged with Redis is typically older. Periodically Antirez will pull newer versions of hiredis into Redis as needed (in much the same way he periodically updates jemalloc).

I think Redis unstable currently has hiredis 0153527 or at least very close to that. Redis will likely be updated to hiredis v1.0.0 after we release it.

@mehrdadn
Copy link
Author

Ah, so we should probably use this one. Great, I'll close this for now and let you know if I see anything else. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants