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

Feature/add njs module #79

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

akselikap
Copy link

@akselikap akselikap commented May 27, 2021

This PR adds njs module to nginx which brings a lot of flexibility with it. The only downside I can think of is that this increases the size of the compiled nginx binary by almost a 100% from 5.5M to 9.1M for Heroku-18 which while considerable is not gigantic. I'm not sure if there is any security indications if someone doesn't use this module at all.

If this PR is rejected as not wanted I think this buildpack should offer support for dynamic modules in some way. Now the only way for me to use njs with this buildpack is to fork this repository and add whatever modules I want. Dynamic modules could be added through some sort of a configuration.

Side note: I built the .tgz -files using the make command but after every build the file seems to be different than it previously was. Is this intended and if so why does it happen?

@beanieboi
Copy link
Member

Hey,

thanks for the PR. we do not intend to add more modules at this point. Please use a fork of the buildpack for now.

Thanks
Ben

@akselikap
Copy link
Author

Thanks for the quick reply!

thanks for the PR. we do not intend to add more modules at this point. Please use a fork of the buildpack for now.

Is there plans to support adding dynamic modules through configuration? Just trying to interpret the meaning of "for now".

@beanieboi
Copy link
Member

I haven't looked at dynamic modules yet. I'm wondering how it could work. you would have to bring the module in your app. you also need to make sure that you compile it for the correct stack. it also means that you probably need to update your modules when you are attempting to upgrade the stack. I need to think more about the consequences of that.

@akselikap
Copy link
Author

Dynamic modules can be installed through apt and they have a package repository. The user would have to give the correct repository and module through configuration and then it could be downloaded. Although the modules are linked to the nginx binary installed through apt which might make my idea completely non functional.

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