The Apache service allows different restart commands to be used, whereas the nginx one does not. It should. I'll do the code change on Friday at the sprint.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

realityloop’s picture

Here is the command for OS X where nginx was installed using homebrew:
/usr/local/sbin/nginx -s reload

realityloop’s picture

Hi Steve,

I've written the code for this, which branch should I create the patch against?

Steven Jones’s picture

Against the 7.x-2.x would be best, and we'll just cherry-pick it back into the 6.x-1.x branch anyway.

Steven Jones’s picture

Assigned: Steven Jones » Unassigned
realityloop’s picture

Version: 6.x-1.2 » 7.x-2.x-dev
FileSize
989 bytes

Patch against 7.x attached

realityloop’s picture

Status: Active » Needs review

forgot to change status

realityloop’s picture

Updated quotes on anarcat's suggestion

omega8cc’s picture

Category: bug » feature
Status: Needs review » Needs work

This patch breaks my install, debugging now.

omega8cc’s picture

Status: Needs work » Needs review

This patch fixes the issue for me, because it lists the default command as first (default) and not last, which matches the c.a.o how-to for sudoers entry.

http://drupalcode.org/sandbox/omega8cc/1111100.git/commit/5aa3e49

[EDIT] The problem with original patch was because it matched also non-init.d versions/locations, which didn't match default sudoers entry. This is also a docs candidate.

anarcat’s picture

Status: Needs review » Patch (to be ported)

pushed to 2.x, waiting for jenkins.

anarcat’s picture

Status: Patch (to be ported) » Fixed

merged, part of 1.3!

realityloop’s picture

Status: Fixed » Needs review
FileSize
691 bytes

Unfortunately the merged patch doesn't work with non unix systems, this updated patch resolves the issue

omega8cc’s picture

Status: Needs review » Postponed (maintainer needs more info)

I'm not sure I understand this patch.

Could you provide more detailed feedback/debugging info?

If there is no /etc/init.d/nginx on your server, it should just work and the patch doesn't make any difference, imo.

What is the path to the Nginx binary on your system and what is the entry in your /etc/sudoers?

Deciphered’s picture

@omega8cc,

The patch is for installing Aegir on OS X, with Nginx installed via Homebrew. There is no /etc/init.d on a mac.

omega8cc’s picture

@Deciphered, I know, but it doesn't answer my questions. The way the patch is trying to fix the issue doesn't look like a fix, but rather workaround, so we need to learn what exactly the problem is and fix it properly to avoid introducing issues for other systems (as we did for OS X).

Steven Jones’s picture

@realityloop/Deciphered the patch in #12 doesn't seem to change all that much, does that actually resolve the issue for you? If it does, do you know why?

I agree with omega8cc that we should avoid hacks for specific systems if we can, but it seems that here we can with very little effort make things work with osx, in a nice general way.

omega8cc’s picture

Status: Postponed (maintainer needs more info) » Needs review

This (untested) patch should fix the issue: http://drupalcode.org/sandbox/omega8cc/1111100.git/commit/a208ed4

omega8cc’s picture

Deciphered’s picture

Status: Needs review » Postponed (maintainer needs more info)

I agree, the patch doesn't seem to do anything at all except change the namespace. I'll put it down to Realityloop's DrupalCon Flu ;)

Will test out those other patches shortly, thanks omega8cc.

Deciphered’s picture

Patch at #17 seems to do the job.

For sanity sake/smaller code, and maybe due to laziness as I had already applied this seemingly do-nothing patch, I changed the condition in patch #17 to be ($test == $default).

Didn't apply #18 because the original didn't seem to be present to be patched.

 

Unfortunately while the primary issue of Nginx not being reloaded has been resolved by this issue, I am still having problems with other sections, but won't post those issues here as I'd like to at least get this one issue resolved.

Deciphered’s picture

Status: Postponed (maintainer needs more info) » Needs review

Oops, didn't mean to change status.

Steven Jones’s picture

Version: 7.x-2.x-dev » 6.x-1.x-dev
Status: Needs review » Needs work

I've pushed #17 into 6.x-1.x, but not #18, as that seems to depend on other changes not in 6.x-1.x?

omega8cc’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev

Right, #18 is safe to skip for now, as we have already rewritten that code in the meantime anyway, while #17 should work fine.

Now I need to submit a patch for 2.x, I guess, so I'm changing the version.

omega8cc’s picture

Status: Needs work » Patch (to be ported)

Patch to be ported.

omega8cc’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

anarcat’s picture

Version: 6.x-2.x-dev » 6.x-1.x-dev

  • Commit 63e05e9 on 7.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x authored by omega8cc, committed by anarcat:
    Issue #1259098 - better support for different Nginx reload commands.
    
    
  • Commit 3256443 on 6.x-1.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x authored by omega8cc, committed by anarcat:
    Issue #1259098 - better support for different Nginx reload commands.
    
    
  • Commit 97d727a on 6.x-1.x, dev-drupal-8, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x authored by omega8cc, committed by Steven Jones:
    Issue #1259098 - is_executable() does not accept arguments so -s should...