# [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. 11 messages from 2008-05-11 to 2008-05-11. Participants: nathan spindel, Junio C Hamano, David Symonds. Thread: https://gitlist.dev/t/13472 ## nathan spindel, 2008-05-11 05:26 Subject: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <1210483565-12415-1-git-send-email-nathans@gmail.com> URL: https://gitlist.dev/e/1210483565-12415-1-git-send-email-nathans%40gmail.com ``` Signed-off-by: nathan spindel --- git-instaweb.sh | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/git-instaweb.sh b/git-instaweb.sh index 6f91c8f..b744133 100755 --- a/git-instaweb.sh +++ b/git-instaweb.sh @@ -31,8 +31,16 @@ conf="$GIT_DIR/gitweb/httpd.conf" # Defaults: -# if installed, it doesn't need further configuration (module_path) -test -z "$httpd" && httpd='lighttpd -f' +# use lighttpd if it exists, otherwise use apache2 +if test -z "$httpd" +then + if type "lighttpd" > /dev/null 2>&1; + then + httpd="lighttpd -f" + else + httpd="apache2 -f" + fi +fi # any untaken local port will do... test -z "$port" && port=1234 -- 1.5.5.1.179.g2fe4.dirty ``` ## nathan spindel, 2008-05-11 05:26 Subject: [PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation. Message-ID: <1210483565-12415-2-git-send-email-nathans@gmail.com> URL: https://gitlist.dev/e/1210483565-12415-2-git-send-email-nathans%40gmail.com In-Reply-To: <1210483565-12415-1-git-send-email-nathans@gmail.com> ``` When in apache2 mode if there isn't an apache2 binary on the system but there is a httpd command in /usr/sbin/ (like there is on Mac OS X) use that instead. When in apache2 mode and there isn't a module_path specified, look for module paths in /usr/lib/apache2/modules _and_ /usr/libexec/apache2, in that order. Added a LockFile directive to the apache2 config because the default location of /private/var/run is only root-writeable on Mac OS X. Signed-off-by: nathan spindel --- git-instaweb.sh | 19 ++++++++++++++++++- 1 files changed, 18 insertions(+), 1 deletions(-) diff --git a/git-instaweb.sh b/git-instaweb.sh index b744133..1c9412c 100755 --- a/git-instaweb.sh +++ b/git-instaweb.sh @@ -180,7 +180,23 @@ EOF } apache2_conf () { - test -z "$module_path" && module_path=/usr/lib/apache2/modules + # if there isn't an apache2 command on the system but there is a httpd + # command in /usr/sbin/ use that instead for Mac OS X compatibility. + httpd_only="`echo $httpd | cut -f1 -d' '`" + type $httpd_only > /dev/null 2>&1; + test $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/httpd} + + if test -z "$module_path" + then + for path in /usr/lib/apache2/modules /usr/libexec/apache2; do + if test -d "$path" + then + module_path="$path" + break + fi + done + fi + mkdir -p "$GIT_DIR/gitweb/logs" bind= test x"$local" = xtrue && bind='127.0.0.1:' @@ -190,6 +206,7 @@ ServerName "git-instaweb" ServerRoot "$fqgitdir/gitweb" DocumentRoot "$fqgitdir/gitweb" PidFile "$fqgitdir/pid" +LockFile "$fqgitdir/gitweb/logs/accept.lock" Listen $bind$port EOF -- 1.5.5.1.179.g2fe4.dirty ``` ## nathan spindel, 2008-05-11 05:26 Subject: [PATCH] Documentation/git-instaweb.txt: Updated defaults to match my last two git-instaweb.sh changes Message-ID: <1210483565-12415-3-git-send-email-nathans@gmail.com> URL: https://gitlist.dev/e/1210483565-12415-3-git-send-email-nathans%40gmail.com In-Reply-To: <1210483565-12415-2-git-send-email-nathans@gmail.com> ``` Signed-off-by: nathan spindel --- Documentation/git-instaweb.txt | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-instaweb.txt b/Documentation/git-instaweb.txt index 51f1532..aae5334 100644 --- a/Documentation/git-instaweb.txt +++ b/Documentation/git-instaweb.txt @@ -28,11 +28,12 @@ OPTIONS Command-line options may be specified here, and the configuration file will be added at the end of the command-line. Currently lighttpd, apache2 and webrick are supported. - (Default: lighttpd) + (Default: lighttpd if it exists, otherwise apache2) -m|--module-path:: The module path (only needed if httpd is Apache). - (Default: /usr/lib/apache2/modules) + (Default: /usr/lib/apache2/modules or /usr/libexec/apache2, + whichever exists) -p|--port:: The port number to bind the httpd to. (Default: 1234) -- 1.5.5.1.179.g2fe4.dirty ``` ## Junio C Hamano, 2008-05-11 06:44 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <7vlk2h7318.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlk2h7318.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1210483565-12415-1-git-send-email-nathans@gmail.com> ``` nathan spindel writes: > Signed-off-by: nathan spindel > --- > git-instaweb.sh | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/git-instaweb.sh b/git-instaweb.sh > index 6f91c8f..b744133 100755 > --- a/git-instaweb.sh > +++ b/git-instaweb.sh > @@ -31,8 +31,16 @@ conf="$GIT_DIR/gitweb/httpd.conf" > > # Defaults: > > -# if installed, it doesn't need further configuration (module_path) > -test -z "$httpd" && httpd='lighttpd -f' > +# use lighttpd if it exists, otherwise use apache2 > +if test -z "$httpd" > +then > + if type "lighttpd" > /dev/null 2>&1; The exit code from "type" is very loosely defined by POSIX which just says it exits >0 to signal that "an error occured". Presumably it means there is no such command that is executable on the $PATH, and it may be more portable and reliable than using "which", but this still worries me. Doesn't "lighttpd" have an option that reports "I am here" and exit 0, e.g. "--version"? Then we could instead say: if lighttpd --version >/dev/null then ... use it ... and that would be much nicer... ``` ## Junio C Hamano, 2008-05-11 06:58 Subject: Re: [PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation. Message-ID: <7vhcd572e3.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhcd572e3.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1210483565-12415-2-git-send-email-nathans@gmail.com> ``` nathan spindel writes: > When in apache2 mode if there isn't an apache2 binary on the system but > there is a httpd command in /usr/sbin/ (like there is on Mac OS X) > use that instead. How would you ensure that httpd is actually Apache and not something else? > When in apache2 mode and there isn't a module_path specified, look for > module paths in /usr/lib/apache2/modules _and_ /usr/libexec/apache2, > in that order. This one sounds Ok. > apache2_conf () { > - test -z "$module_path" && module_path=/usr/lib/apache2/modules > + # if there isn't an apache2 command on the system but there is a httpd > + # command in /usr/sbin/ use that instead for Mac OS X compatibility. > + httpd_only="`echo $httpd | cut -f1 -d' '`" > + type $httpd_only > /dev/null 2>&1; > + test $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/httpd} I see the same "type" issue as I mentioned, but I see that we use them in mergetool and web--browse and we haven't heard breakages so perhaps this is portable enough ;-) Please avoid ${parameter/pattern/string} expansion, which is not even in POSIX. It is bashism and unportable. > + if test -z "$module_path" > + then > + for path in /usr/lib/apache2/modules /usr/libexec/apache2; do Hmm. If you do discovery like this, maybe you would want to do discovery for "httpd" the same way? After all, why look for it only in /usr/sbin? > @@ -190,6 +206,7 @@ ServerName "git-instaweb" > ServerRoot "$fqgitdir/gitweb" > DocumentRoot "$fqgitdir/gitweb" > PidFile "$fqgitdir/pid" > +LockFile "$fqgitdir/gitweb/logs/accept.lock" > Listen $bind$port We've seen this elsewhere and this part should be Ok. ``` ## David Symonds, 2008-05-11 11:58 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: URL: https://gitlist.dev/e/ee77f5c20805110458s6f1e4908k38be50753ba38d09%40mail.gmail.com In-Reply-To: <1210483565-12415-1-git-send-email-nathans@gmail.com> ``` On Sun, May 11, 2008 at 3:26 PM, nathan spindel wrote: > -# if installed, it doesn't need further configuration (module_path) > -test -z "$httpd" && httpd='lighttpd -f' > +# use lighttpd if it exists, otherwise use apache2 > +if test -z "$httpd" > +then > + if type "lighttpd" > /dev/null 2>&1; > + then > + httpd="lighttpd -f" > + else > + httpd="apache2 -f" > + fi > +fi I personally would prefer to use webrick over apache2, since it's much more lightweight. That's just my two cents. Dave. ``` ## nathan spindel, 2008-05-11 17:00 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <0EB126FA-6752-4C66-B93D-48694159E3B6@gmail.com> URL: https://gitlist.dev/e/0EB126FA-6752-4C66-B93D-48694159E3B6%40gmail.com In-Reply-To: <7vlk2h7318.fsf@gitster.siamese.dyndns.org> ``` On May 10, 2008, at 11:44 PM, Junio C Hamano wrote: > nathan spindel writes: > >> Signed-off-by: nathan spindel >> --- >> git-instaweb.sh | 12 ++++++++++-- >> 1 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/git-instaweb.sh b/git-instaweb.sh >> index 6f91c8f..b744133 100755 >> --- a/git-instaweb.sh >> +++ b/git-instaweb.sh >> @@ -31,8 +31,16 @@ conf="$GIT_DIR/gitweb/httpd.conf" >> >> # Defaults: >> >> -# if installed, it doesn't need further configuration (module_path) >> -test -z "$httpd" && httpd='lighttpd -f' >> +# use lighttpd if it exists, otherwise use apache2 >> +if test -z "$httpd" >> +then >> + if type "lighttpd" > /dev/null 2>&1; > > The exit code from "type" is very loosely defined by POSIX which > just says > it exits >0 to signal that "an error occured". Presumably it means > there > is no such command that is executable on the $PATH, and it may be more > portable and reliable than using "which", but this still worries me. > > Doesn't "lighttpd" have an option that reports "I am here" and exit 0, > e.g. "--version"? Then we could instead say: > > if lighttpd --version >/dev/null > then > ... use it ... > > and that would be much nicer... I didn't know that the portability of "type" was questionable. That's a good idea. Thanks! ``` ## nathan spindel, 2008-05-11 17:03 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <5C50F5DC-CAE4-4BF2-BCAA-8CFC8CC43C59@gmail.com> URL: https://gitlist.dev/e/5C50F5DC-CAE4-4BF2-BCAA-8CFC8CC43C59%40gmail.com In-Reply-To: ``` On May 11, 2008, at 4:58 AM, David Symonds wrote: > On Sun, May 11, 2008 at 3:26 PM, nathan spindel > wrote: > >> -# if installed, it doesn't need further configuration (module_path) >> -test -z "$httpd" && httpd='lighttpd -f' >> +# use lighttpd if it exists, otherwise use apache2 >> +if test -z "$httpd" >> +then >> + if type "lighttpd" > /dev/null 2>&1; >> + then >> + httpd="lighttpd -f" >> + else >> + httpd="apache2 -f" >> + fi >> +fi > > I personally would prefer to use webrick over apache2, since it's much > more lightweight. That's just my two cents. I am apathetic about which web servers are preferred, I would just like to see git-instaweb work well on Mac OS X. Kevin Ballard's suggestion was to fall back on apache2. We could also modify the default to use [lighttpd, webrick, apache2] in that order (whichever is available). Would the community prefer that approach? ``` ## nathan spindel, 2008-05-11 17:11 Subject: Re: [PATCH] instaweb: make it compatible with Mac OS X 10.5's apache installation. Message-ID: <843D3D0E-0121-42E2-B285-E1F4AB9415E4@gmail.com> URL: https://gitlist.dev/e/843D3D0E-0121-42E2-B285-E1F4AB9415E4%40gmail.com In-Reply-To: <7vhcd572e3.fsf@gitster.siamese.dyndns.org> ``` On May 10, 2008, at 11:58 PM, Junio C Hamano wrote: > nathan spindel writes: > >> When in apache2 mode if there isn't an apache2 binary on the system >> but >> there is a httpd command in /usr/sbin/ (like there is on Mac OS X) >> use that instead. > > How would you ensure that httpd is actually Apache and not something > else? Good point. How about running it with -v and searching for Apache in the first line? >> + httpd_only="`echo $httpd | cut -f1 -d' '`" >> + type $httpd_only > /dev/null 2>&1; >> + test $? != 0 && test -x /usr/sbin/httpd && httpd=${httpd/apache2/ >> httpd} > > I see the same "type" issue as I mentioned, but I see that we use > them in > mergetool and web--browse and we haven't heard breakages so perhaps > this > is portable enough ;-) Yes, inspection of mergetool and web--browse were what led me to use type. I will see if the other method you suggested can work and use that here if so. > Please avoid ${parameter/pattern/string} expansion, which is not > even in > POSIX. It is bashism and unportable. I didn't know that was just a bashism. I'll fix that usage. >> + if test -z "$module_path" >> + then >> + for path in /usr/lib/apache2/modules /usr/libexec/apache2; do > > Hmm. If you do discovery like this, maybe you would want to do > discovery > for "httpd" the same way? After all, why look for it only in /usr/ > sbin? Yes, it should look in multiple locations. ``` ## Junio C Hamano, 2008-05-11 17:46 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <7vlk2g4tue.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlk2g4tue.fsf%40gitster.siamese.dyndns.org In-Reply-To: <5C50F5DC-CAE4-4BF2-BCAA-8CFC8CC43C59@gmail.com> ``` nathan spindel writes: > I am apathetic about which web servers are preferred, I would just > like to see git-instaweb work well on Mac OS X. Kevin Ballard's > suggestion was to fall back on apache2. > > We could also modify the default to use [lighttpd, webrick, apache2] > in that order (whichever is available). Would the community prefer > that approach? I think apache is the most well known and widely used among the three, and find Kevin's suggestion sensible. ``` ## nathan spindel, 2008-05-11 18:26 Subject: Re: [PATCH] instaweb: if no httpd is specified and lighttpd doesn't exist, fall back on apache2. Message-ID: <17FDB7BC-D8FC-4016-8704-61D557BF874F@gmail.com> URL: https://gitlist.dev/e/17FDB7BC-D8FC-4016-8704-61D557BF874F%40gmail.com In-Reply-To: <7vlk2h7318.fsf@gitster.siamese.dyndns.org> ``` On May 10, 2008, at 11:44 PM, Junio C Hamano wrote: > nathan spindel writes: > >> Signed-off-by: nathan spindel >> --- >> git-instaweb.sh | 12 ++++++++++-- >> 1 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/git-instaweb.sh b/git-instaweb.sh >> index 6f91c8f..b744133 100755 >> --- a/git-instaweb.sh >> +++ b/git-instaweb.sh >> @@ -31,8 +31,16 @@ conf="$GIT_DIR/gitweb/httpd.conf" >> >> # Defaults: >> >> -# if installed, it doesn't need further configuration (module_path) >> -test -z "$httpd" && httpd='lighttpd -f' >> +# use lighttpd if it exists, otherwise use apache2 >> +if test -z "$httpd" >> +then >> + if type "lighttpd" > /dev/null 2>&1; > > The exit code from "type" is very loosely defined by POSIX which > just says > it exits >0 to signal that "an error occured". Presumably it means > there > is no such command that is executable on the $PATH, and it may be more > portable and reliable than using "which", but this still worries me. > > Doesn't "lighttpd" have an option that reports "I am here" and exit 0, > e.g. "--version"? Then we could instead say: > > if lighttpd --version >/dev/null > then > ... use it ... > > and that would be much nicer... Actually, this approach has the same problem that's worked around here: . (You might have lighttpd installed but not be on your $PATH, which the script should handle for you). It seems the best approach would be to function-ize the aforementioned httpd-path-checking logic [that currently uses "which"] and use it for (a) detecting if lighttpd is installed and (b) looking for apache2 binaries named httpd. I think for now these particular patches will continue to use "type" as you somewhat suggested earlier today. :) ```