Skip to content

[RFC] luci-nginx and luci-ssl-nginx: collect files from nginx and uwsgi-cgi packages#2981

Closed
peter-stadler wants to merge 2 commits into
openwrt:masterfrom
peter-stadler:luci-on-nginx-new
Closed

[RFC] luci-nginx and luci-ssl-nginx: collect files from nginx and uwsgi-cgi packages#2981
peter-stadler wants to merge 2 commits into
openwrt:masterfrom
peter-stadler:luci-on-nginx-new

Conversation

@peter-stadler
Copy link
Copy Markdown

@peter-stadler peter-stadler commented Aug 14, 2019

I am new to contributing and I am not sure if this change is a good idea. I tried to collect the files that are needed to run luci on nginx from the uwsgi-cgi and the nginx package and would move it here. See my other draft PRs in packages, too:
openwrt/packages#9736 ([RFC] package etesync and its dependencies and move luci on nginx)
uwsgi: add packages with modules and nginx: enable /etc/nginx/conf.d directory.
I would do this only if Ansuel approves it since it is his project.

See also the topic at:
https://forum.openwrt.org/t/package-etesync-and-its-dependencies/42269
I hope it is understandable what I try to do.

@peter-stadler peter-stadler reopened this Aug 14, 2019
@peter-stadler peter-stadler deleted the luci-on-nginx-new branch August 14, 2019 23:36
@peter-stadler peter-stadler restored the luci-on-nginx-new branch August 14, 2019 23:39
@peter-stadler
Copy link
Copy Markdown
Author

Sorry for the confusion by opening and closing this PR repeatedly. I wanted to change it to a draft PR.

@@ -0,0 +1,16 @@
#!/bin/sh
chmod +x /etc/init.d/luci
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? We could install this file with the right mods during installation in the Makefile

@@ -0,0 +1,37 @@
#!/bin/sh
chmod +x /etc/init.d/luci
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, how to do it. We need a $(INSTALL_BIN) … somewhere, right? But where, if we want to use the Package/$(PKG_NAME)/install from luci.mk?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats right. In LuCI you have to put the files into root with the right subdirectories and with the right mod flags and it will gets installed. See for example https://github.com/openwrt/luci/tree/master/applications/luci-app-adblock/root/etc/uci-defaults

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, now I have a bunch of commits with the wrong username/email. Will continue tomorrow and change the mods of the files, too.

@feckert
Copy link
Copy Markdown
Member

feckert commented Aug 27, 2019

Please add a prefix in front of your commits. Have a look at the commit history
luci-<package>: <what have you done>
I also did not find the package uwsgi-cgi-plugin you depend on.

@peter-stadler
Copy link
Copy Markdown
Author

Thank you :-) I will change the prefixes. Right now I am changing the PR for the packages repository …

@peter-stadler
Copy link
Copy Markdown
Author

The uwsgi-cgi-plugin would be in the uwsgi Makefile of my other commits in the packages repository. Right now I am redoing them with better commit messages, it will be in https://github.com/peter-stadler/packages/tree/uwsgi – right now I want to change username/email before the PR …

* depends on PRs for nginx (enable /etc/nginx/conf.d) and uwsgi (modular)
* makes nginx-mod-luci obsolete and uses uwsgi-cgi-plugin insted of uwsgi-cgi

Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at>
* depends on PRs for nginx (enable /etc/nginx/conf.d) and uwsgi (modular)
* makes nginx-mod-luci-ssl obsolete and uses uwsgi-cgi-plugin instead of uwsgi-cgi

Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at>
@peter-stadler
Copy link
Copy Markdown
Author

peter-stadler commented Sep 3, 2019

Sorry for the late answer (it took me some time to fix the CircleCI build for the etesync-server).

I made the init files executable, changed the commit messages and included luci-app-opkg as dependency (it is in the standard luci collection, too). Thank you @feckert.

Both commits depend on the uwsgi-cgi-plugin of the modular uwsgi and that the /etc/nginx/conf.d directory is enabled.

@peter-stadler peter-stadler changed the title [RFC] move the luci on nginx files from packages to this repository luci-nginx and luci-ssl-nginx: collect files from nginx and uwsgi-cgi packages [RFC] Sep 3, 2019
@peter-stadler peter-stadler changed the title luci-nginx and luci-ssl-nginx: collect files from nginx and uwsgi-cgi packages [RFC] [RFC] luci-nginx and luci-ssl-nginx: collect files from nginx and uwsgi-cgi packages Sep 3, 2019
@peter-stadler
Copy link
Copy Markdown
Author

@jow- do you think this is a good idea (regarding also the openwrt guidelines)?

@feckert feckert added the RFC issue to Request for Comments label Sep 22, 2019
@jow-
Copy link
Copy Markdown
Contributor

jow- commented Feb 23, 2020

Closing this as its outside of the scope of LuCI. The necessary work appears to have happened in the package repo now.

@jow- jow- closed this Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends on other pr pull request depends on another pull request RFC issue to Request for Comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants