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

Enable --with-compat NGINX option in source-built images #114

Closed
robinkb opened this issue Jul 15, 2019 · 4 comments · Fixed by #115
Closed

Enable --with-compat NGINX option in source-built images #114

robinkb opened this issue Jul 15, 2019 · 4 comments · Fixed by #115

Comments

@robinkb
Copy link
Contributor

robinkb commented Jul 15, 2019

Enabling --with-compat makes it much easier for end-users to build and use dynamic modules for NGINX in custom images.

The way NGINX dynamic module compilation works is that a user has to download the source code for the matching NGINX version, and then run:

./configure --with-compat --add-dynamic-module=/module/src/path
make modules

This will compile the dynamic modules for the matching NGINX version. However, this approach only works when the main NGINX binary was compiled with --with-compat. If that is not the case, the ./configure line must specify the exact same compilation options that the main NGINX binary was compiled with. This makes the build process for dynamic modules much more complicated, especially for OpenResty where external libraries like OpenSSL and PCRE are downloaded outside of the build system.

What happens under the covers is that extra fields are added or extra memory reserved in internal data structures. This does result in a slight increase in memory usage.

See:
https://forum.nginx.org/read.php?29,270210,270213#msg-270213
https://mailman.nginx.org/pipermail/nginx-devel/2018-May/011119.html

The --with-compat option is enabled in the upstream NGINX packages and Docker image.
It was enabled in Ubuntu with Disco Dingo.

Unfortunately, it does not seem to be enabled in the upstream OpenResty binaries. Maybe an issue should be raised there as well?

@neomantra
Copy link
Member

Thanks for the issue and the PR. For the PR, can you update the README with the build-arg change? Also add yourself to AUTHORS if you want.

Before merging, I'd will ask the upstream maintainers about this. The Docker images don't have any testing infrastructure, so we wouldn't detect any regressions.

But, since the built-from-source images are intended to be customized and easily extended, this change make sense.

@neomantra
Copy link
Member

Oh, actually see this: openresty/openresty-packaging#33

agentzh seems to think it is OK. They mention issues with that and --with-dtrace-probes, but we don't include that option yet. I was actually adding it while working on #109 but that is not merged yet.

So make those README and AUTHORS changes and I'll merge it.

@robinkb
Copy link
Contributor Author

robinkb commented Jul 15, 2019

Thank you for the quick feedback!

I went and updated the README and AUTHORS documents.

neomantra pushed a commit that referenced this issue Jul 15, 2019
Enabling --with-compat makes it much easier for end-users to build
and use dynamic modules for NGINX in custom images.
@neomantra
Copy link
Member

Great, I just squashed those to one commit. I'll probably add one or two other things before tagging a new release. Thanks again for the contribution.

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.

2 participants