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 --with-compat by default #33

Open
ruslantalpa opened this issue Jun 18, 2019 · 6 comments
Open

add --with-compat by default #33

ruslantalpa opened this issue Jun 18, 2019 · 6 comments

Comments

@ruslantalpa
Copy link

without this flag, building dynamic modules, targeting a specific openresty package release is a pain. One has to use -V option to figure out the build arguments used when building the official release and jsut a copy/pasted does not work since paths are not the same

Is this flag missing on purpose?

with this flag one basically needs just ./configure --with-compat --add-dynamic-module=..

@ruslantalpa
Copy link
Author

also when building modules, it seems --with-dtrace-probes flag breaks the process.
without the flag, at the end there is a .so file, after adding the flag, the file is not longer generated, but the build finishes successfully (i don't see any errors)

I don't know if all modules are affected or just the one i am building (https://github.com/ruslantalpa/lua-upstream-cache-nginx-module)

@agentzh
Copy link
Member

agentzh commented Jun 19, 2019

We could add --with-compat. That seems reasonable though at a small price. Pull requests welcome :)

Not sure about the --with-dtrace-probes thing, maybe our nginx dtrace patch needs some love for dynamic module building. See here:

https://github.com/openresty/openresty/blob/master/patches/nginx-1.15.8-dtrace.patch

This patch was created before nginx has got dynamic module support. It would be great if you can help dig this up. Pull requests welcome :)

Thanks!

@ruslantalpa
Copy link
Author

a quick followup for anyone reading this, i was able to build the module using this

./configure \
        --with-pcre-jit \
		--without-http_rds_json_module \
		--without-http_rds_csv_module \
		--without-lua_rds_parser \
		--with-stream \
		--with-stream_ssl_module \
		--with-stream_ssl_preread_module \
		--with-http_v2_module \
		--without-mail_pop3_module \
		--without-mail_imap_module \
		--without-mail_smtp_module \
		--with-http_stub_status_module \
		--with-http_realip_module \
		--with-http_addition_module \
		--with-http_auth_request_module \
		--with-http_secure_link_module \
		--with-http_random_index_module \
		--with-http_gzip_static_module \
		--with-http_sub_module \
		--with-http_dav_module \
		--with-http_flv_module \
		--with-http_mp4_module \
		--with-http_gunzip_module \
		--with-threads \
		--with-luajit-xcflags='-DLUAJIT_NUMMODE=2 -DLUAJIT_ENABLE_LUA52COMPAT' \
		--with-pcre=/tmp/pcre-${RESTY_PCRE_VERSION} \
		--with-openssl=/tmp/openssl-${RESTY_OPENSSL_VERSION} \
		--with-zlib=/tmp/zlib-${RESTY_ZLIB_VER} \
		--add-dynamic-module=/tmp/lua-upstream-cache-nginx-module

this was for openresty/openresty:1.15.8.1-2-stretch-fat

and the vars were

ENV NGINX_VERSION="1.15.8"
############ This section is copied from the parent image ##################
# Docker Build Arguments
ARG RESTY_VERSION="1.15.8.1"
# ARG RESTY_LUAROCKS_VERSION="2.4.3"
ARG RESTY_OPENSSL_VERSION="1.1.0j"
ARG RESTY_PCRE_VERSION="8.42"
ARG RESTY_ZLIB_VER="1.2.11"

it seems it's safe to eliminate --with-dtrace-probes from config params, the module will still be compatible

@ruslantalpa
Copy link
Author

the dtrace part will probably be hard for me to figure out but the --with-compat part I will make a PR
Thank you. All I can say about dtrace patch is that it's safe to eliminate when building modules (i'll double check that today)

@ruslantalpa
Copy link
Author

about the dtrace, if you can point me in the right direction about that patch (list of files from there that could cause this, just to narrow it down a bit), i could look into it.
thank you

@agentzh
Copy link
Member

agentzh commented Jun 19, 2019

about the dtrace, if you can point me in the right direction about that patch (list of files from there that could cause this, just to narrow it down a bit), i could look into it.

Thanks. It might be our changes in the files under the auto/ directory, i.e., the nginx build system.

ruslantalpa added a commit to ruslantalpa/openresty-packaging that referenced this issue Nov 15, 2019
Per discussion in openresty#33 this flag is is needed to be able to easely build
aditional dynamic nginx modules
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

No branches or pull requests

2 participants