{"thread":{"id":"21679","subject":"[PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location","startedAt":"2009-11-20T01:22:02Z","lastAt":"2010-01-02T22:04:25Z","messageCount":10,"participants":["Tarmigan Casebolt","Jay Soffian","Tarmigan","Junio C Hamano","Clemens Buchacher"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"127948","messageId":"1258680123-28684-1-git-send-email-tarmigan+git@gmail.com","threadId":"21679","inReplyTo":null,"subject":"[PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location","fromName":"Tarmigan Casebolt","fromEmail":"tarmigan+git@gmail.com","sentAt":"2009-11-20T01:22:02Z","receivedAt":"2009-11-20T01:22:02Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"On my machine with CentOS, httpd is located at /usr/sbin/httpd, and\nthe modules are located at /usr/lib64/httpd/modules.  To enable easy\ntesting of httpd, we would like those locations to be detected\nautomatically.\n\nuname might not be the best way to determine the default location for\nhttpd since different Linux distributions apparently put httpd in\ndifferent places, so we test a couple different locations for httpd,\nand use the first one that we come across.  We do the same for the\nmodules directory.\n\nSigned-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>\n---\n\nWould any machines have httpd or the modules/ directory in several of\nthese locations?\n\nAlso I don't really know shell scripting, so while this Works For Me,\nit may be completely wrong.\n\n t/lib-httpd.sh |   19 +++++++++++++------\n 1 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6765b08..6b86353 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -12,16 +12,23 @@ fi\n \n HTTPD_PARA=\"\"\n \n+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+do\n+\ttest -x \"$DEFAULT_HTTPD_PATH\" && break\n+done\n+\n+for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n+                                 '/usr/lib/apache2/modules' \\\n+                                 '/usr/lib64/httpd/modules' \\\n+                                 '/usr/lib/httpd/modules'\n+do\n+\ttest -d \"$DEFAULT_HTTPD_MODULE_PATH\" && break\n+done\n+\n case $(uname) in\n \tDarwin)\n-\t\tDEFAULT_HTTPD_PATH='/usr/sbin/httpd'\n-\t\tDEFAULT_HTTPD_MODULE_PATH='/usr/libexec/apache2'\n \t\tHTTPD_PARA=\"$HTTPD_PARA -DDarwin\"\n \t;;\n-\t*)\n-\t\tDEFAULT_HTTPD_PATH='/usr/sbin/apache2'\n-\t\tDEFAULT_HTTPD_MODULE_PATH='/usr/lib/apache2/modules'\n-\t;;\n esac\n \n LIB_HTTPD_PATH=${LIB_HTTPD_PATH-\"$DEFAULT_HTTPD_PATH\"}\n-- \n1.6.5.52.g35487\n"},{"id":"127949","messageId":"1258680123-28684-2-git-send-email-tarmigan+git@gmail.com","threadId":"21679","inReplyTo":"1258680123-28684-1-git-send-email-tarmigan+git@gmail.com","subject":"[PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Tarmigan Casebolt","fromEmail":"tarmigan+git@gmail.com","sentAt":"2009-11-20T01:22:03Z","receivedAt":"2009-11-20T01:22:03Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"With smart http, git over http is likely to become much more common.\nTo increase testing of smart http, enable the http tests by default.\n\nIf we cannot detect httpd, we still skip these tests, so it should not\ncause problems on platforms where we cannot run the tests.\n\nSigned-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>\n---\n t/lib-httpd.sh |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6b86353..db537b4 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -3,11 +3,12 @@\n # Copyright (c) 2008 Clemens Buchacher <drizzd@aon.at>\n #\n \n-if test -z \"$GIT_TEST_HTTPD\"\n+if test -n \"$NO_GIT_TEST_HTTPD\"\n then\n-\tsay \"skipping test, network testing disabled by default\"\n-\tsay \"(define GIT_TEST_HTTPD to enable)\"\n+\tsay \"Skipping http tests because NO_GIT_TEST_HTTPD is defined\"\n \ttest_done\n+else\n+\tsay \"Define NO_GIT_TEST_HTTPD to disable http testing\"\n fi\n \n HTTPD_PARA=\"\"\n-- \n1.6.5.52.g35487\n"},{"id":"127971","messageId":"76718490911191914n23d067b8teb17907de9ec83d5@mail.gmail.com","threadId":"21679","inReplyTo":"1258680123-28684-1-git-send-email-tarmigan+git@gmail.com","subject":"Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-11-20T03:14:02Z","receivedAt":"2009-11-20T03:14:02Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt\n<tarmigan+git@gmail.com> wrote:\n> uname might not be the best way to determine the default location for\n> httpd since different Linux distributions apparently put httpd in\n> different places, so we test a couple different locations for httpd,\n> and use the first one that we come across.  We do the same for the\n> modules directory.\n\nPerhaps testing the distribution and looking in the known location for\nthat distribution then? That said, going through a list of well known\nlocations should work too.\n\n> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n> +do\n> +       test -x \"$DEFAULT_HTTPD_PATH\" && break\n> +done\n\nUnfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the\nlist even if the test does not pass. You can add an empty item to the\nend of the list if you want to do this way.\n\n> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n> +                                 '/usr/lib/apache2/modules' \\\n> +                                 '/usr/lib64/httpd/modules' \\\n> +                                 '/usr/lib/httpd/modules'\n> +do\n> +       test -d \"$DEFAULT_HTTPD_MODULE_PATH\" && break\n> +done\n\nDitto.\n\nj.\n"},{"id":"127972","messageId":"905315640911191930rc33cabdr290b534ffbe85690@mail.gmail.com","threadId":"21679","inReplyTo":"76718490911191914n23d067b8teb17907de9ec83d5@mail.gmail.com","subject":"Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location","fromName":"Tarmigan","fromEmail":"tarmigan+git@gmail.com","sentAt":"2009-11-20T03:30:34Z","receivedAt":"2009-11-20T03:30:34Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"On Thu, Nov 19, 2009 at 7:14 PM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt\n> <tarmigan+git@gmail.com> wrote:\n>> uname might not be the best way to determine the default location for\n>> httpd since different Linux distributions apparently put httpd in\n>> different places, so we test a couple different locations for httpd,\n>> and use the first one that we come across.  We do the same for the\n>> modules directory.\n>\n> Perhaps testing the distribution and looking in the known location for\n> that distribution then? That said, going through a list of well known\n> locations should work too.\n\nIs there a nice way to test the distribution?  Seems to me like doing\nthat might be more complicated and also more fragile.\n\n>> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n>> +do\n>> +       test -x \"$DEFAULT_HTTPD_PATH\" && break\n>> +done\n>\n> Unfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the\n> list even if the test does not pass. You can add an empty item to the\n> end of the list if you want to do this way.\n\nYes.  I think this is how it was before though too, and it is caught\nlater in the script with the LIB_HTTPD_PATH setting and testing.\n\n>> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n>> +                                 '/usr/lib/apache2/modules' \\\n>> +                                 '/usr/lib64/httpd/modules' \\\n>> +                                 '/usr/lib/httpd/modules'\n>> +do\n>> +       test -d \"$DEFAULT_HTTPD_MODULE_PATH\" && break\n>> +done\n>\n> Ditto.\n\nYes.  Again, this is still more thorough than before, but in this case\nthe script does not check later.  Perhaps the script should test this\nvalue and test_done if it's not a directory?\n\nThanks,\nTarmigan\n"},{"id":"127983","messageId":"7vd43d7hdo.fsf@alter.siamese.dyndns.org","threadId":"21679","inReplyTo":"1258680123-28684-2-git-send-email-tarmigan+git@gmail.com","subject":"Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T08:03:15Z","receivedAt":"2009-11-20T08:03:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tarmigan Casebolt <tarmigan+git@gmail.com> writes:\n\n> With smart http, git over http is likely to become much more common.\n> To increase testing of smart http, enable the http tests by default.\n\nSorry, but no test that listens to network ports should be enabled by\ndefault; otherwise it will break automated, unattended tests people have\nalready set up randomly, depending on when the port happens to be\navailable for use by the tests.\n"},{"id":"128036","messageId":"905315640911201103w6d1da86duf41a53537672be8e@mail.gmail.com","threadId":"21679","inReplyTo":"7vd43d7hdo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Tarmigan","fromEmail":"tarmigan+git@gmail.com","sentAt":"2009-11-20T19:03:13Z","receivedAt":"2009-11-20T19:03:13Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"On Fri, Nov 20, 2009 at 12:03 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tarmigan <tarmigan+git@gmail.com> writes:\n>\n>> With smart http, git over http is likely to become much more common.\n>> To increase testing of smart http, enable the http tests by default.\n>\n> Sorry, but no test that listens to network ports should be enabled by\n> default; otherwise it will break automated, unattended tests people have\n> already set up randomly, depending on when the port happens to be\n> available for use by the tests.\n\nIs this the only concern or are there security or other issues as well?\n\nIf that is the only concern, we could have the tests automatically\nfall back to listening on a different port.  Even if we didn't, if\nhttpd cannot startup because it can't bind to the port, the http tests\nsay\n* skipping test, web server setup failed\nand exit with test_done before any of the tests actually fail.\n\nHere's a patch (cut-n-paste so it will probably be munged) for\ndiscussion of the port-fallback idea.  If httpd cannot bind to 5541,\nit tries 15541 etc.  You can test this by running \"nc -l 5541 &\"\nbefore the test.  If this approach might be acceptable, I can send a\nproperly formatted patch.\n\nComments?\n\nThanks,\nTarmigan\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 797a2d6..a8eb6fa 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -77,7 +77,7 @@ prepare_httpd() {\n\n        if test -n \"$LIB_HTTPD_SSL\"\n        then\n-               HTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT\n+               HTTPD_URL=https://127.0.0.1\n\n                RANDFILE_PATH=\"$HTTPD_ROOT_PATH\"/.rnd openssl req \\\n                        -config \"$TEST_PATH/ssl.cnf\" \\\n@@ -88,7 +88,7 @@ prepare_httpd() {\n                export GIT_SSL_NO_VERIFY\n                HTTPD_PARA=\"$HTTPD_PARA -DSSL\"\n        else\n-               HTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT\n+               HTTPD_URL=http://127.0.0.1\n        fi\n\n        if test -n \"$LIB_HTTPD_DAV\" -o -n \"$LIB_HTTPD_SVN\"\n@@ -109,16 +109,29 @@ start_httpd() {\n\n        trap 'code=$?; stop_httpd; (exit $code); die' EXIT\n\n-       \"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n-               -f \"$TEST_PATH/apache.conf\" $HTTPD_PARA \\\n-               -c \"Listen 127.0.0.1:$LIB_HTTPD_PORT\" -k start \\\n-               >&3 2>&4\n-       if test $? -ne 0\n-       then\n-               say \"skipping test, web server setup failed\"\n-               trap 'die' EXIT\n-               test_done\n-       fi\n+       while true\n+       do\n+               \"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n+                       -f \"$TEST_PATH/apache.conf\" $HTTPD_PARA \\\n+                       -c \"Listen 127.0.0.1:$LIB_HTTPD_PORT\" -k start \\\n+                       >&3 2>&4\n+               if test $? -ne 0\n+               then\n+                       if test $LIB_HTTPD_PORT -gt 40000\n+                       then\n+                               say \"skipping test, web server setup failed\"\n+                               trap 'die' EXIT\n+                               test_done\n+                       fi\n+                       LIB_HTTPD_PORT=$(($LIB_HTTPD_PORT + 10000))\n+                       say \"trying port $LIB_HTTPD_PORT\"\n+                       continue\n+               else\n+                       break\n+               fi\n+       done\n+\n+       HTTPD_URL=\"$HTTPD_URL:$LIB_HTTPD_PORT\"\n }\n\n stop_httpd() {\n"},{"id":"128043","messageId":"20091120201116.GA19131@localhost","threadId":"21679","inReplyTo":"905315640911201103w6d1da86duf41a53537672be8e@mail.gmail.com","subject":"Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-11-20T20:11:16Z","receivedAt":"2009-11-20T20:11:16Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Fri, Nov 20, 2009 at 11:03:13AM -0800, Tarmigan wrote:\n\n> Here's a patch (cut-n-paste so it will probably be munged) for\n> discussion of the port-fallback idea.  If httpd cannot bind to 5541,\n> it tries 15541 etc.\n\nI would prefer if we skip the test right away. If we really want to try\ndifferent ports, we should first check that the port really is the problem.\nOtherwise, the test will uselessly retry several times. Apache 2 writes\n\n (98)Address already in use: make_sock: could not bind to address\n 127.0.0.1:5541\n\nto stderr, which we could use to detect that error condition. But other web\nservers are bound to behave differently.\n\nClemens\n"},{"id":"128048","messageId":"7v6394x6ha.fsf@alter.siamese.dyndns.org","threadId":"21679","inReplyTo":"905315640911201103w6d1da86duf41a53537672be8e@mail.gmail.com","subject":"Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T20:54:09Z","receivedAt":"2009-11-20T20:54:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tarmigan <tarmigan+git@gmail.com> writes:\n\n> On Fri, Nov 20, 2009 at 12:03 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Tarmigan <tarmigan+git@gmail.com> writes:\n>>\n>>> With smart http, git over http is likely to become much more common.\n>>> To increase testing of smart http, enable the http tests by default.\n>>\n>> Sorry, but no test that listens to network ports should be enabled by\n>> default; otherwise it will break automated, unattended tests people have\n>> already set up randomly, depending on when the port happens to be\n>> available for use by the tests.\n>\n> Is this the only concern or are there security or other issues as well?\n\nI thought security was too obvious to mention.\n"},{"id":"128050","messageId":"7vtywovrsz.fsf@alter.siamese.dyndns.org","threadId":"21679","inReplyTo":"20091120201116.GA19131@localhost","subject":"Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T20:56:28Z","receivedAt":"2009-11-20T20:56:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Fri, Nov 20, 2009 at 11:03:13AM -0800, Tarmigan wrote:\n>\n>> Here's a patch (cut-n-paste so it will probably be munged) for\n>> discussion of the port-fallback idea.  If httpd cannot bind to 5541,\n>> it tries 15541 etc.\n>\n> I would prefer if we skip the test right away.\n\nRetrying is a different issue, and when tests are enabled I think it is Ok\nto retry all you want.\n\nBut I don't want to see them enabled unless the user explicitly told us\nto.\n"},{"id":"130716","messageId":"1262469865-9443-1-git-send-email-tarmigan+git@gmail.com","threadId":"21679","inReplyTo":"905315640911191930rc33cabdr290b534ffbe85690@mail.gmail.com","subject":"[PATCH v2] t/lib-http.sh: Restructure finding of default httpd location","fromName":"Tarmigan Casebolt","fromEmail":"tarmigan+git@gmail.com","sentAt":"2010-01-02T22:04:25Z","receivedAt":"2010-01-02T22:04:25Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"On CentOS 5, httpd is located at /usr/sbin/httpd, and the modules are\nlocated at /usr/lib64/httpd/modules.  To enable easy testing of httpd,\nwe would like those locations to be detected automatically.\n\nuname might not be the best way to determine the default location for\nhttpd since different Linux distributions apparently put httpd in\ndifferent places, so we test a couple different locations for httpd,\nand use the first one that we come across.  We do the same for the\nmodules directory.\n\ncc: Jay Soffian <jaysoffian@gmail.com>\nSigned-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>\n---\nJay was concerned about the final fallthrough cases for testing these\nlists.  I have added a test for the modules directory and the existing\ntests later in the script already test that the apache executable\nexists.  If either cannot be found, we do test_done.\n\nWould any machines have httpd or the modules/ directory in several of\nthese locations?\n---\n t/lib-httpd.sh |   30 ++++++++++++++++++++++++------\n 1 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6765b08..27b466b 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -12,16 +12,29 @@ fi\n \n HTTPD_PARA=\"\"\n \n+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'\n+do\n+\tif test -x \"$DEFAULT_HTTPD_PATH\"\n+\tthen\n+\t        break\n+\tfi\n+done\n+\n+for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \\\n+                                 '/usr/lib/apache2/modules' \\\n+                                 '/usr/lib64/httpd/modules' \\\n+                                 '/usr/lib/httpd/modules'\n+do\n+        if test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\tthen\n+\t        break\n+\tfi\n+done\n+\n case $(uname) in\n \tDarwin)\n-\t\tDEFAULT_HTTPD_PATH='/usr/sbin/httpd'\n-\t\tDEFAULT_HTTPD_MODULE_PATH='/usr/libexec/apache2'\n \t\tHTTPD_PARA=\"$HTTPD_PARA -DDarwin\"\n \t;;\n-\t*)\n-\t\tDEFAULT_HTTPD_PATH='/usr/sbin/apache2'\n-\t\tDEFAULT_HTTPD_MODULE_PATH='/usr/lib/apache2/modules'\n-\t;;\n esac\n \n LIB_HTTPD_PATH=${LIB_HTTPD_PATH-\"$DEFAULT_HTTPD_PATH\"}\n@@ -49,6 +62,11 @@ then\n \t\t\tsay \"skipping test, at least Apache version 2 is required\"\n \t\t\ttest_done\n \t\tfi\n+\t\tif ! test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n+\t\tthen\n+\t\t\tsay \"Apache module directory not found.  Skipping tests.\"\n+\t\t\ttest_done\n+\t\tfi\n \n \t\tLIB_HTTPD_MODULE_PATH=\"$DEFAULT_HTTPD_MODULE_PATH\"\n \tfi\n-- \n1.6.6.236.gc56f3\n"}]}