11 ms·
Microsoft team submits Redis patch to enable Windows support
- deleted 15y ago[deleted]
- jrockway 15y agoEmbrace, 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 15y agoNot 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.
- jrockway 15y agoSo this 146821 line monstrosity isn't even theoretically usable? (You know, I could rewrite Redis in less than 146,000 lines of code...)
- deleted 15y ago[deleted]
- burke 15y agoThere are 453 mentions of "fork" in the patch, so I'm imagining it does.
- karolist 15y agoI 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.
- tptacek 15y agoI'd be surprised if he accepted a patch that added a dep; Salvatore seems allergic to deps (something I like about Redis).
- darklajid 15y ago(on a mobile and fat-fingered a downvote, sorry) I wouldn't expect anyone to accept the patch in this state, but I hope that the "who cares about Windows" attitude dies and a dialog to get proper support into redis is started. It's a shame that we're using an unofficial version on that platform right now.
- tptacek 15y agoYou don't have to have a "who cares about Windows?" attitude to write programs that don't work on Windows; all you have to have is a "want to make the best use of Unix" attitude.
- j_baker 15y agoCorrect me if I'm wrong, but casting the result of malloc is generally considered bad form in C, isn't it?
- jrockway 15y agoI'm just assuming it's a Windowsism.
- to3m 15y agoIn general, yes. For Win32, it is safe. I would imagine they have activated "Compile as C++" for certain files. VC++ doesn't support C99, and it's probably easier to fix up any C++ incompatibilities than try to C89-ize everything.
- nknight 15y agoI 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 15y agoI 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.
- unwind 15y agohttp://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc http://stackoverflow.com/questions/605845/do-i-cast-the-resu... is a good place to start for a deeper discussion of this topic, if someone is interested. My answer (obviously) is "yes", although as some folks point out here too, there are portability concerns that make it worth it sometimes.
- splitbrain 15y agoA patch file in a gist instead of a pull request? sigh
- aaronlerch 15y agoThat was my first thought too. :)
- diminish 15y agocan 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?)
- wisty 15y agoI think they use Perforce (or at least, a heavily patched version they can "Source Depot"). That might have changed since Team Foundation was brought out. I don't think they ever used visual sourcesafe.
- Splines 15y agoI'm in Office and we currently use Source Depot, which is a modified version of Perforce. We have so many tools that interoperate with it (and everyone knows how it works) that I don't see us moving away from SD anytime soon. We use Team Foundation for a few other things related to project management (personally, I'm not a fan), but not for source control.
- darklajid 15y agoYou might want to check out git tfs. As someone working in a company that relies on tfs I'm going to do the same to keep my sanity..
- ComputerGuru 15y agoA 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?
- donspaulding 15y agoThe README on that gist reminds me how fortunate I am to work with Ubuntu servers. Building redis from source on Linux is literally "git clone https://github.com/antirez/redis https://github.com/antirez/redis && cd redis && make". And that's the hard way to install things in DebianLand.
- vier 15y agoUsing libuv from Node.js too. Awesome.
- adrianpike 15y agoCan somebody more familiar with the Windows environment explain why prn.h is an "invalid file name"? edit:// thanks all! :)
- manojlds 15y agoprn is for printer and there are many such reserved device names http://en.wikipedia.org/wiki/Device_file#Device_files http://en.wikipedia.org/wiki/Device_file#Device_files
- jharsman 15y agoYou 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 15y agohttps://github.com/antirez/redis/issues/100 https://github.com/antirez/redis/issues/100
- smackfu 15y ago"finally the behavior of the window filesystem is so incredibly broken that well they should really fix it I guess." Ha. Just allow "prn", I'm sure that won't have any side effects.
- kogir 15y agoIt'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/aa365247(v=vs.85).aspx#naming_conventions http://msdn.microsoft.com/en-us/library/windows/desktop/aa36...
- 15y ago
- manojlds 15y agoLook 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.
- mythz 15y agoGiven 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.
- js2 15y ago[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 15y ago> 1. cloned redis > 2. cloned libuv Of course you mean "fork", not "clone".
- deleted 15y ago[deleted]
- deleted 15y ago[deleted]
- spicyj 15y agoDid most of what you suggested: https://github.com/antirez/redis/issues/238#issuecomment-3070742 https://github.com/antirez/redis/issues/238#issuecomment-307....
- js2 15y agoExcellent. Thank you, and I apologize for the harshness of my original comment. Was honestly just trying to be helpful.
- yread 15y agoWow, 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.
- to3m 15y agoI 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.
- deleted 15y ago[deleted]
- manojlds 15y agoI 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.
- antirez 15y agoThe 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.
- cientifico 15y agoSpent 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.
- cientifico 15y agoOk. I got it! They don't want to use git, because git is from Linus ! so they just submit a patch :-P
- mythz 15y agoThe 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 15y ago> 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 15y agoThey 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 15y ago> 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.
- sehugg 15y agoI'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 http://support.microsoft.com/kb/324081
- thedumpster 15y agoBeen looking to port SQL Server to NIX but I can't seem to find it on github?