{"thread":{"id":"13472","subject":"[PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","startedAt":"2008-05-11T05:26:03Z","lastAt":"2008-05-11T18:26:31Z","messageCount":11,"participants":["nathan spindel","Junio C Hamano","David Symonds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"76578","messageId":"1210483565-12415-1-git-send-email-nathans@gmail.com","threadId":"13472","inReplyTo":null,"subject":"[PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T05:26:03Z","receivedAt":"2008-05-11T05:26:03Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"Signed-off-by: nathan spindel <nathans@gmail.com>\n---\n git-instaweb.sh |   12 ++++++++++--\n 1 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 6f91c8f..b744133 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -31,8 +31,16 @@ conf=\"$GIT_DIR/gitweb/httpd.conf\"\n \n # Defaults:\n \n-# if installed, it doesn't need further configuration (module_path)\n-test -z \"$httpd\" && httpd='lighttpd -f'\n+# use lighttpd if it exists, otherwise use apache2\n+if test -z \"$httpd\"\n+then\n+\tif type \"lighttpd\" > /dev/null 2>&1;\n+\tthen\n+\t\thttpd=\"lighttpd -f\"\n+\telse\n+\t\thttpd=\"apache2 -f\"\n+\tfi\n+fi\n \n # any untaken local port will do...\n test -z \"$port\" && port=1234\n-- \n1.5.5.1.179.g2fe4.dirty\n"},{"id":"76579","messageId":"1210483565-12415-2-git-send-email-nathans@gmail.com","threadId":"13472","inReplyTo":"1210483565-12415-1-git-send-email-nathans@gmail.com","subject":"[PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T05:26:04Z","receivedAt":"2008-05-11T05:26:04Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"When in apache2 mode if there isn't an apache2 binary on the system but\nthere is a httpd command in /usr/sbin/ (like there is on Mac OS X)\nuse that instead.\n\nWhen in apache2 mode and there isn't a module_path specified, look for\nmodule paths in /usr/lib/apache2/modules _and_ /usr/libexec/apache2,\nin that order.\n\nAdded a LockFile directive to the apache2 config because the default\nlocation of /private/var/run is only root-writeable on Mac OS X.\n\nSigned-off-by: nathan spindel <nathans@gmail.com>\n---\n git-instaweb.sh |   19 ++++++++++++++++++-\n 1 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex b744133..1c9412c 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -180,7 +180,23 @@ EOF\n }\n \n apache2_conf () {\n-\ttest -z \"$module_path\" && module_path=/usr/lib/apache2/modules\n+\t# if there isn't an apache2 command on the system but there is a httpd\n+\t# command in /usr/sbin/ use that instead for Mac OS X compatibility.\n+\thttpd_only=\"`echo $httpd | cut -f1 -d' '`\"\n+\ttype $httpd_only > /dev/null 2>&1;\n+\ttest $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/httpd}\n+\n+\tif test -z \"$module_path\"\n+\tthen\n+\t\tfor path in /usr/lib/apache2/modules /usr/libexec/apache2; do\n+\t\t\tif test -d \"$path\"\n+\t\t\tthen\n+\t\t\t\tmodule_path=\"$path\"\n+\t\t\t\tbreak\n+\t\t\tfi\n+\t\tdone\n+\tfi\n+\n \tmkdir -p \"$GIT_DIR/gitweb/logs\"\n \tbind=\n \ttest x\"$local\" = xtrue && bind='127.0.0.1:'\n@@ -190,6 +206,7 @@ ServerName \"git-instaweb\"\n ServerRoot \"$fqgitdir/gitweb\"\n DocumentRoot \"$fqgitdir/gitweb\"\n PidFile \"$fqgitdir/pid\"\n+LockFile \"$fqgitdir/gitweb/logs/accept.lock\"\n Listen $bind$port\n EOF\n \n-- \n1.5.5.1.179.g2fe4.dirty\n"},{"id":"76580","messageId":"1210483565-12415-3-git-send-email-nathans@gmail.com","threadId":"13472","inReplyTo":"1210483565-12415-2-git-send-email-nathans@gmail.com","subject":"[PATCH] Documentation/git-instaweb.txt: Updated defaults to match my last two git-instaweb.sh changes","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T05:26:05Z","receivedAt":"2008-05-11T05:26:05Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"Signed-off-by: nathan spindel <nathans@gmail.com>\n---\n Documentation/git-instaweb.txt |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-instaweb.txt b/Documentation/git-instaweb.txt\nindex 51f1532..aae5334 100644\n--- a/Documentation/git-instaweb.txt\n+++ b/Documentation/git-instaweb.txt\n@@ -28,11 +28,12 @@ OPTIONS\n \tCommand-line options may be specified here, and the\n \tconfiguration file will be added at the end of the command-line.\n \tCurrently lighttpd, apache2 and webrick are supported.\n-\t(Default: lighttpd)\n+\t(Default: lighttpd if it exists, otherwise apache2)\n \n -m|--module-path::\n \tThe module path (only needed if httpd is Apache).\n-\t(Default: /usr/lib/apache2/modules)\n+\t(Default: /usr/lib/apache2/modules or /usr/libexec/apache2, \n+\twhichever exists)\n \n -p|--port::\n \tThe port number to bind the httpd to.  (Default: 1234)\n-- \n1.5.5.1.179.g2fe4.dirty\n"},{"id":"76585","messageId":"7vlk2h7318.fsf@gitster.siamese.dyndns.org","threadId":"13472","inReplyTo":"1210483565-12415-1-git-send-email-nathans@gmail.com","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T06:44:51Z","receivedAt":"2008-05-11T06:44:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"nathan spindel <nathans@gmail.com> writes:\n\n> Signed-off-by: nathan spindel <nathans@gmail.com>\n> ---\n>  git-instaweb.sh |   12 ++++++++++--\n>  1 files changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-instaweb.sh b/git-instaweb.sh\n> index 6f91c8f..b744133 100755\n> --- a/git-instaweb.sh\n> +++ b/git-instaweb.sh\n> @@ -31,8 +31,16 @@ conf=\"$GIT_DIR/gitweb/httpd.conf\"\n>  \n>  # Defaults:\n>  \n> -# if installed, it doesn't need further configuration (module_path)\n> -test -z \"$httpd\" && httpd='lighttpd -f'\n> +# use lighttpd if it exists, otherwise use apache2\n> +if test -z \"$httpd\"\n> +then\n> +\tif type \"lighttpd\" > /dev/null 2>&1;\n\nThe exit code from \"type\" is very loosely defined by POSIX which just says\nit exits >0 to signal that \"an error occured\".  Presumably it means there\nis no such command that is executable on the $PATH, and it may be more\nportable and reliable than using \"which\", but this still worries me.\n\nDoesn't \"lighttpd\" have an option that reports \"I am here\" and exit 0,\ne.g. \"--version\"?  Then we could instead say:\n\n\tif lighttpd --version >/dev/null\n        then\n        \t... use it ...\n\nand that would be much nicer...\n"},{"id":"76586","messageId":"7vhcd572e3.fsf@gitster.siamese.dyndns.org","threadId":"13472","inReplyTo":"1210483565-12415-2-git-send-email-nathans@gmail.com","subject":"Re: [PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T06:58:44Z","receivedAt":"2008-05-11T06:58:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"nathan spindel <nathans@gmail.com> writes:\n\n> When in apache2 mode if there isn't an apache2 binary on the system but\n> there is a httpd command in /usr/sbin/ (like there is on Mac OS X)\n> use that instead.\n\nHow would you ensure that httpd is actually Apache and not something else?\n\n> When in apache2 mode and there isn't a module_path specified, look for\n> module paths in /usr/lib/apache2/modules _and_ /usr/libexec/apache2,\n> in that order.\n\nThis one sounds Ok.\n\n>  apache2_conf () {\n> -\ttest -z \"$module_path\" && module_path=/usr/lib/apache2/modules\n> +\t# if there isn't an apache2 command on the system but there is a httpd\n> +\t# command in /usr/sbin/ use that instead for Mac OS X compatibility.\n\n\n> +\thttpd_only=\"`echo $httpd | cut -f1 -d' '`\"\n> +\ttype $httpd_only > /dev/null 2>&1;\n> +\ttest $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/httpd}\n\nI see the same \"type\" issue as I mentioned, but I see that we use them in\nmergetool and web--browse and we haven't heard breakages so perhaps this\nis portable enough ;-)\n\nPlease avoid ${parameter/pattern/string} expansion, which is not even in\nPOSIX.  It is bashism and unportable.\n\n> +\tif test -z \"$module_path\"\n> +\tthen\n> +\t\tfor path in /usr/lib/apache2/modules /usr/libexec/apache2; do\n\nHmm.  If you do discovery like this, maybe you would want to do discovery\nfor \"httpd\" the same way?  After all, why look for it only in /usr/sbin?\n\n> @@ -190,6 +206,7 @@ ServerName \"git-instaweb\"\n>  ServerRoot \"$fqgitdir/gitweb\"\n>  DocumentRoot \"$fqgitdir/gitweb\"\n>  PidFile \"$fqgitdir/pid\"\n> +LockFile \"$fqgitdir/gitweb/logs/accept.lock\"\n>  Listen $bind$port\n\nWe've seen this elsewhere and this part should be Ok.\n"},{"id":"76602","messageId":"ee77f5c20805110458s6f1e4908k38be50753ba38d09@mail.gmail.com","threadId":"13472","inReplyTo":"1210483565-12415-1-git-send-email-nathans@gmail.com","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2008-05-11T11:58:34Z","receivedAt":"2008-05-11T11:58:34Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Sun, May 11, 2008 at 3:26 PM, nathan spindel <nathans@gmail.com> wrote:\n\n> -# if installed, it doesn't need further configuration (module_path)\n> -test -z \"$httpd\" && httpd='lighttpd -f'\n> +# use lighttpd if it exists, otherwise use apache2\n> +if test -z \"$httpd\"\n> +then\n> +       if type \"lighttpd\" > /dev/null 2>&1;\n> +       then\n> +               httpd=\"lighttpd -f\"\n> +       else\n> +               httpd=\"apache2 -f\"\n> +       fi\n> +fi\n\nI personally would prefer to use webrick over apache2, since it's much\nmore lightweight. That's just my two cents.\n\n\nDave.\n"},{"id":"76637","messageId":"0EB126FA-6752-4C66-B93D-48694159E3B6@gmail.com","threadId":"13472","inReplyTo":"7vlk2h7318.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T17:00:11Z","receivedAt":"2008-05-11T17:00:11Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"On May 10, 2008, at 11:44 PM, Junio C Hamano wrote:\n\n> nathan spindel <nathans@gmail.com> writes:\n>\n>> Signed-off-by: nathan spindel <nathans@gmail.com>\n>> ---\n>> git-instaweb.sh |   12 ++++++++++--\n>> 1 files changed, 10 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/git-instaweb.sh b/git-instaweb.sh\n>> index 6f91c8f..b744133 100755\n>> --- a/git-instaweb.sh\n>> +++ b/git-instaweb.sh\n>> @@ -31,8 +31,16 @@ conf=\"$GIT_DIR/gitweb/httpd.conf\"\n>>\n>> # Defaults:\n>>\n>> -# if installed, it doesn't need further configuration (module_path)\n>> -test -z \"$httpd\" && httpd='lighttpd -f'\n>> +# use lighttpd if it exists, otherwise use apache2\n>> +if test -z \"$httpd\"\n>> +then\n>> +\tif type \"lighttpd\" > /dev/null 2>&1;\n>\n> The exit code from \"type\" is very loosely defined by POSIX which  \n> just says\n> it exits >0 to signal that \"an error occured\".  Presumably it means  \n> there\n> is no such command that is executable on the $PATH, and it may be more\n> portable and reliable than using \"which\", but this still worries me.\n>\n> Doesn't \"lighttpd\" have an option that reports \"I am here\" and exit 0,\n> e.g. \"--version\"?  Then we could instead say:\n>\n> \tif lighttpd --version >/dev/null\n>        then\n>        \t... use it ...\n>\n> and that would be much nicer...\n\nI didn't know that the portability of \"type\" was questionable.  That's  \na good idea.  Thanks!\n"},{"id":"76638","messageId":"5C50F5DC-CAE4-4BF2-BCAA-8CFC8CC43C59@gmail.com","threadId":"13472","inReplyTo":"ee77f5c20805110458s6f1e4908k38be50753ba38d09@mail.gmail.com","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T17:03:34Z","receivedAt":"2008-05-11T17:03:34Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"On May 11, 2008, at 4:58 AM, David Symonds wrote:\n\n> On Sun, May 11, 2008 at 3:26 PM, nathan spindel <nathans@gmail.com>  \n> wrote:\n>\n>> -# if installed, it doesn't need further configuration (module_path)\n>> -test -z \"$httpd\" && httpd='lighttpd -f'\n>> +# use lighttpd if it exists, otherwise use apache2\n>> +if test -z \"$httpd\"\n>> +then\n>> +       if type \"lighttpd\" > /dev/null 2>&1;\n>> +       then\n>> +               httpd=\"lighttpd -f\"\n>> +       else\n>> +               httpd=\"apache2 -f\"\n>> +       fi\n>> +fi\n>\n> I personally would prefer to use webrick over apache2, since it's much\n> more lightweight. That's just my two cents.\n\nI am apathetic about which web servers are preferred, I would just  \nlike to see git-instaweb work well on Mac OS X.  Kevin Ballard's  \nsuggestion was to fall back on apache2.\n\nWe could also modify the default to use [lighttpd, webrick, apache2]  \nin that order (whichever is available). Would the community prefer  \nthat approach?\n"},{"id":"76641","messageId":"843D3D0E-0121-42E2-B285-E1F4AB9415E4@gmail.com","threadId":"13472","inReplyTo":"7vhcd572e3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T17:11:50Z","receivedAt":"2008-05-11T17:11:50Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"On May 10, 2008, at 11:58 PM, Junio C Hamano wrote:\n\n> nathan spindel <nathans@gmail.com> writes:\n>\n>> When in apache2 mode if there isn't an apache2 binary on the system  \n>> but\n>> there is a httpd command in /usr/sbin/ (like there is on Mac OS X)\n>> use that instead.\n>\n> How would you ensure that httpd is actually Apache and not something  \n> else?\n\nGood point. How about running it with -v and searching for Apache in  \nthe first line?\n\n>> +\thttpd_only=\"`echo $httpd | cut -f1 -d' '`\"\n>> +\ttype $httpd_only > /dev/null 2>&1;\n>> +\ttest $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/ \n>> httpd}\n>\n> I see the same \"type\" issue as I mentioned, but I see that we use  \n> them in\n> mergetool and web--browse and we haven't heard breakages so perhaps  \n> this\n> is portable enough ;-)\n\nYes, inspection of mergetool and web--browse were what led me to use  \ntype.  I will see if the other method you suggested can work and use  \nthat here if so.\n\n> Please avoid ${parameter/pattern/string} expansion, which is not  \n> even in\n> POSIX.  It is bashism and unportable.\n\nI didn't know that was just a bashism. I'll fix that usage.\n\n>> +\tif test -z \"$module_path\"\n>> +\tthen\n>> +\t\tfor path in /usr/lib/apache2/modules /usr/libexec/apache2; do\n>\n> Hmm.  If you do discovery like this, maybe you would want to do  \n> discovery\n> for \"httpd\" the same way?  After all, why look for it only in /usr/ \n> sbin?\n\nYes, it should look in multiple locations.\n"},{"id":"76646","messageId":"7vlk2g4tue.fsf@gitster.siamese.dyndns.org","threadId":"13472","inReplyTo":"5C50F5DC-CAE4-4BF2-BCAA-8CFC8CC43C59@gmail.com","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T17:46:17Z","receivedAt":"2008-05-11T17:46:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"nathan spindel <nathans@gmail.com> writes:\n\n> I am apathetic about which web servers are preferred, I would just\n> like to see git-instaweb work well on Mac OS X.  Kevin Ballard's\n> suggestion was to fall back on apache2.\n>\n> We could also modify the default to use [lighttpd, webrick, apache2]\n> in that order (whichever is available). Would the community prefer\n> that approach?\n\nI think apache is the most well known and widely used among the three, and\nfind Kevin's suggestion sensible.\n"},{"id":"76649","messageId":"17FDB7BC-D8FC-4016-8704-61D557BF874F@gmail.com","threadId":"13472","inReplyTo":"7vlk2h7318.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2.","fromName":"nathan spindel","fromEmail":"nathans@gmail.com","sentAt":"2008-05-11T18:26:31Z","receivedAt":"2008-05-11T18:26:31Z","isPatch":true,"sender":{"key":"nathans@gmail.com","avatar":"https://gravatar.com/avatar/cccc9df7264bf4abc5d9d2fa5ce120607d2dd3741d429740b8edca4bfb4dcd1a?d=mp&s=160"},"body":"On May 10, 2008, at 11:44 PM, Junio C Hamano wrote:\n\n> nathan spindel <nathans@gmail.com> writes:\n>\n>> Signed-off-by: nathan spindel <nathans@gmail.com>\n>> ---\n>> git-instaweb.sh |   12 ++++++++++--\n>> 1 files changed, 10 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/git-instaweb.sh b/git-instaweb.sh\n>> index 6f91c8f..b744133 100755\n>> --- a/git-instaweb.sh\n>> +++ b/git-instaweb.sh\n>> @@ -31,8 +31,16 @@ conf=\"$GIT_DIR/gitweb/httpd.conf\"\n>>\n>> # Defaults:\n>>\n>> -# if installed, it doesn't need further configuration (module_path)\n>> -test -z \"$httpd\" && httpd='lighttpd -f'\n>> +# use lighttpd if it exists, otherwise use apache2\n>> +if test -z \"$httpd\"\n>> +then\n>> +\tif type \"lighttpd\" > /dev/null 2>&1;\n>\n> The exit code from \"type\" is very loosely defined by POSIX which  \n> just says\n> it exits >0 to signal that \"an error occured\".  Presumably it means  \n> there\n> is no such command that is executable on the $PATH, and it may be more\n> portable and reliable than using \"which\", but this still worries me.\n>\n> Doesn't \"lighttpd\" have an option that reports \"I am here\" and exit 0,\n> e.g. \"--version\"?  Then we could instead say:\n>\n> \tif lighttpd --version >/dev/null\n>        then\n>        \t... use it ...\n>\n> and that would be much nicer...\n\nActually, this approach has the same problem that's worked around  \nhere: <http://repo.or.cz/w/git.git?a=blob;f=git- \ninstaweb.sh;hb=HEAD#l46>. (You might have lighttpd installed but not  \nbe on your $PATH, which the script should handle for you).\n\nIt seems the best approach would be to function-ize the aforementioned  \nhttpd-path-checking logic [that currently uses \"which\"] and use it for  \n(a) detecting if lighttpd is installed and (b) looking for apache2  \nbinaries named httpd.\n\nI think for now these particular patches will continue to use \"type\"  \nas you somewhat suggested earlier today.  :)\n"}]}