Skip to content

Conversation

@weltling
Copy link
Contributor

@weltling weltling commented Nov 1, 2018

The rework adapts the lib names expectation to the usual naming scheme in PHP. The dependency libs header need a light patching, the current packages are accessible here https://windows.php.net/downloads/pecl/deps/. The latest PECL release build was redone using this config.w32 patch and the linked deps.

Thanks.

@rtheunissen
Copy link
Contributor

Thank you for your help @weltling. 🙏 Do you know why the appveyor build is failing?

Checking for mpdecimal.h ...  deps\include
Checking for library libmpdec.lib ... deps\lib\libmpdec.lib
...
php_decimal.obj : error LNK2019: unresolved external symbol __imp_mpd_init referenced in function zm_startup_decimal

@weltling
Copy link
Contributor Author

weltling commented Nov 2, 2018

@rtheunissen yes, the current Appveyor script uses a static dependency build, https://github.com/php-decimal/extension/blob/master/appveyor.yml#L67. The import library is named libmpdec-2.4.2.dll.lib in the libmpdec's Makefile. The usual scheme used in PHP and implemented in this patch, is that a static library is called somelib_a.lib, and an import library somelib.lib. Also, for extensions built shared also shared dependency builds are preferred (as it's done in this patch and would be delivered in pecl build). That can be changed of course, depends on what makes sense for you.

I've pushed a relevant fix to appveyor script, which will suffice for now. There might be other improvements to AppVeyor, if needed.

Thanks.

@weltling
Copy link
Contributor Author

weltling commented Nov 2, 2018

Ah, also why it needs some extra code in this case - libmpdec requires an extra build flag, when header is used for builds linking with DLL. Normally one would check that either static or dynamic dep is available.

Thanks.

@rtheunissen rtheunissen merged commit 1be5756 into php-decimal:master Nov 2, 2018
@rtheunissen
Copy link
Contributor

That can be changed of course, depends on what makes sense for you.

I have a lot to learn here, thank you for helping. 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants