port ngx_udp_connect() to fix compilation errors with nginx 1.12.0+#30
port ngx_udp_connect() to fix compilation errors with nginx 1.12.0+#30trungtm wants to merge 3 commits intozebrafishlabs:masterfrom
Conversation
| else | ||
| HTTP_MODULES="$HTTP_MODULES ngx_http_statsd_module" | ||
| NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_statsd.c" | ||
| CORE_LIBS="$CORE_LIBS -lssl" |
There was a problem hiding this comment.
Why do you still need -lssl? I believe that USE_OPENSSL=YES already accomplishes that. Or is there an issue with older nginx versions?
I had to remove -lssl in my fork anyway since we try to link with openssl statically, and this -lssl was screwing it up on CentOS/RedHat.
There was a problem hiding this comment.
You're right. I just converted the old config to the new one
USE_OPENSSL=YES is better option.
|
Just wondering what else is getting blocked from merging this PR? It'd be nice to have this fixed, so we can get it supported for new nginx versions! |
|
@levb could you tell us if this PR could be merged into the main branch? This fix would help us to get rid of |
|
@sane4ek-2 I am not working on nginx modules any longer, so do not have a strong opinion. However, I think my comment about |
|
@zacman85 hello. Could you merge this fix into the main branch? It looks like everything works fine. I could build this module with Nginx 1.20.2 without any problems. |
Since nginx 1.12.0, ngx_udp_connect() is never publicly available, and never have public declaration.
So I backport the function from nginx source code to fix compilation errors
Issue: #28
more info:
https://trac.nginx.org/nginx/ticket/1244