threads / patch / 13481

v2instaweb: make it compatible with Mac OS X 10.5's apache installation.

Subject: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

## tl;dr

6 messages between May 11, 2008 and May 13, 2008. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

nathan spindel· May 11, 2008, 19:36 UTC · lore

When in apache2 mode if there isn't an apache2 command on the system but there is a httpd command installed (like there is on Mac OS X) use that command 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 <nathans@gmail.com>
---
 git-instaweb.sh |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)
Show changes to git-instaweb.sh +54 −1
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6f91c8f..00e147f 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -172,7 +172,59 @@ 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 which looks like apache, use that instead
+	# for Mac OS X compatibility.
+	httpd_only="`echo $httpd | cut -f1 -d' '`"
+	if ! type $httpd_only > /dev/null 2>&1
+	then
+		found_apache_command=1
+		httpd_parent_paths="/usr/local/sbin /usr/sbin"
+		for i in $httpd_parent_paths; do
+			if test -x "$i/$httpd_only"
+			then
+				found_apache_command=0
+				break
+			fi
+		done
+
+		if test $found_apache_command != 0
+		then
+			alt_apache="httpd"
+			if type $alt_apache > /dev/null 2>&1
+			then
+				$alt_apache -v | grep Apache > /dev/null 2>&1
+				if test $? == 0
+				then
+					httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
+				else
+					for i in $httpd_parent_paths; do
+						if test -x "$i/$alt_apache"
+						then
+							$i/$alt_apache -v | grep Apache > /dev/null 2>&1
+							if test $? == 0
+							then
+								httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
+								break
+							fi
+						fi
+					done
+				fi
+			fi
+		fi
+	fi
+
+	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:'
@@ -182,6 +234,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· May 11, 2008, 19:41 UTC · re: nathan spindel · lore

Re: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

On May 11, 2008, at 12:36 PM, nathan spindel wrote:
Show 43 quoted lines
> 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 which looks like apache, use that instead
> +	# for Mac OS X compatibility.
> +	httpd_only="`echo $httpd | cut -f1 -d' '`"
> +	if ! type $httpd_only > /dev/null 2>&1
> +	then
> +		found_apache_command=1
> +		httpd_parent_paths="/usr/local/sbin /usr/sbin"
> +		for i in $httpd_parent_paths; do
> +			if test -x "$i/$httpd_only"
> +			then
> +				found_apache_command=0
> +				break
> +			fi
> +		done
> +
> +		if test $found_apache_command != 0
> +		then
> +			alt_apache="httpd"
> +			if type $alt_apache > /dev/null 2>&1
> +			then
> +				$alt_apache -v | grep Apache > /dev/null 2>&1
> +				if test $? == 0
> +				then
> +					httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
> +				else
> +					for i in $httpd_parent_paths; do
> +						if test -x "$i/$alt_apache"
> +						then
> +							$i/$alt_apache -v | grep Apache > /dev/null 2>&1
> +							if test $? == 0
> +							then
> +								httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
> +								break
> +							fi
> +						fi
> +					done
> +				fi
> +			fi
> +		fi
> +	fi

I'd like to see a cleaner implementation of this logic, if anyone has strong shell-fu.

Christian Couder· May 12, 2008, 17:52 UTC · re: nathan spindel · lore

Re: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

Le dimanche 11 mai 2008, nathan spindel a écrit :
Show 46 quoted lines
> On May 11, 2008, at 12:36 PM, nathan spindel wrote:
> > 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 which looks like apache, use that instead
> > +	# for Mac OS X compatibility.
> > +	httpd_only="`echo $httpd | cut -f1 -d' '`"
> > +	if ! type $httpd_only > /dev/null 2>&1
> > +	then
> > +		found_apache_command=1
> > +		httpd_parent_paths="/usr/local/sbin /usr/sbin"
> > +		for i in $httpd_parent_paths; do
> > +			if test -x "$i/$httpd_only"
> > +			then
> > +				found_apache_command=0
> > +				break
> > +			fi
> > +		done
> > +
> > +		if test $found_apache_command != 0
> > +		then
> > +			alt_apache="httpd"
> > +			if type $alt_apache > /dev/null 2>&1
> > +			then
> > +				$alt_apache -v | grep Apache > /dev/null 2>&1
> > +				if test $? == 0
> > +				then
> > +					httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
> > +				else
> > +					for i in $httpd_parent_paths; do
> > +						if test -x "$i/$alt_apache"
> > +						then
> > +							$i/$alt_apache -v | grep Apache > /dev/null 2>&1
> > +							if test $? == 0
> > +							then
> > +								httpd=`echo "$httpd" | sed "s/apache2/httpd/"`
> > +								break
> > +							fi
> > +						fi
> > +					done
> > +				fi
> > +			fi
> > +		fi
> > +	fi
>
> I'd like to see a cleaner implementation of this logic, 
Maybe you can use a function like this not tested one:
check_cmd() {
	cmd="$1"
	if type "$cmd" > /dev/null 2>&1; then
		if "$cmd" -v | grep Apache > /dev/null 2>&1; then
			httpd=$(echo "$cmd" | sed "s/apache2/httpd/")
		fi
	fi
}

