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

v5.0.0 PECL build fails on Windows #440

Closed
aand18 opened this issue Jan 22, 2021 · 10 comments · Fixed by #450
Closed

v5.0.0 PECL build fails on Windows #440

aand18 opened this issue Jan 22, 2021 · 10 comments · Fixed by #450

Comments

@aand18
Copy link

aand18 commented Jan 22, 2021

  • PHP version: 8.0.1
  • librdkafka version: 1.5.3
  • php-rdkafka version: 5.0.0
  • kafka version:

PECL build fails for latest v5.0.0 DLL build.
See logs at https://windows.php.net/downloads/pecl/releases/rdkafka/5.0.0/logs/php_rdkafka-5.0.0-8.0-nts-vs16-x64-logs.zip

ext\rdkafka\kafka_consumer.c(789): warning C4133: 'function': incompatible types - from 'long *' to 'int64_t *'
ext\rdkafka\kafka_consumer.c(789): warning C4133: 'function': incompatible types - from 'long *' to 'int64_t *'
metadata_topic.c
queue.c
rdkafka.c
topic.c
topic_partition.c
ext\rdkafka\rdkafka.c(639): warning C4133: 'function': incompatible types - from 'long *' to 'int64_t *'
ext\rdkafka\rdkafka.c(639): warning C4133: 'function': incompatible types - from 'long *' to 'int64_t *'
	rc /nologo  /I . /I main /I Zend /I TSRM /I ext /n /fo C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.dll.res /d FILE_DESCRIPTION="\"rdkafka\"" /d FILE_NAME="\"php_rdkafka.dll\"" /d URL="\"http://www.php.net\"" /d INTERNAL_NAME="\"RDKAFKA extension\"" /d THANKS_GUYS="\"Thanks to Arnaud Le Blanc\"" win32\build\template.rc
	"link.exe" @"C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\resp\RDKAFKA_GLOBAL_OBJS.txt" C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php8.lib librdkafka.lib kernel32.lib ole32.lib user32.lib advapi32.lib shell32.lib ws2_32.lib Dnsapi.lib psapi.lib bcrypt.lib argon2_a.lib C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.dll.res /out:C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.dll /dll /nologo /GUARD:CF /incremental:no /debug /opt:ref,icf /libpath:"C:\php-snap-build\php-src\php80\x64\deps\lib" /libpath:"C:\php-snap-build\dep-aux\vs16\x64\librdkafka\lib" /NXCOMPAT /DYNAMICBASE /libpath:"..\deps\lib" 
   Creating library C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.lib and object C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.exp
rdkafka.obj : error LNK2019: unresolved external symbol create_kafka_error referenced in function kafka_log_syslog_print
rdkafka.obj : error LNK2019: unresolved external symbol kafka_error_minit referenced in function register_err_constants
C:\php-snap-build\obj\8.0-nts-windows-vs16-x64\Release\php_rdkafka.dll : fatal error LNK1120: 2 unresolved externals
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.28.29333\bin\HostX64\x64\link.exe"' : return code '0x460'
Stop.

I've replicated the error locally with librdkafka v1.5.3, v1.4.4 , v1.3.0, v0.11.6 .

@nick-zh
Copy link
Collaborator

nick-zh commented Mar 8, 2021

I investigated a bit on the weekend, but the error message is strange to me as i am uncertain how these functions are referenced in kafka_sys_log_print and register_err_constants.
I would need to build on windows to maybe gain more insight, but i do not have the proper setup for this.
As a side note librdkafka:0.11.6 is not supported anymore for the windows build.
@cmb69 was nice enough to provide the CI tests for the windows build, so i am kind of confused how this can happen atm, maybe he has some insight?

@cmb69
Copy link
Contributor

cmb69 commented Mar 8, 2021

There is a problem with the build system. phpize builds are working fine, but in-tree builds (and these are done for PECL) don't. For some reason, AC_DEFINE doesn't work. I need to investigate closer (planned for today or tomorrow), but it seems a viable workaround would be to set CFLAGS with /D ... instead of using AC_DEFINE on Windows.

@nick-zh
Copy link
Collaborator

