Microsoft team submits Redis patch to enable Windows support

[yread]

Wow, I'm kind of surprised by the amount of snark ("eww it's 140k lines", "mini git tutorial", "patch file instead of pull request").

It seems it's so big because it contains the libuv. The instructions to compile on Windows don't seem trivial at all and if I cared enough to try this I would appreciate that they wrote it step-by-step.

The guys at MS just sat down and made it work while antirez was throwing out suggestions how to make it work with " the behavior of the window filesystem is so incredibly broken that well they should really fix it I guess"

Sorry for the rant I would just expect better from the community.

[antirez]

The patch provided does not handle persistence correctly (saving blocks), does not make tests passing. The "libuv" part was the trivial part, already solved by the community, see the unofficial win32/win64 port that fixed it natively, with less code.

So nice to see Microsoft contributing code to Redis, but this is not a production ready port and is practically equivalent to what we already had made by the community.

Also, what is the point on having a production quality Redis server on Windows? That it will slow down the Redis POSIX development if we merge the two projects, the WIN32 API is different, there is a lot of care needed to maintain a port, but what is the real gain? Even services based on Windows like Stack Overflow had no issues running their Linux boxes to use Redis.

I've the bad habit of doing the interest of the Redis community, so once I saw there was a reasonable port of Redis for win32 (that is, enough to code under Windows and test stuff, no production ready) I avoided additional efforts in this regard to provide more value in the "real" Redis, the one running where 99.99% of people need it to run well, under POSIX environments.

EDIT: I was not clear in this post about what I'll do. I'll not merge the patch, but I'll see with interest the creation of a "redis-win32" project that has a different repository, different issues page, and so forth, and is not officially supported by me. But I'll be happy to provide a page in the redis.io site about it, to link at the project, to collaborate with the developers, and so forth.

[Legion]

I think patch versus pull request is a valid complaint, but there's absolutely no reason to be snarky about it.

Everything else just reeks of MS bashing that reinforces the negative view of the open source crowd that many on the MS side of the divide have.

Github is looking to pull the Windows crowd into their world; on the blog, they stated that as an explicit goal for their hiring of Phil Haack.

So, please, play nice. I would much rather Github stay a fairly harmonious place than be another open source vs. MS battleground.

[to3m]

I couldn't be bothered with writing a rant, so I'm glad somebody put the effort in. You're right... it's a bit off. Just a chance for people to feel superior over the Windoze lusers, I guess :)

I trust that the people behind the patch have got from it what they want, and that if the patch is just sent straight to the recycle bin then it will be no skin off their collective noses.

[manojlds]

I mentioned the mini git tutorial

I am a MS fan and I use msysgit on Windows.

My point being, git now has good enough implementation on Windows and the team should have taken the time to learn it, as in this context, it doesn't look good that they are ignorant of git.

[blhack]

If anybody, like me, was curious what a "pull request" is[1]:

You fork the repo, submit a patch to that, then ask that the maintainer(s) of the original repo "pull" your code into theirs.

[1]http://help.github.com/send-pull-requests/

[mythz]

The snark on this thread is concerning, Microsoft has made a good gesture in trying improve the Redis story on Windows and IMO it's something we all should be encouraging as it can only serve to improve the Redis ecosystem.

Historically Microsoft hasn't been too fond of NoSQL but positive steps like this validates Redis in the eyes of Windows devs which has the potential to attract new devs to the world of Redis and NoSQL.

I personally hope to see this implementation improve so it runs flawlessly on Windows and Azure.

[beagle3]

> Microsoft has made a good gesture

When on the other hand they are poisoning the Android/Linux ecosystem with FUD, patent extortion and the like. While I would prefer the discussion to stay civil and technical, Microsoft is consistently earning every snark they are receiving, and then some.

[js2]

[Edited for tone which I guess is the reason for the down votes. I appreciate the quick response below.]

Most of this patch is adding libuv, which is included in its entirety due to "the version included in the patch is different than the one available on github, some changes have been added to the code". There is also a lot of cleanup. Also a small nit, in the patch instructions:

  git checkout 3fac86ff1d
  git checkout -b 2.4_win_uv

is equivalent to:

  git checkout -b 2.4_win_uv 3fac86ff1d

Here's what would make it more easily reviewable.

1. clone redis

2. clone libuv

3. make whatever changes needed to libuv as its own commit.

4. add libuv as a submodule to redis.

5. perform all the misc compiler cleanup stuff to redis as its own commit; usually you want your cleanup/refactored to happen before you perform functional changes.

6. add the ms-specific code as its own commit.

7. push up the new commits to the two forked repos.

This would make it all a bit easier to review. The only questionable part is adding libuv as a reddis submodule (4). Maybe I'd leave that part out initially and instead just specify the equivalent manual step needed there (clone our fork of libuv into X and checkout Y).

