Fix compilation errors on Win32 with VC11

[f0ff886f]

This brings the W32 compilation fixes done by @char101 up to date and
allows building of php_redis.dll with VC11 on Win32 (allows for a php5.5
version).

[weltling]

C89 compatibility is the rule for PHP, plus there aren't only gcc and visual studio in the world :)

Apart from that, looks good, the automatic snap succeeded for 5.5 http://windows.php.net/downloads/pecl/snaps/redis/2.2.4/ . Also it loads into PHP. Perhaps you could test that snap DLLs too?

Btw. what's the issue with older PHP versions?

[f0ff886f]

Understood about C89 standrs, I was taking the path of least resistance to get a DLL built against 5.5.

I don't know of issues with older versions of PHP, there are libs built against 5.3 and 5.4 here: http://char101.github.io/phpredis/

I just wanted to get 5.5 builds for my environment, I'm not one of the developers/maintainers of phpredis :)

[weltling]

Builds for older PHP versions was failed from your master, please see logs.

Yep, i was pointed to that issue ticket by Remi and found your PR. From the logs, could you pls try to

#include "win32/php_stdint.h"

instead of stdint.h ? That should fix compilation for older PHP versions, you could try if you've got VC9. Even if you're not the ext dev, why not to get it worky for the older PHPs?

[weltling]

i mean like

#ifdef PHP_WIN32

include "win32/php_stdint.h"

#else

include <stdint.h>

#endif

[weltling]

