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

Add config.w32 for building on Windows #115

Merged
merged 4 commits into from
Jan 4, 2021
Merged

Add config.w32 for building on Windows #115

merged 4 commits into from
Jan 4, 2021

Conversation

Jan-E
Copy link
Contributor

@Jan-E Jan-E commented Dec 25, 2020

This is a minimal config.w32 for building php_maxminddb.dll on Windows.
Tests with a GeoIP2-City.mmdb in \usr\local\share\GeoIP

=====================================================================
Number of tests :    3                 3
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :    3 (100.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :    0 seconds
=====================================================================

It works for PHP 7.0 - 7.4 and PHP 8.0, provided the libmaxminddb dependencies are in deps/lib and deps/include. Please review and merge.

See a phpinfo dump:
https://phpdev.toolsforresearch.com/php-8.0.1RC1-nts-Win32-vs16-x64.htm

And the complete distro
https://phpdev.toolsforresearch.com/php-8.0.1RC1-nts-Win32-vs16-x64.zip

Copy link
Member

@oschwald oschwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I had a couple of small comments. Also please add the file to package.xml so that it ends up in the PECL package.

ext/config.w32 Outdated Show resolved Hide resolved
ext/config.w32 Outdated Show resolved Hide resolved
@Jan-E
Copy link
Contributor Author

Jan-E commented Dec 26, 2020

There is another thing needed to make it compile under PHP 7.4 and PHP 8.0. php/php-src@86aac3e: uint does not exist anymore in the Windows environment. Usually it is replaced by uint32_t. e.g. php/php-src@afb6ca2

Shall I make that change in the same PR?

@Jan-E
Copy link
Contributor Author

Jan-E commented Dec 26, 2020

@cmb69 The dependencies are in https://github.com/maxmind/libmaxminddb
Building of libmaxminddb.lib and maxminddb.pdb is something like

md vc15.x64 
cd vc15.x64
cmake -G "Visual Studio 15 2017 Win64" ..
devenv maxminddb.sln /Clean "RelWithDebInfo|x64"
devenv maxminddb.sln /Build "RelWithDebInfo|x64"
copy maxminddb.dir\RelWithDebInfo\*.pdb RelWithDebInfo /y
copy RelWithDebInfo\* \php-sdk\win64build.vc15\lib\ /y
copy ..\include\*.h \php-sdk\win64build.vc15\include /y

The last two lines are specific for my build environment, but you'll get the grip.
I have done something like this for VC14 (PHP 7.0/7.1), VC15 (PHP 7.2, 7.3. 7.4) and VS16 (PHP 8.0), all x64/Win64 and x86/Win32. Can we go for a release of the Windows DLL's on https://pecl.php.net/package/maxminddb ?

@Jan-E
Copy link
Contributor Author

Jan-E commented Dec 26, 2020

There is another thing needed to make it compile under PHP 7.4 and PHP 8.0. php/php-src@86aac3e: uint does not exist anymore in the Windows environment. Usually it is replaced by uint32_t. e.g. php/php-src@afb6ca2

Changed in https://github.com/Jan-E/MaxMind-DB-Reader-php/commit/eb2e004db5d84cd300e2583b722f5d1f5b94c905

@cmb69
Copy link

cmb69 commented Dec 28, 2020

@Jan-E, I'm somewhat reluctant to deploy an unreleased master build of libmaxminddb. @oschwald, are you planning to release a new libmaxminddb release with CMake support?

@Jan-E
Copy link
Contributor Author

Jan-E commented Dec 28, 2020

The best way forward would be to make a new release of libmaxminddb with the commit that prevents getopt: maxmind/libmaxminddb@73f1236

And then make a new release of the PHP maxminddb extension with support for Windows.

@oschwald oschwald merged commit fbc1257 into maxmind:master Jan 4, 2021
@oschwald
Copy link
Member

oschwald commented Jan 4, 2021

Yes, I think it would make sense to do a release of libmaxminddb with the CMake support.

@oschwald
Copy link
Member

oschwald commented Jan 7, 2021

Releases of both this and libmaxminddb have been done.

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

Successfully merging this pull request may close these issues.

3 participants