

Microsoft team submits Redis patch to enable Windows support - aaronlerch
https://github.com/antirez/redis/issues/238

======
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.

~~~
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.

~~~
burgerbrain
The complaint about the size is most certainly _not_ just "MS bashing".
Dumping massive flawed pieces of code on people out of the blue and expecting
them to be grateful is bad form, no matter who you are.

Unfortunately this seems to be Microsoft's MO.

~~~
Gigablah
How are the MS people who submitted the patch "expecting" others to be
"grateful"?

~~~
burgerbrain
Perhaps they are not, but the people complaining about "snark" are what I am
talking about.

Anyway, my point is that complaints about the size are very valid.

------
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.

~~~
chimeracoder
They might be doing other things you don't like, but at least give credit
where credit is due.

Submitting code upstream to Redis is completely independent of their patent
decisions regarding Android. I'm not a fan of their overall stance with
respect to FOSS/Linux/Android, and they may be earning the snark there, but
not in this case.

These discussions become a lot more valuable when we stop characterizing any
organization, especially one as complex as a large corporation, as universally
'good' or 'bad'.

~~~
rbanffy
> give credit where credit is due

Sure. Just don't forget they profit from Windows sales and, thus, will do
anything to justify the deployment of a Windows server instead of a Linux one.
Including contributing to a Unix-native product. If, in the end, Redis'
codebase becomes cluttered and performance and maintenance suffer, we all
lose. I mean, all of us except Microsoft, who wins both by us losing and from
gaining space for their own future offerings in this segment.

There is no good or bad. It's self interest. When their self-interest
coincides with the society's, I'm for them. OTOH, it's been a long time since
it last did. It certainly never happened after the mid 90's.

~~~
mythz
Of course they're selfishly motivated - everyone is. The point is this time
Redis/NoSQL stands to benefit as well.

~~~
d0mine
It seems you've skipped the sentence:

 _If, in the end, Redis' codebase becomes cluttered and performance and
maintenance suffer, we all lose._ </quote>

~~~
mythz
That's a case for not merging, not for a parallel win32-based project - which
this implementation could serve either of.

~~~
rbanffy
It would risk diverting attention from the Unix port to the Win32 one. Even if
the Redis developers don't pay attention to this distraction, its mere
existence fragments the codebase and creates two semi-compatible versions.

And if the Windows version sucks badly, any Windows user who tries to install
it on Windows will end up blaming Redis and try something else. On Windows.

Microsoft wins and we lose on all scenarios.

~~~
mythz
This is a very pessimistic view of the world and future events (which I don't
share). I'd get that looked at.

A bad Redis port by Microsoft makes them look bad, whilst their endorsement of
Redis makes Redis looks good.

The 'pro developers' that Redis attracts are going to be more than capable of
identifying the culprit behind a shitty port.

------
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).

~~~
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.

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

~~~
nknight
I wouldn't so much call it "bad form" as just non-idiomatic. It doesn't really
have big downsides in practice.

It's mostly a habit people pick up from C++ (where it's mandatory due to
stricter typing), and if you want to build C code with a C++ compiler, you
need to add the casts.

Given Microsoft's C++ fetish, I'm unsurprised by this.

~~~
mappu
I suppose one downside would be if you cast a voidptr to a pointer to (an
element that has a different size), then calling operator++ moves it by more
than if you had left it as a voidptr.

------
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...](http://msdn.microsoft.com/en-
us/library/windows/desktop/aa365247\(v=vs.85\).aspx#naming_conventions)

~~~
dugmartin
I once saw some interesting behavior on a Windows 95 machine - my brother
printed a drawing for a convention to a file. The printer was a HP LaserJet
with plug in module that enabled Postscript. He named the file "con" and when
he hit OK on the print dialog the entire screen buffer (over the windowing
interface) started filling with Postscript.

~~~
motti
Windows tries to prevent you naming a file "con". I once renamed a text file
con in Win95; using a raw disk editor to change the file entry and it did this
too - every time I saved text in the file it would write it console/DOS style
to the screen.

------
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_

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

~~~
mindstab
And if someone previously uninvolved with your work showed up one day and
dumped a few weeks of a teams work in your lap in one giant patch that
affected most of your codebase, you'd say thanks and look to merging it in
rather than talk about proper process?

Most projects have a process and gigantic monolithic patches that include
entire new projects and "cleanup" updates to core all wrapped up in one are
unmanaged and unmergable. It's for the sake of process.

~~~
briancurtin
Of course I'd prefer they work in a way which meshes with existing process,
but I'm not going to act dismissive and sigh if someone goes another way. If
someone removed the GIL from CPython and gave me a context diff, for one I
would barely be able to read that format, but I'd find a way to make it work.

Pull requests are great, but it's not like someone couldn't apply the patch
and give them a hand - earlier in the thread someone did just that.

------
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.

