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

www-servers/nginx: Add njs module (javascript) #3200

Closed
wants to merge 2 commits into from
Closed

www-servers/nginx: Add njs module (javascript) #3200

wants to merge 2 commits into from

Conversation

burik666
Copy link

nginScript is a subset of the JavaScript language that allows implementing location and variable handlers in http and stream.

http://nginx.org/en/docs/njs_about.html
http://nginx.org/en/docs/http/ngx_http_js_module.html
http://nginx.org/en/docs/stream/ngx_stream_js_module.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds, profiles
Packages affected: www-servers/nginx

www-servers/nginx: @Whissi, @dev-zero, @jbergstroem

@gentoo-repo-qa-bot gentoo-repo-qa-bot added the assigned PR successfully assigned to the package maintainer(s). label Dec 21, 2016
@jbergstroem
Copy link
Contributor

@burik666 is this really enough? Don't we need a compile flag?

@burik666
Copy link
Author

@jbergstroem yes.

From njs documentation:

Then the modules should be compiled using the --add_module configuration parameter:
./configure --add-module=path-to-njs/nginx

@jbergstroem
Copy link
Contributor

@burik666 my fault; i didn't notice that github hid the diff for the ebuild. Sorry!

@gentoo-repo-qa-bot
Copy link
Collaborator

😞 The QA check for this pull request has found the following issues:

Issues inherited from Gentoo (may be modified by PR):
https://qa-reports.gentoo.org/output/gentoo-ci/31052dc42/output.html#net-analyzer/wireshark

@Whissi
Copy link
Contributor

Whissi commented Dec 21, 2016

@burik666: Thank you for your contribution but we won't add any additional modules: It is too much work for us to keep up with all the modules no developer uses and therefore nobody really cares. For example, if we add this module but it won't work in future, should we hold back a new nginx version just because of this extension (yes, I noticed that this module is from nginx developers but there's a reason why it isn't included per default)? So like we have already denied requests for other modules we have to deny this one as well. I hope you understand.

But I got an idea from another dev how we could solve this problem for extensions like yours which can be built as dynamic module and don't require a patched core nor depend on ngx_module_order. I hope to finish my PoC in the holidays.

@Whissi Whissi added the do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. label Dec 21, 2016
@jbergstroem
Copy link
Contributor

jbergstroem commented Dec 21, 2016

@Whissi: isn't njs core though?

edit: moving to modules is something i'd really like to do as well. we should look at freebsd's portfile.

@Whissi
Copy link
Contributor

Whissi commented Dec 21, 2016

Like said there's a reason why upstream decided not to merge njs with nginx core (yet?).

If the current experiment will fail (we don't follow FreeBSD because FreeBSD is maintaining everything in their main nginx Makefile... currently working on something like the mod_apache thing with a nginx eclass involved, looks promising at the moment.. thought we had more modules requiring a patched core but this isn't the case) and we don't have solution until 2017-01-31 we will re-consider the situation and add a bunch of long requested modules (including this one). But I am very positive that we will have a working solution in near future to maintain extension in their own ebuild.

@mgorny
Copy link
Member

mgorny commented May 8, 2017

@Whissi, ping. It seems that the deadline has passed and nobody replied here. What's the status? Do we rebase it or reject it?

@Whissi
Copy link
Contributor

Whissi commented May 9, 2017

I have the changes in my overlay. I hope to get this moved into the main repository this month.
I'll close this PR now because it is foreseeable that we are not going to merge this into the current ebuild.

@Whissi Whissi closed this May 9, 2017
gentoo-bot pushed a commit that referenced this pull request Feb 16, 2018
Ebuild changes:
===============
- LDAP auth module bumped to commit 42d195d7a7575ebab1c369ad3fc5d78dc2c2669c
  to add OpenSSL 1.1.x support and other bugfixes.

- HTTP upstream check module bumped to commit 9aecf15ec379fe98f62355c57b60c0bc83296f04
  to fix possible segfault when reloading configuration.

- Virtual host traffic status module added.

- nginScript module added. [PR 3200]

- Brotli module added. [Bug 628898]

See: #3200
Closes: https://bugs.gentoo.org/628898
Package-Manager: Portage-2.3.24, Repoman-2.3.6
NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Feb 17, 2018
Ebuild changes:
===============
- LDAP auth module bumped to commit 42d195d7a7575ebab1c369ad3fc5d78dc2c2669c
  to add OpenSSL 1.1.x support and other bugfixes.

- HTTP upstream check module bumped to commit 9aecf15ec379fe98f62355c57b60c0bc83296f04
  to fix possible segfault when reloading configuration.

- Virtual host traffic status module added.

- nginScript module added. [PR 3200]

- Brotli module added. [Bug 628898]

See: gentoo#3200
Closes: https://bugs.gentoo.org/628898
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI.
Projects
None yet
5 participants