[spicyj]

Did most of what you suggested: https://github.com/antirez/redis/issues/238#issuecomment-307....

[tantalor]

  > 1. cloned redis
  > 2. cloned libuv

Of course you mean "fork", not "clone".

[jrockway]

Embrace, extend, extinguish.

Does Redis still do that thing where it forks and the child writes its core to disk? How does that work under Windows, which doesn't have fork?

Finally, this is one big patch:

    339 files changed, 146821 insertions(+), 290 deletions(-)

With many of the changes along the lines of:

     static void *callbackValDup(void *privdata, const void *src) {
    -    ((void) privdata);
         redisCallback *dup = malloc(sizeof(*dup));
    +    ((void) privdata);
         memcpy(dup,src,sizeof(*dup));
         return dup;
     }

or

    -    cmd = malloc(totlen+1);
    +    cmd = (char*)malloc(totlen+1);

Eliminating compiler warnings is nice and whatever, but probably not the best thing to include in your "add major feature" patch.

[atuladhar]

Not sure what they're going to do regarding fork, but they say this at the end of the gist:

TODO

    Snapshotting (Fork and Write) is not perfect, right now we simply block requests while memory is dumped on disk. We are working on a solution that will give us better performance. An update will be released soon.

[karolist]

I wonder if they contacted Redis author before starting to work on this. You know, with the patch so big and radical, there's a possibility he doesn't even want to accept it.

What then, all this effort for basically nothing except a fork, which you then have to continue maintaining etc.

[j_baker]

Correct me if I'm wrong, but casting the result of malloc is generally considered bad form in C, isn't it?

[burke]

There are 453 mentions of "fork" in the patch, so I'm imagining it does.

[adrianpike]

Can somebody more familiar with the Windows environment explain why prn.h is an "invalid file name"?

edit:// thanks all! :)

[kogir]

It's reserved for compatibility:

"Do not use the following reserved device names for the name of a file:

CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, and LPT9. Also avoid these names followed immediately by an extension; for example, NUL.txt is not recommended."[1]

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/aa36...

[jharsman]

You can't have files named like the old DOS devices: CON, LPT, AUX, PRN etc. This is originaly due to CP/M backwards compatibility, it didn't have directories so magic files were udde to pipe stuff to printers and other devices.

[X-Istence]

https://github.com/antirez/redis/issues/100

[rbanffy]

PC-DOS inherited it from CP/M 80. Windows was built at first on top of it and then replaced it in stages. I have no idea why would this still be present in the current codebase. It would probably be easier just to fix the filesystem.

[manojlds]

prn is for printer and there are many such reserved device names

http://en.wikipedia.org/wiki/Device_file#Device_files

[mythz]

Given its 'alpha state' it won't likely be merged in the main Redis anytime soon.

Though it is at least a validation by Microsoft of how good Redis is and its wish to see it run natively on Windows.

[vier]

Using libuv from Node.js too. Awesome.

[sehugg]

I'm way behind on Windows tech, but it's made a token attempt to support a crippled POSIX subsystem since Windows NT. This is supposed to be the modern equivalent:

http://support.microsoft.com/kb/324081

[thedumpster]

Been looking to port SQL Server to NIX but I can't seem to find it on github?

[manojlds]

Look at the instructions to create a branch out of a commit.

Just

git checkout -b 2.4_win_uv 3fac86ff1d

would have sufficed

Instead, they give a mini git tutorial.

This comment is not a taunt at MS. Just that they have tried to learn git and the process, given that git now has a very good implementation on Windows and since they are trying to do something similar here - bringing Redis to Windows - it doesn't look good.

[splitbrain]

A patch file in a gist instead of a pull request? sigh

[ComputerGuru]

A comment at TFA by benatkins has a nice answer:

they probably mainly want feedback from the project maintainers at this stage, and the project maintainers can apply a patch just about as easily as they could apply a pull request. (It's so big that the web view isn't likely to be useful.) Also this will probably be going into a new branch if anywhere, so does a pull request to an existing branch (which is all that's possible AFAIK) even make sense?

[briancurtin]

It's really no wonder why people don't contribute to open source when this is the first response.

[diminish]

can someone please submit a patch to MS to add git pull support to team foundation server or visual sourcesafe (or what are MS guys using these days as version control?)

[aaronlerch]

That was my first thought too. :)

[cientifico]

Ok. I got it! They don't want to use git, because git is from Linus ! so they just submit a patch :-P

[cientifico]

Spent the time on doing such big patch, and not spend the time on learning (5 min max) how to do a pull request... make me think they were forced to do this kind of thing.

So once the code is in, no maintainer from microsoft will be.