nick-zh commented Mar 8, 2021

thx a lot @cmb69 just let me know what i can do and if we should go with CFLAGS once you have finished your investigation and i will try to adapt it 🙇

@cmb69
Copy link
Contributor

cmb69 commented Mar 9, 2021

Okay, I checked that more thoroughly, and this part of the Windows build system is quite messy. While on other systems the configuration is written to main/php_config.h, on Windows it's written to main/config.w32.h. However, phpize builds also generate main/config.pickle.h and automatically include this file. This is why on Windows phpize builds currently work as desired, but in-tree builds don't.

There is more to this, though. php.h includes php_compat.h which in turn includes php_config.h and config.w32.h, respectively. However, kafka_error_exeception.c includes php.h only if HAS_RD_KAFKA_TRANSACTIONS is defined, while rdkafka.c includes php.h unconditionally, so HAS_RD_KAFKA_TRANSACTIONS is defined for that compilation unit, and as such create_kafka_error() is used, although it effectively never has been defined.

This isn't a Windows specific issue; if I build ext/rdkafka with librdkafka 1.4.2 on Ubuntu 20.10, I get:

/home/vagrant/php-src/ext/rdkafka/rdkafka.c:806:5: warning: implicit declaration of function ‘create_kafka_error’ [-Wimplicit-function-declaration]

It seems to me that a proper fix would be to swap these two lines.

@cmb69
Copy link
Contributor

cmb69 commented Mar 9, 2021

Oh, I forgot to mention that there is actually an issue regarding the PECL builds for Windows: while version 4.x is supposed to be built with librdkafka 1.2.1, 5.x is supposed to use librdkafka 1.5.3. Unfortunately, the automated build system doesn't support different library versions for different extension versions, so I need to swap the libraries for version 4/5 builds manually. Not a real problem, but if a new rdkafka version 5.x is released, the build can initially fail, because librdkafka defaults to 1.2.1. I can trigger a second build in that case.

@nick-zh
Copy link
Collaborator

nick-zh commented Mar 10, 2021

@cmb69 thx a lot for your effort and the elaborate explanation , i will it as soon as i find time. Oh i wasn't aware of that regarding libraries, i will do more investigation to see if it would be feasible to go with the same, manual switchting seems like a hassle 😅

@cmb69
Copy link
Contributor

cmb69 commented Mar 10, 2021

It is likely feasible to build rdkafka 4.x with librdkafka 1.5.3, but for stability reasons, it still may not be desired.

@aand18
Copy link
Author

aand18 commented Mar 29, 2021

Thanks to the tip from @cmb69 about swapping those two lines, after some tinkering I've managed to build the DLL.

I'm attaching my build below for anybody on Windows wanting to migrate to PHP v8.
Hope it helps someone until the official PECL build comes out.

You'll need:

  • PHP v8.0.1 x64 NTS (edit: added v8.0.3 build)
  • librdkafka.dll (get v1.5.3 from NuGet , extract all DLLs from runtimes\win-x64\native into the php.exe path)
  • extract rd_kafka.dll into /ext folder and enable extension in php.ini

After that you should see:

> php.exe -i | findstr "kafka"
rdkafka
rdkafka support => enabled
librdkafka version (runtime) => 1.5.3
librdkafka version (build) => 1.4.4.255

php_rdkafka.dll_v5.0.0_x64_php8.0.1_NTS.zip
php_rdkafka.dll_v5.0.0_x64_php8.0.3_NTS.zip

@nick-zh
Copy link
Collaborator

nick-zh commented Mar 29, 2021

Thx guys, sry this slipped my mind. I created a PR for it

@nick-zh nick-zh linked a pull request Mar 29, 2021 that will close this issue
@nick-zh
Copy link
Collaborator

nick-zh commented Mar 29, 2021

@arnaud-lb i think it could be beneficial to build the windows dll also with an 1.5.x or even 1.6.x version of librdkafka, which adds a lot of fixes and would enable some of the newer features for windows users. What do you think?
If possible, i would want to avoid manual fixes when we are building the ext on pecl.

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 a pull request may close this issue.

3 participants