Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

convenient compatibility changes for nginx #15433

Closed
wants to merge 1 commit into from

6 participants

@mactijn

add '--with-debug' flag
load nginx in foreground so launchctl can keep it alive

@mactijn mactijn Update Library/Formula/nginx.rb
add '--with-debug' flag
load nginx in foreground so launchctl can keep it alive
6a730fb
@jacknagel
Owner

@loonypandora comments on the plist stuff?

@LoonyPandora

KeepAlive was explicitly turned off in d34b2b1 ~4 months ago. I'm not entirely sure it should be changed back. I often encountered people struggling to kill the nginx process because launchd kept it alive.

However, the deubg option would be very useful to have. I like that part.

@mactijn - Can you explain the problem you have with KeepAlive, and why it should be turned back on?

@mikemcquaid
Owner

If nginx is started with launchd, launchd should keep it alive. If you want to stop it you can stop it using launchd. That's normal behaviour for launchd processes, no?

@mactijn

Without KeepAlive + not running as daemon, nginx will not stop when using launchctl unload , which in my opinion is quite annoying as this punishes users who do it the "right" way. Also, other services like php53 using fpm, mysql etc do use it, so it would be inconsistent.

@mactijn

I think it would be a far better idea to educate users on this matter, for instance through "brew info nginx".

@LoonyPandora

I'm fine with it either way, I was mostly concerned that it was explicitly turned off ~4 months ago. This commit essentially reverts those changes without saying why d34b2b1 no longer applies. If @mactijn is confident that these changes don't cause launchd to continually try and launch nginx, then I see no problem.

@mactijn

@LoonyPandora I just checked to be sure, my logs are clean. I am unsure why @slarew writes that nginx daemonizes, while his commit essentially removes just that.

@slarew

I no longer use nginx through homebrew, so I cannot comment definitively on the current state of the KeepAlive issue. The correct behavior for a launchd controlled daemon is to let launchd control relaunching. So if nginx will play nice with launchd and its expectations, KeepAlive=true is probably the proper way to control relaunching nginx. However, I seem to recall having trouble getting launchd and nginx to fully cooperate.

@mactijn

launchctl unload [-w] <plist> stops nginx, launchctl load [-w] <plist> starts it. Using /usr/local/sbin/nginx -s <signal> is also fully functional, however signals stop and quit will effectively restart the nginx process, as launchctl will respawn it.

@adamv
Owner

We pulled an alternate request for the debug switch. So at the very least this request will need to be rebased on master.

Any final word on what the correct startup behavior is here?

@mikemcquaid
Owner

Pretty sure this works properly now. Shout if it doesn't.

@mikemcquaid mikemcquaid closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 12, 2012
  1. @mactijn

    Update Library/Formula/nginx.rb

    mactijn authored
    add '--with-debug' flag
    load nginx in foreground so launchctl can keep it alive
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 1 deletion.
  1. +4 −1 Library/Formula/nginx.rb
View
5 Library/Formula/nginx.rb
@@ -16,6 +16,7 @@ class Nginx < Formula
option 'with-passenger', 'Compile with support for Phusion Passenger module'
option 'with-webdav', 'Compile with support for WebDAV module'
+ option 'with-debug', 'Compile with debug support'
skip_clean 'logs'
@@ -53,6 +54,7 @@ def install
"--http-uwsgi-temp-path=#{var}/run/nginx/uwsgi_temp",
"--http-scgi-temp-path=#{var}/run/nginx/scgi_temp"]
+ args << "--with-debug" if build.include? 'with-debug'
args << passenger_config_args if build.include? 'with-passenger'
args << "--with-http_dav_module" if build.include? 'with-webdav'
@@ -92,12 +94,13 @@ def startup_plist
<key>RunAtLoad</key>
<true/>
<key>KeepAlive</key>
- <false/>
+ <true/>
<key>UserName</key>
<string>#{`whoami`.chomp}</string>
<key>ProgramArguments</key>
<array>
<string>#{HOMEBREW_PREFIX}/sbin/nginx</string>
+ <string>-g 'daemon off;'</string>
</array>
<key>WorkingDirectory</key>
<string>#{HOMEBREW_PREFIX}</string>
Something went wrong with that request. Please try again.