Nginx capabilities detection is not reliable

I'm aware of this, but I stopped to work on a fix because of this issue #962188: Nginx version check works for server.tpl.php but doesn't work for/in vhost.tpl.php since even if all tests results are correct and stored in server_master.alias.drushrc.php as bellow:

  'nginx_has_gzip' => 1,
  'nginx_has_upload_progress' => 1,

you still can't use them in vhost.tpl.php for some unknown reason (at least I have no idea yet why it doesn't work) so it will use nginx_simple_include.conf, no matter what. It will work only in server.tpl.php, which is not enough.

Correct me if I'm wrong.

I believe it is fine now, after you fixed the issue #962188: Nginx version check works for server.tpl.php but doesn't work for/in vhost.tpl.php.

Thanks again.

+++ http/nginx/nginx_service.incundefined
@@ -18,13 +18,28 @@ class provisionService_http_nginx extends provisionService_http_public {
+      return;

do we invariably want to fail if we don't find it here? and is "return" a proper way to fail? I feel an error should be raised with drush_set_error() here....

+++ http/nginx/nginx_service.incundefined
@@ -18,13 +18,28 @@ class provisionService_http_nginx extends provisionService_http_public {
+    }
+    ¶
     // Check if some nginx features are supported and save them for later.

again extra whitespace here, and below...

+++ http/nginx/nginx_service.incundefined
@@ -18,13 +18,28 @@ class provisionService_http_nginx extends provisionService_http_public {
-    $this->server->nginx_has_upload_progress = preg_match("/(nginx-upload-progress-module)/", implode('', drush_shell_exec_output()), $match);
     $this->server->nginx_has_new_version = preg_match("/(Barracuda\/0\.9\.)/", implode('', drush_shell_exec_output()), $match);
     $this->server->provision_db_cloaking = FALSE;
     $this->server->nginx_web_server = 1;
+    ¶
+    // Check upload progress capability. Because configure parameters may vary,
+    // test sample config.
+    $this->server->shell_exec($path . ' -t -c ' . dirname(__FILE__) . '/upload_progress_test.conf');
+    $this->server->nginx_has_upload_progress = preg_match("/upload_progress_test\.conf syntax is ok/", implode('', drush_shell_exec_output()), $match);

i don't understand why this is better than checking the actual output here... We're assuming the configuration for the upload_progress module is going to be a certain name, why not detect the right module directly?

is there *anything* in your output that says the module is configured?

Powered by Dreditor.

That is (almost) correct, because in fact current regex checks for module *name* (and not any arbitral path), which is nginx-upload-progress-module, unless you renamed it - see the original here: https://github.com/masterzen/nginx-upload-progress-module

But I agree that it is better to use the config check trick instead of relying on module name regex, because it is too easy to have the module name/directory renamed on git clone, etc.

We no longer use any extra file, because it caused already too many issues for too many people, on upgrades, for remote heads etc. We check the output for "upload" keyword, which should be enough unique to make sure it is an upload progress module compiled in. If you are using some unrelated name for module with official name "nginx-upload-progress-module", then it is a FAQ/docs candidate and we should be able to assume that "upload" keyword is present, like we assume that you didn't rename "nginx" to something else.

Changes committed to 6.x-1.x and 6.x-2.x:

http://drupalcode.org/project/provision.git/commit/009294bca51396c890423...
http://drupalcode.org/project/provision.git/commit/569d5489b7e40bb55981d...