[PATCH] Enhanced auto-discovery of httpd location and call conventions.

[PATCH] Enhanced auto-discovery of httpd location and call conventions.

[Flavio Poletti (polettix)]

This patch allows calling:

   git-instaweb -d apache2

and have the script Do The Right Thing. In particular, the auto-discovery
mechanism has been extended in order to be used for module listing as
well, and the call convention is that if the daemon is apache2/lighttpd
and the parameter to the "-d" option does not end by "-f", the "-f" is
added to the end of the option itself.

Another minor change is a substitution of a shell sub-call using "$()"
by means of backticks. In my understanding, plain /bin/sh does not support
"$()".

Signed-off-by: Flavio Poletti <flavio@polettix.it>
---
Hello,

   this is the first time I submit a patch here so please point me to the
right FM to Read if I've messed up with the standard submission
procedure at http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD

Cheers,

   Flavio.

 git-instaweb.sh |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6f91c8f..cc9d4f3 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -37,11 +37,23 @@ test -z "$httpd" && httpd='lighttpd -f'
 # any untaken local port will do...
 test -z "$port" && port=1234
 
-start_httpd () {
-	httpd_only="`echo $httpd | cut -f1 -d' '`"
+resolve_httpd () {
+	local_httpd=$1
+
+	case "$local_httpd" in
+	*apache2*|*lighttpd*)
+		# ensure that the apache2/lighttpd command ends with "-f"
+		if ! echo "$local_httpd" | grep -- '-f *$' >/dev/null 2>&1
+		then
+			local_httpd="$local_httpd -f"
+		fi
+		;;
+	esac
+
+	httpd_only="`echo $local_httpd | cut -f1 -d' '`"
 	if case "$httpd_only" in /*) : ;; *) which $httpd_only >/dev/null;; esac
 	then
-		$httpd "$fqgitdir/gitweb/httpd.conf"
+		echo $local_httpd
 	else
 		# many httpds are installed in /usr/sbin or /usr/local/sbin
 		# these days and those are not in most users $PATHs
@@ -51,16 +63,19 @@ start_httpd () {
 		do
 			if test -x "$i/$httpd_only"
 			then
-				# don't quote $httpd, there can be
-				# arguments to it (-f)
-				$i/$httpd "$fqgitdir/gitweb/httpd.conf"
+				echo $i/$local_httpd
 				return
 			fi
 		done
-		echo "$httpd_only not found. Install $httpd_only or use" \
+		echo >&2 "$httpd_only not found. Install $httpd_only or use" \
 		     "--httpd to specify another http daemon."
 		exit 1
 	fi
+}
+
+start_httpd () {
+	# don't quote $httpd, there can be arguments to it (-f)
+	$httpd "$fqgitdir/gitweb/httpd.conf"
 	if test $? != 0; then
 		echo "Could not execute http daemon $httpd."
 		exit 1
@@ -71,6 +86,7 @@ stop_httpd () {
 	test -f "$fqgitdir/pid" && kill `cat "$fqgitdir/pid"`
 }
 
+httpd=`resolve_httpd "$httpd"`
 while test $# != 0
 do
 	case "$1" in
@@ -92,7 +108,7 @@ do
 		;;
 	-d|--httpd)
 		shift
-		httpd="$1"
+		httpd=`resolve_httpd "$1"`
 		;;
 	-b|--browser)
 		shift
@@ -228,7 +244,7 @@ EOF
 }
 
 script='
-s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
+s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'`dirname "$fqgitdir"`'";#;
 s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
 s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
 s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
-- 
1.5.5.3

Re: [PATCH] Enhanced auto-discovery of httpd location and call conventions.

[Jakub Narebski]

"Flavio Poletti (polettix)" <flavio@polettix.it> writes:

> Another minor change is a substitution of a shell sub-call using "$()"
> by means of backticks. In my understanding, plain /bin/sh does not support
> "$()".

But POSIX shell does.  And git does make use of it rather extensively.

Besides, see Documentation/CodingGuidelines:

  For shell scripts specifically (not exhaustive):

   - We prefer $( ... ) for command substitution; unlike ``, it
     properly nests.  It should have been the way Bourne spelled
     it from day one, but unfortunately isn't.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

[PATCH v2] git-instaweb: Enhanced auto-discovery of httpd and call conventions. Removed backticks.

[Flavio Poletti]

This patch allows calling:

   git-instaweb -d apache2

and have the script Do The Right Thing. In particular, the auto-discovery
mechanism has been extended in order to be used for module listing as
well, and the call convention is that if the daemon is apache2/lighttpd
and the parameter to the "-d" option does not end by "-f", the "-f" is
added to the end of the option itself.

Changed all backticks to $( ... ) as per Documentation/CodingGuidelines:

   For shell scripts specifically (not exhaustive):

      - We prefer $( ... ) for command substitution; unlike ``, it
         properly nests.  It should have been the way Bourne spelled
         it from day one, but unfortunately isn't.

The rationale is that the POSIX shell does support $( ... ), and git
does make use of it rather extensively. (Thanks to Jakub Narebski for
pointing this out).

Signed-off-by: Flavio Poletti <flavio@polettix.it>
---
Following Jakub's observation about backticks, I sanitised the whole
script to use $( ... ) instead of backticks. I hope that bundling it
with the proposed patch can be acceptable in this case; if not, I can
produce two different patches and send them separately.

Cheers,

   Flavio.
 git-instaweb.sh |   46 +++++++++++++++++++++++++++++++---------------
 1 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6f91c8f..ec22344 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -22,10 +22,10 @@ restart        restart the web server
 . git-sh-setup
 
 fqgitdir="$GIT_DIR"
-local="`git config --bool --get instaweb.local`"
-httpd="`git config --get instaweb.httpd`"
-port=`git config --get instaweb.port`
-module_path="`git config --get instaweb.modulepath`"
+local="$(git config --bool --get instaweb.local)"
+httpd="$(git config --get instaweb.httpd)"
+port=$(git config --get instaweb.port)
+module_path="$(git config --get instaweb.modulepath)"
 
 conf="$GIT_DIR/gitweb/httpd.conf"
 
@@ -37,11 +37,23 @@ test -z "$httpd" && httpd='lighttpd -f'
 # any untaken local port will do...
 test -z "$port" && port=1234
 
-start_httpd () {
-	httpd_only="`echo $httpd | cut -f1 -d' '`"
+resolve_httpd () {
+	local_httpd=$1
+
+	case "$local_httpd" in
+	*apache2*|*lighttpd*)
+		# ensure that the apache2/lighttpd command ends with "-f"
+		if ! echo "$local_httpd" | grep -- '-f *$' >/dev/null 2>&1
+		then
+			local_httpd="$local_httpd -f"
+		fi
+		;;
+	esac
+
+	httpd_only="$(echo $local_httpd | cut -f1 -d' ')"
 	if case "$httpd_only" in /*) : ;; *) which $httpd_only >/dev/null;; esac
 	then
-		$httpd "$fqgitdir/gitweb/httpd.conf"
+		echo $local_httpd
 	else
 		# many httpds are installed in /usr/sbin or /usr/local/sbin
 		# these days and those are not in most users $PATHs
@@ -51,16 +63,19 @@ start_httpd () {
 		do
 			if test -x "$i/$httpd_only"
 			then
-				# don't quote $httpd, there can be
-				# arguments to it (-f)
-				$i/$httpd "$fqgitdir/gitweb/httpd.conf"
+				echo $i/$local_httpd
 				return
 			fi
 		done
-		echo "$httpd_only not found. Install $httpd_only or use" \
+		echo >&2 "$httpd_only not found. Install $httpd_only or use" \
 		     "--httpd to specify another http daemon."
 		exit 1
 	fi
+}
+
+start_httpd () {
+	# don't quote $httpd, there can be arguments to it (-f)
+	$httpd "$fqgitdir/gitweb/httpd.conf"
 	if test $? != 0; then
 		echo "Could not execute http daemon $httpd."
 		exit 1
@@ -68,9 +83,10 @@ start_httpd () {
 }
 
 stop_httpd () {
-	test -f "$fqgitdir/pid" && kill `cat "$fqgitdir/pid"`
+	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
 }
 
+httpd=$(resolve_httpd "$httpd")
 while test $# != 0
 do
 	case "$1" in
@@ -92,7 +108,7 @@ do
 		;;
 	-d|--httpd)
 		shift
-		httpd="$1"
+		httpd=$(resolve_httpd "$1")
 		;;
 	-b|--browser)
 		shift
@@ -116,7 +132,7 @@ do
 done
 
 mkdir -p "$GIT_DIR/gitweb/tmp"
-GIT_EXEC_PATH="`git --exec-path`"
+GIT_EXEC_PATH="$(git --exec-path)"
 GIT_DIR="$fqgitdir"
 export GIT_EXEC_PATH GIT_DIR
 
@@ -215,7 +231,7 @@ PerlPassEnv GIT_EXEC_DIR
 EOF
 	else
 		# plain-old CGI
-		list_mods=`echo "$httpd" | sed "s/-f$/-l/"`
+		list_mods=$(echo "$httpd" | sed "s/-f$/-l/")
 		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
 		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
 		cat >> "$conf" <<EOF
-- 
1.5.5.3

[PATCH v3] git-instaweb: Enhanced auto-discovery of httpd and call conventions. Removed backticks.

[Flavio Poletti]

This patch allows calling:

   git-instaweb -d apache2

and have the script Do The Right Thing. In particular, the auto-discovery
mechanism has been extended in order to be used for module listing as
well, and the call convention is that if the daemon is apache2/lighttpd
and the parameter to the "-d" option does not end by "-f", the "-f" is
added to the end of the option itself.

Changed all backticks to $( ... ) as per Documentation/CodingGuidelines:

   For shell scripts specifically (not exhaustive):

      - We prefer $( ... ) for command substitution; unlike ``, it
         properly nests.  It should have been the way Bourne spelled
         it from day one, but unfortunately isn't.

The rationale is that the POSIX shell does support $( ... ), and git
does make use of it rather extensively. (Thanks to Jakub Narebski for
pointing this out).

Signed-off-by: Flavio Poletti <flavio@polettix.it>
---

Sorry for the traffic, but I noticed that a stray warning was emitted
due to a last-second change (which teaches me once more that those
"innocent" last-second it-cannot-spoil-anything changes are the
most poisonus snakes in programming.

 git-instaweb.sh |   48 +++++++++++++++++++++++++++++++++---------------
 1 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6f91c8f..af0fde5 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -22,10 +22,10 @@ restart        restart the web server
 . git-sh-setup
 
 fqgitdir="$GIT_DIR"
-local="`git config --bool --get instaweb.local`"
-httpd="`git config --get instaweb.httpd`"
-port=`git config --get instaweb.port`
-module_path="`git config --get instaweb.modulepath`"
+local="$(git config --bool --get instaweb.local)"
+httpd="$(git config --get instaweb.httpd)"
+port=$(git config --get instaweb.port)
+module_path="$(git config --get instaweb.modulepath)"
 
 conf="$GIT_DIR/gitweb/httpd.conf"
 
@@ -37,11 +37,21 @@ test -z "$httpd" && httpd='lighttpd -f'
 # any untaken local port will do...
 test -z "$port" && port=1234
 
-start_httpd () {
-	httpd_only="`echo $httpd | cut -f1 -d' '`"
+resolve_full_httpd () {
+	case "$httpd" in
+	*apache2*|*lighttpd*)
+		# ensure that the apache2/lighttpd command ends with "-f"
+		if ! echo "$httpd" | grep -- '-f *$' >/dev/null 2>&1
+		then
+			httpd="$httpd -f"
+		fi
+		;;
+	esac
+
+	httpd_only="$(echo $httpd | cut -f1 -d' ')"
 	if case "$httpd_only" in /*) : ;; *) which $httpd_only >/dev/null;; esac
 	then
-		$httpd "$fqgitdir/gitweb/httpd.conf"
+		full_httpd=$httpd
 	else
 		# many httpds are installed in /usr/sbin or /usr/local/sbin
 		# these days and those are not in most users $PATHs
@@ -51,16 +61,23 @@ start_httpd () {
 		do
 			if test -x "$i/$httpd_only"
 			then
-				# don't quote $httpd, there can be
-				# arguments to it (-f)
-				$i/$httpd "$fqgitdir/gitweb/httpd.conf"
+				full_httpd=$i/$httpd
 				return
 			fi
 		done
-		echo "$httpd_only not found. Install $httpd_only or use" \
-		     "--httpd to specify another http daemon."
+
+		echo >&2 "$httpd_only not found. Install $httpd_only or use" \
+		     "--httpd to specify another httpd daemon."
 		exit 1
 	fi
+}
+
+start_httpd () {
+	# here $httpd should have a meaningful value
+	resolve_full_httpd
+
+	# don't quote $full_httpd, there can be arguments to it (-f)
+	$full_httpd "$fqgitdir/gitweb/httpd.conf"
 	if test $? != 0; then
 		echo "Could not execute http daemon $httpd."
 		exit 1
@@ -68,7 +85,7 @@ start_httpd () {
 }
 
 stop_httpd () {
-	test -f "$fqgitdir/pid" && kill `cat "$fqgitdir/pid"`
+	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
 }
 
 while test $# != 0
@@ -116,7 +133,7 @@ do
 done
 
 mkdir -p "$GIT_DIR/gitweb/tmp"
-GIT_EXEC_PATH="`git --exec-path`"
+GIT_EXEC_PATH="$(git --exec-path)"
 GIT_DIR="$fqgitdir"
 export GIT_EXEC_PATH GIT_DIR
 
@@ -215,7 +232,8 @@ PerlPassEnv GIT_EXEC_DIR
 EOF
 	else
 		# plain-old CGI
-		list_mods=`echo "$httpd" | sed "s/-f$/-l/"`
+		resolve_full_httpd
+		list_mods=$(echo "$full_httpd" | sed "s/-f$/-l/")
 		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
 		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
 		cat >> "$conf" <<EOF
-- 
1.5.5.4

Re: [PATCH v3] git-instaweb: Enhanced auto-discovery of httpd and call conventions. Removed backticks.

[Eric Wong]

Flavio Poletti <flavio@polettix.it> wrote:
> This patch allows calling:
> 
>    git-instaweb -d apache2
> 
> and have the script Do The Right Thing. In particular, the auto-discovery
> mechanism has been extended in order to be used for module listing as
> well, and the call convention is that if the daemon is apache2/lighttpd
> and the parameter to the "-d" option does not end by "-f", the "-f" is
> added to the end of the option itself.
> 
> Changed all backticks to $( ... ) as per Documentation/CodingGuidelines:
> 
>    For shell scripts specifically (not exhaustive):
> 
>       - We prefer $( ... ) for command substitution; unlike ``, it
>          properly nests.  It should have been the way Bourne spelled
>          it from day one, but unfortunately isn't.
> 
> The rationale is that the POSIX shell does support $( ... ), and git
> does make use of it rather extensively. (Thanks to Jakub Narebski for
> pointing this out).
> 
> Signed-off-by: Flavio Poletti <flavio@polettix.it>

Acked-by: Eric Wong <normalperson@yhbt.net>

> ---
> 
> Sorry for the traffic, but I noticed that a stray warning was emitted
> due to a last-second change (which teaches me once more that those
> "innocent" last-second it-cannot-spoil-anything changes are the
> most poisonus snakes in programming.
> 
>  git-instaweb.sh |   48 +++++++++++++++++++++++++++++++++---------------
>  1 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/git-instaweb.sh b/git-instaweb.sh
> index 6f91c8f..af0fde5 100755
> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -22,10 +22,10 @@ restart        restart the web server
>  . git-sh-setup
>  
>  fqgitdir="$GIT_DIR"
> -local="`git config --bool --get instaweb.local`"
> -httpd="`git config --get instaweb.httpd`"
> -port=`git config --get instaweb.port`
> -module_path="`git config --get instaweb.modulepath`"
> +local="$(git config --bool --get instaweb.local)"
> +httpd="$(git config --get instaweb.httpd)"
> +port=$(git config --get instaweb.port)
> +module_path="$(git config --get instaweb.modulepath)"
>  
>  conf="$GIT_DIR/gitweb/httpd.conf"
>  
> @@ -37,11 +37,21 @@ test -z "$httpd" && httpd='lighttpd -f'
>  # any untaken local port will do...
>  test -z "$port" && port=1234
>  
> -start_httpd () {
> -	httpd_only="`echo $httpd | cut -f1 -d' '`"
> +resolve_full_httpd () {
> +	case "$httpd" in
> +	*apache2*|*lighttpd*)
> +		# ensure that the apache2/lighttpd command ends with "-f"
> +		if ! echo "$httpd" | grep -- '-f *$' >/dev/null 2>&1
> +		then
> +			httpd="$httpd -f"
> +		fi
> +		;;
> +	esac
> +
> +	httpd_only="$(echo $httpd | cut -f1 -d' ')"
>  	if case "$httpd_only" in /*) : ;; *) which $httpd_only >/dev/null;; esac
>  	then
> -		$httpd "$fqgitdir/gitweb/httpd.conf"
> +		full_httpd=$httpd
>  	else
>  		# many httpds are installed in /usr/sbin or /usr/local/sbin
>  		# these days and those are not in most users $PATHs
> @@ -51,16 +61,23 @@ start_httpd () {
>  		do
>  			if test -x "$i/$httpd_only"
>  			then
> -				# don't quote $httpd, there can be
> -				# arguments to it (-f)
> -				$i/$httpd "$fqgitdir/gitweb/httpd.conf"
> +				full_httpd=$i/$httpd
>  				return
>  			fi
>  		done
> -		echo "$httpd_only not found. Install $httpd_only or use" \
> -		     "--httpd to specify another http daemon."
> +
> +		echo >&2 "$httpd_only not found. Install $httpd_only or use" \
> +		     "--httpd to specify another httpd daemon."
>  		exit 1
>  	fi
> +}
> +
> +start_httpd () {
> +	# here $httpd should have a meaningful value
> +	resolve_full_httpd
> +
> +	# don't quote $full_httpd, there can be arguments to it (-f)
> +	$full_httpd "$fqgitdir/gitweb/httpd.conf"
>  	if test $? != 0; then
>  		echo "Could not execute http daemon $httpd."
>  		exit 1
> @@ -68,7 +85,7 @@ start_httpd () {
>  }
>  
>  stop_httpd () {
> -	test -f "$fqgitdir/pid" && kill `cat "$fqgitdir/pid"`
> +	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
>  }
>  
>  while test $# != 0
> @@ -116,7 +133,7 @@ do
>  done
>  
>  mkdir -p "$GIT_DIR/gitweb/tmp"
> -GIT_EXEC_PATH="`git --exec-path`"
> +GIT_EXEC_PATH="$(git --exec-path)"
>  GIT_DIR="$fqgitdir"
>  export GIT_EXEC_PATH GIT_DIR
>  
> @@ -215,7 +232,8 @@ PerlPassEnv GIT_EXEC_DIR
>  EOF
>  	else
>  		# plain-old CGI
> -		list_mods=`echo "$httpd" | sed "s/-f$/-l/"`
> +		resolve_full_httpd
> +		list_mods=$(echo "$full_httpd" | sed "s/-f$/-l/")
>  		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
>  		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
>  		cat >> "$conf" <<EOF
> -- 
> 1.5.5.4
>