oh no, it eats up my code (

[f0ff886f]

OK, with that change to the include I have built the DLL on VC9 for php5.3 x86, php5.4 x86, and VC11 php5.5 x86, php5 x64.

I also fixed all the comments, new commit should be in the pull request.

[weltling]

Huh, there've been pretty much comments :)

I've triggered the snap again (overwrote the previous URL) and now all the builds are successful. That means this PR is ready to be merged. Good job!

[pierrejoye]

Any chance to merge it and release a new version? DLLs can then be available from pecl.php.net

[michael-grunder]

Hey,

I really am planning on going through this (as well as a few other recent issues) so we can get it up for everyone. I have been out of town on business and haven't had a chance recently.

Cheers,
Mike

[michael-grunder]

OK guys. I now have a Windows laptop and should be able to get myself a copy of VC11 so I can test the changes (and then in Unix). I'm unfamiliar with the windows build system so I may have questions after sorting merge conflicts.

Cheers,
Mike

[guysoft]

@michael-grunder I can help you out with that.

For now it seems like what ever I do I get this error (managed to pass the igbinary by compiling it and placing the files where redis is looking for them):
#455 (comment)

If you make a windows branch and try and fix that, I can give you instant feedback

[f0ff886f]

@guysoft I don't know why you're having troubles with VC9 and my branch, it compiled fine previously and also for the PECL guys (http://windows.php.net/downloads/pecl/snaps/redis/2.2.4/).

I'll try again tomorrow when I'm back at the machine that I was doing the work on, but its odd it doesn't work.

@michael-grunder If any questions/problems arise I'd be happy to help as well (on VC11 it's a much simpler process to get everything going), but in any case https://wiki.php.net/internals/windows/stepbystepbuild is a great resource.

[michael-grunder]

@vostok4 So should I grab a copy of VC9, VC11 or both? Once I get them and the branch merged I'll try and knock down any compilation errors. Like I said, the only concern I have is my complete unfamiliarity with the windows build system 😃

[guysoft]

@vostok4 Tried, and it fails.
This is what I get from your branch:

library.c
ext\redis\library.c(314) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
ext\redis\library.c(1502) : error C2065: 'uint8_t' : undeclared identifier
ext\redis\library.c(1502) : error C2065: 'val8' : undeclared identifier

[f0ff886f]

@michael-grunder to me you should start on VC11, it is the simplest to get going. In the meantime, tomorrow I will try to merge all the commits since October (and have there been a few!), and provide an up-to-date patch that builds.

[f0ff886f]

OK, just to clarify, I just built phpredis and igbinary against php-5.5.11 in VC11 x64 on Win7x64. Here are the steps (based on the guide at https://wiki.php.net/internals/windows/stepbystepbuild):

cd \php-sdk
bin\phpsdk_setvars.bat
cd phpdev\vc11\x64\php-5.5.11
buildconf
configure --enable-cli --enable-redis=shared --enable-redis-session --enable-igbinary=shared --enable-redis-igbinary
nmake

And for clarity, I used the branch from my repo (nothing changed since october), and igbinary-master as of today (from https://github.com/phadej/igbinary).

I will now try VC9 nts to confirm that still works.

[char101]

For faster compilation you can use this flags

configure --disable-all --enable-cli --enable-session --enable-hash --enable-redis=shared --enable-redis-session --enable-igbinary=shared --enable-redis-igbinary

for nts version

configure --disable-all --enable-cli --enable-session --enable-hash --enable-redis=shared --enable-redis-session --enable-igbinary=shared --enable-redis-igbinary --disable-zts

[f0ff886f]

Thanks @char101, I was getting hung up on on why --enable-session didn't work in conjunction with --disable-all (I needed --enable-hash).

[guysoft]

@vostok4 Hey,
Managed to compile your tree, but I had to add at the start of library.c:

typedef unsigned char     uint8_t;
typedef unsigned short    uint16_t;
typedef unsigned int      uint32_t;
typedef unsigned long int uint64_t;

[f0ff886f]

OK, I've updated my code to reflect the top of tree from upstream, so hopefully the merge is easier for you. It's in two commits:

1. vostok4@9c12c40 This is the main commit that merges code (and fixing issues for Win32)
2. vostok4@566d673 This is just a commit fixing a ton of comments as per @weltling's comment earlier for C89 compliance

It builds with VC11 and VC9, tested here.

[char101]

VC 2012 has stdint.h so you can just include it

#include <stdint.h>

I think vostok4 doesn't got these errors because he linked with igbinary. Igbinary header include stdint.h.

[f0ff886f]

I think @guysoft is building with VC9 so we should probably use:

#ifdef PHP_WIN32
include "win32/php_stdint.h"
#endif

What's odd is I don't get any build errors even without building/linking igbinary on my system with VC9.

[char101]

I miss the code, all the uint8_t in library.c are inside #ifdef HAVE_REDIS_IGBINARY, so if it fail it should fail when compiling with igbinary support.

[guysoft]

Ok seems like f0ff886f@9c12c40 builds for me fine on VC9, no changes required. Will push it over for people to test it.

[michael-grunder]

Man, this is fantastic you guys. Honestly I think it may require a version bump so I can get you in the release notes. Amazing work.

Cheers!
Mike

[guysoft]

First glance here looks ok. Builds and the extension loads on windows.

[Jan-E]

See http://phpdev.toolsforresearch.com/php-5.5.11-Win32-VC11-x86.zip
I have built this with vostok4's sources. Periodically I will announce new builds on Apachelounge.

[pierrejoye]

Ok, it has been months since we provide PRs to fix windows builds and other issues.

What is the actual issues that prevent these PRs to be merged? It looks like a common case.

Maybe a fork could help and ease the development of this package. Or how can we help?

[pierrejoye]

And I am sorry but the current master is totally broken. So a long term solution must be found, one way or another. I am open to suggestion but I can't ask my team to provide PRs endlessly without results. At some point we need a working extension with released fixes.

[michael-grunder]

Hey,

The issue on my end is that I don't have a windows box on which to test it, and don't run phpredis in a windows environment at all. That, and I'm currently about 10k lines into implementing cluster support for phpredis, which is taking up my time.

This pull can no longer be automatically merged, but if I got it merged I would need assistance from the community to build/test it (again, I don't run Windows and would prefer not to drop a grand on Visual Studio).

[pierrejoye]

@michael-grunder We maintain almost all PECL extensions on Windows, you can trust our PRs :)

Also, as I already suggested (do not remember if it was you or someone else), we can you a MSDN subscription right away too :)

What would be the next steps to get trivial fixes in faster?

[pierrejoye]

ok

We cruelly need our PRs merged and a release to get redis client up and working.

What is the status?

[weltling]

@michael-grunder i've merged the @vostok4 branch with the current master, please see here

https://github.com/weltling/phpredis/compare/nicolasff:master...master

Here are also the snapshots from the latest master with the patch from @vostok4 applied over, --enable-redis=shared --enable-redis-session

http://windows.php.net/downloads/pecl/snaps/redis/2.2.5/

So at this point it will only cost applying the patch to the master, no conflicts and worky build. Please ping directly on user reports regarding windows support.

Thanks.

[michael-grunder]

Awesome, will pull your branch into a feature branch in this repo, build and run unit tests, and then merge it.

Thanks!
Mike

[michael-grunder]

Merged into master and develop guys. It compiles and runes fine for me (osx), Let me know if there are issues.

Cheers!
Mike

[Jan-E]

Compiled fine in all 16 flavours (5.3, 5.4, 5.5, 5.6.0RC2 / TS, NTS / x86, x64). I recompiled and uploaded all builds at http://www.apachelounge.com/viewtopic.php?t=6043

[weltling]

That's great, the next release should be fine with windows. Thanks for merging :)