[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Sebastian Kisela]

On Fedora-derived systems, the apache httpd package installs modules
under /usr/lib{,64}/httpd/modules, depending on whether the system is
32- or 64-bit.  A symlink from /etc/httpd/modules is created which
points to the proper module path.  Use it to support apache on Fedora,
CentOS, and Red Hat systems.

Written with assistance of Todd Zullinger <tmz@pobox.com>

Signed-off-by: Sebastian Kisela <skisela@redhat.com>
---
 git-instaweb.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 47e38f34c..e42e58528 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -332,6 +332,8 @@ apache2_conf () {
 			module_path="/usr/lib/httpd/modules"
 		test -d "/usr/lib/apache2/modules" &&
 			module_path="/usr/lib/apache2/modules"
+		test -d "/etc/httpd/modules" &&
+			module_path="/etc/httpd/modules"
 	fi
 	bind=
 	test x"$local" = xtrue && bind='127.0.0.1:'
-- 
2.18.0.99.gd2ee41e0e

[PATCH 2/2] git-instaweb: fix apache2 config with apache >= 2.4

[Sebastian Kisela]

The generated apache2 config fails with apache >= 2.4.  The error log
states:

    AH00136: Server MUST relinquish startup privileges before accepting
    connections.  Please ensure mod_unixd or other system security
    module is loaded.
    AH00016: Configuration Failed

Fix this by loading the unixd module.  This works with older httpd as
well, so no IfVersion conditional is needed.  (Tested with httpd-2.2.15
on CentOS-6.)

Written with assistance of Todd Zullinger <tmz@pobox.com>

Signed-off-by: Sebastian Kisela <skisela@redhat.com>
---
 git-instaweb.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index e42e58528..b1da2c374 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -358,7 +358,7 @@ EOF
 			break
 		fi
 	done
-	for mod in mime dir env log_config authz_core
+	for mod in mime dir env log_config authz_core unixd
 	do
 		if test -e $module_path/mod_${mod}.so
 		then
-- 
2.18.0.99.gd2ee41e0e

Re: [PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Junio C Hamano]

Sebastian Kisela <skisela@redhat.com> writes:

> On Fedora-derived systems, the apache httpd package installs modules
> under /usr/lib{,64}/httpd/modules, depending on whether the system is
> 32- or 64-bit.  A symlink from /etc/httpd/modules is created which
> points to the proper module path.  Use it to support apache on Fedora,
> CentOS, and Red Hat systems.
>
> Written with assistance of Todd Zullinger <tmz@pobox.com>
>
> Signed-off-by: Sebastian Kisela <skisela@redhat.com>
> ---
>  git-instaweb.sh | 2 ++
>  1 file changed, 2 insertions(+)

Thanks for a patch, and welcome to the Git development community.

> diff --git a/git-instaweb.sh b/git-instaweb.sh
> index 47e38f34c..e42e58528 100755
> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -332,6 +332,8 @@ apache2_conf () {
>  			module_path="/usr/lib/httpd/modules"
>  		test -d "/usr/lib/apache2/modules" &&
>  			module_path="/usr/lib/apache2/modules"
> +		test -d "/etc/httpd/modules" &&
> +			module_path="/etc/httpd/modules"

The original already has the same issue with two entries but this
patch makes it even worse by adding yet another one.  The longer the
code follows a bad pattern, the more it encourages future developers
to follow the same bad pattern, and usually the best time to do a
clean up like the following

	if test -z "$module_path"
	then
		for candidate in \
			/etc/httpd \
			/usr/lib/apache2 \
			/usr/lib/httpd \
		do
			if test -d "$candidate/modules"
			then
				module_path="$candidate/modules"
				break
			fi
		done
	fi

is when you go from 2 to 3.  Two points to note are:

 - It would be easier to add the fourth one this way

 - The explicit "break" makes it clear that the paths are listed in
   decreasing order of precedence (i.e. /etc/httpd if exists makes
   /usr/lib/httpd ignored even if the latter exists); the original
   "test -d X && M=X ; test -d Y && M=Y" gives higher precedence to
   the later items but readers need to wonder if it is intended or
   the code is simply being sloppy.

Hope this helps.

Re: [PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 	if test -z "$module_path"
> 	then
> 		for candidate in \
> 			/etc/httpd \
> 			/usr/lib/apache2 \
> 			/usr/lib/httpd \

I obviously missed semicolon here...

> 		do
> 			if test -d "$candidate/modules"
> 			then
> 				module_path="$candidate/modules"
> 				break
> 			fi

One more thing to note is that the fourth candidate might not end
with "/modules" and force us to update these existing three to have
"/modules" at the end and lose appending "/modules" from these two
lines to compensate.  That is sort of deliberate (i.e. as long as we
can share "/modules" as a common substring at the end, it is OK to
take advantage of that).

> 		done
> 	fi
>
> is when you go from 2 to 3.  Two points to note are:
>
>  - It would be easier to add the fourth one this way
>
>  - The explicit "break" makes it clear that the paths are listed in
>    decreasing order of precedence (i.e. /etc/httpd if exists makes
>    /usr/lib/httpd ignored even if the latter exists); the original
>    "test -d X && M=X ; test -d Y && M=Y" gives higher precedence to
>    the later items but readers need to wonder if it is intended or
>    the code is simply being sloppy.
>
> Hope this helps.

[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Sebastian Kisela]

Junio thanks for the tip! Your suggestion definitely looks better.

BTW. I apologize for polluting the git mailing list with the recent
email. I am still new to git-send-email.

[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path

[Sebastian Kisela]

On Fedora-derived systems, the apache httpd package installs modules
under /usr/lib{,64}/httpd/modules, depending on whether the system is
32- or 64-bit.  A symlink from /etc/httpd/modules is created which
points to the proper module path.  Use it to support apache on Fedora,
CentOS, and Red Hat systems.

Written with assistance of Todd Zullinger <tmz@pobox.com> and
Junio C Hamano <gitster@pobox.com>.

Signed-off-by: Sebastian Kisela <skisela@redhat.com>
---
 git-instaweb.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 47e38f34c..675add184 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -326,13 +326,17 @@ EOF
 }
 
 apache2_conf () {
-	if test -z "$module_path"
-	then
-		test -d "/usr/lib/httpd/modules" &&
-			module_path="/usr/lib/httpd/modules"
-		test -d "/usr/lib/apache2/modules" &&
-			module_path="/usr/lib/apache2/modules"
-	fi
+	for candidate in \
+		/etc/httpd \
+		/usr/lib/apache2 \
+		/usr/lib/httpd ;
+	do
+		if test -d "$candidate/modules"
+		then
+			module_path="$candidate/modules"
+			break
+		fi
+	done
 	bind=
 	test x"$local" = xtrue && bind='127.0.0.1:'
 	echo 'text/css css' > "$fqgitdir/mime.types"
-- 
2.14.4