Regards, Christian.

Junio C Hamano· May 12, 2008, 18:42 UTC · re: Christian Couder · lore

Re: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

Christian Couder <chriscool@tuxfamily.org> writes:
Show 11 quoted lines
> Maybe you can use a function like this not tested one:
>
> check_cmd() {
> 	cmd="$1"
>
> 	if type "$cmd" > /dev/null 2>&1; then
> 		if "$cmd" -v | grep Apache > /dev/null 2>&1; then
> 			httpd=$(echo "$cmd" | sed "s/apache2/httpd/")
> 		fi
> 	fi
> }

One worry I have with that approach is if any and all random implementations of "httpd" that live somewhere in path do not do any harm when started with "-v" option. Namely, they should exit without becoming a daemon and/or start the service.

I am not convinced that would be the case.
nathan spindel· May 13, 2008, 02:54 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

On May 12, 2008, at 11:42 AM, Junio C Hamano wrote:
Show 22 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> Maybe you can use a function like this not tested one:
>>
>> check_cmd() {
>> 	cmd="$1"
>>
>> 	if type "$cmd" > /dev/null 2>&1; then
>> 		if "$cmd" -v | grep Apache > /dev/null 2>&1; then
>> 			httpd=$(echo "$cmd" | sed "s/apache2/httpd/")
>> 		fi
>> 	fi
>> }
>
> One worry I have with that approach is if any and all random
> implementations of "httpd" that live somewhere in path do not do any  
> harm
> when started with "-v" option.  Namely, they should exit without  
> becoming
> a daemon and/or start the service.
>
> I am not convinced that would be the case.

I agree with that worry. Solving that in the general case is pretty difficult, so I think we could instead look for other clues in the system. Some ideas:

- Use $HTTPD as defined in apachectl. (How portable is that?)
- Inspect the text of 'man httpd' to determine if it's Apache or not.
- Only fallback on the httpd command on Mac OS X 10.5? That's pretty  
safe.
Jakub Narebski· May 13, 2008, 12:30 UTC · re: nathan spindel · lore

Re: [PATCH v2] instaweb: make it compatible with Mac OS X 10.5's apache installation.

nathan spindel <nathans@gmail.com> writes:
Show 22 quoted lines
> On May 12, 2008, at 11:42 AM, Junio C Hamano wrote:
> 
>> Christian Couder <chriscool@tuxfamily.org> writes:
>>
>>> Maybe you can use a function like this not tested one:
>>>
>>> check_cmd() {
>>> 	cmd="$1"
>>>
>>> 	if type "$cmd"> /dev/null 2>&1; then
>>> 		if "$cmd" -v | grep Apache> /dev/null 2>&1; then
>>> 			httpd=$(echo "$cmd" | sed "s/apache2/httpd/")
>>> 		fi
>>> 	fi
>>> }
>>
>> One worry I have with that approach is if any and all random
>> implementations of "httpd" that live somewhere in path do not do
>> any harm when started with "-v" option.  Namely, they should exit
>> without becoming a daemon and/or start the service.
>>
>> I am not convinced that would be the case.

By the way, it is a bit strange that Apache doesn't understand long equivalent of -v, namely --version.

Show 5 quoted lines
> I agree with that worry. Solving that in the general case is pretty
> difficult, so I think we could instead look for other clues in the
> system. Some ideas:
> 
> - Use $HTTPD as defined in apachectl. (How portable is that?)
You mean what apachectl returns in Usage: (first line)?
  1014:jnareb@roke:~> /usr/sbin/apachectl          
  Usage: /usr/sbin/httpd [-D name] [-d directory] [-f file]
> - Inspect the text of 'man httpd' to determine if it's Apache or not.

Manpages might be not installed. I think 'httpd -v' is less error prone...

> - Only fallback on the httpd command on Mac OS X 10.5? That's pretty
> safe.

Not only MacOS X has Apache installed as httpd binary. Some Linux distributions do that too.

P.S. I wonder if it would be possible, as absolutely last resort fallback, to make git-instaweb to create/use very simple web server in Perl, using HTTP::Daemon module (if it is installed), which comes from libwww-perl.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

← back to recent threads