# [PATCH] Enhanced auto-discovery of httpd location and call conventions. 5 messages from 2008-06-12 to 2008-06-13. Participants: Flavio Poletti (polettix), Jakub Narebski, Flavio Poletti, Eric Wong. Thread: https://gitlist.dev/t/13907 ## Flavio Poletti (polettix), 2008-06-12 01:51 Subject: [PATCH] Enhanced auto-discovery of httpd location and call conventions. Message-ID: <1213235480-21925-1-git-send-email-flavio@polettix.it> URL: https://gitlist.dev/e/1213235480-21925-1-git-send-email-flavio%40polettix.it ``` 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 --- 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 ``` ## Jakub Narebski, 2008-06-12 07:52 Subject: Re: [PATCH] Enhanced auto-discovery of httpd location and call conventions. Message-ID: URL: https://gitlist.dev/e/m3r6b3qedj.fsf%40localhost.localdomain In-Reply-To: <1213235480-21925-1-git-send-email-flavio@polettix.it> ``` "Flavio Poletti (polettix)" 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 ``` ## Flavio Poletti, 2008-06-12 09:59 Subject: [PATCH v2] git-instaweb: Enhanced auto-discovery of httpd and call conventions. Removed backticks. Message-ID: <1213264759-5264-1-git-send-email-flavio@polettix.it> URL: https://gitlist.dev/e/1213264759-5264-1-git-send-email-flavio%40polettix.it In-Reply-To: <1213235480-21925-1-git-send-email-flavio@polettix.it> ``` 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 --- 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" < URL: https://gitlist.dev/e/1213307695-2563-1-git-send-email-flavio%40polettix.it In-Reply-To: <1213264759-5264-1-git-send-email-flavio@polettix.it> ``` 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 --- 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" < URL: https://gitlist.dev/e/20080613170214.GF5966%40yp-box.dyndns.org In-Reply-To: <1213307695-2563-1-git-send-email-flavio@polettix.it> ``` Flavio Poletti 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 Acked-by: Eric Wong > --- > > 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" < -- > 1.5.5.4 > ```