{"thread":{"id":"49059","subject":"[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","startedAt":"2018-08-07T07:25:54Z","lastAt":"2018-08-08T08:51:11Z","messageCount":6,"participants":["Sebastian Kisela","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"354708","messageId":"20180807072548.12764-1-skisela@redhat.com","threadId":"49059","inReplyTo":null,"subject":"[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","fromName":"Sebastian Kisela","fromEmail":"skisela@redhat.com","sentAt":"2018-08-07T07:25:47Z","receivedAt":"2018-08-07T07:25:54Z","isPatch":true,"sender":{"key":"skisela@redhat.com","avatar":"https://avatars.githubusercontent.com/u/26842433?v=4"},"body":"On Fedora-derived systems, the apache httpd package installs modules\nunder /usr/lib{,64}/httpd/modules, depending on whether the system is\n32- or 64-bit. A symlink from /etc/httpd/modules is created which\npoints to the proper module path. Use it to support apache on Fedora,\nCentOS, and Red Hat systems.\n\nWritten with assistance of Todd Zullinger \n\nSigned-off-by: Sebastian Kisela \n---\n git-instaweb.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 47e38f34c..e42e58528 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -332,6 +332,8 @@ apache2_conf () {\n \t\t\tmodule_path=\"/usr/lib/httpd/modules\"\n \t\ttest -d \"/usr/lib/apache2/modules\" &&\n \t\t\tmodule_path=\"/usr/lib/apache2/modules\"\n+\t\ttest -d \"/etc/httpd/modules\" &&\n+\t\t\tmodule_path=\"/etc/httpd/modules\"\n \tfi\n \tbind=\n \ttest x\"$local\" = xtrue && bind='127.0.0.1:'\n-- \n2.18.0.99.gd2ee41e0e\n\n"},{"id":"354709","messageId":"20180807072548.12764-2-skisela@redhat.com","threadId":"49059","inReplyTo":"20180807072548.12764-1-skisela@redhat.com","subject":"[PATCH 2/2] git-instaweb: fix apache2 config with apache >= 2.4","fromName":"Sebastian Kisela","fromEmail":"skisela@redhat.com","sentAt":"2018-08-07T07:25:48Z","receivedAt":"2018-08-07T07:25:56Z","isPatch":true,"sender":{"key":"skisela@redhat.com","avatar":"https://avatars.githubusercontent.com/u/26842433?v=4"},"body":"The generated apache2 config fails with apache >= 2.4. The error log\nstates:\n\n AH00136: Server MUST relinquish startup privileges before accepting\n connections. Please ensure mod_unixd or other system security\n module is loaded.\n AH00016: Configuration Failed\n\nFix this by loading the unixd module. This works with older httpd as\nwell, so no IfVersion conditional is needed. (Tested with httpd-2.2.15\non CentOS-6.)\n\nWritten with assistance of Todd Zullinger \n\nSigned-off-by: Sebastian Kisela \n---\n git-instaweb.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex e42e58528..b1da2c374 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -358,7 +358,7 @@ EOF\n \t\t\tbreak\n \t\tfi\n \tdone\n-\tfor mod in mime dir env log_config authz_core\n+\tfor mod in mime dir env log_config authz_core unixd\n \tdo\n \t\tif test -e $module_path/mod_${mod}.so\n \t\tthen\n-- \n2.18.0.99.gd2ee41e0e\n\n"},{"id":"354773","messageId":"xmqqk1p1kgwq.fsf@gitster-ct.c.googlers.com","threadId":"49059","inReplyTo":"20180807072548.12764-1-skisela@redhat.com","subject":"Re: [PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-07T21:25:57Z","receivedAt":"2018-08-07T21:26:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Kisela writes:\n\n> On Fedora-derived systems, the apache httpd package installs modules\n> under /usr/lib{,64}/httpd/modules, depending on whether the system is\n> 32- or 64-bit. A symlink from /etc/httpd/modules is created which\n> points to the proper module path. Use it to support apache on Fedora,\n> CentOS, and Red Hat systems.\n>\n> Written with assistance of Todd Zullinger \n>\n> Signed-off-by: Sebastian Kisela \n> ---\n> git-instaweb.sh | 2 ++\n> 1 file changed, 2 insertions(+)\n\nThanks for a patch, and welcome to the Git development community.\n\n> diff --git a/git-instaweb.sh b/git-instaweb.sh\n> index 47e38f34c..e42e58528 100755\n> --- a/git-instaweb.sh\n> +++ b/git-instaweb.sh\n> @@ -332,6 +332,8 @@ apache2_conf () {\n> \t\t\tmodule_path=\"/usr/lib/httpd/modules\"\n> \t\ttest -d \"/usr/lib/apache2/modules\" &&\n> \t\t\tmodule_path=\"/usr/lib/apache2/modules\"\n> +\t\ttest -d \"/etc/httpd/modules\" &&\n> +\t\t\tmodule_path=\"/etc/httpd/modules\"\n\nThe original already has the same issue with two entries but this\npatch makes it even worse by adding yet another one. The longer the\ncode follows a bad pattern, the more it encourages future developers\nto follow the same bad pattern, and usually the best time to do a\nclean up like the following\n\n\tif test -z \"$module_path\"\n\tthen\n\t\tfor candidate in \\\n\t\t\t/etc/httpd \\\n\t\t\t/usr/lib/apache2 \\\n\t\t\t/usr/lib/httpd \\\n\t\tdo\n\t\t\tif test -d \"$candidate/modules\"\n\t\t\tthen\n\t\t\t\tmodule_path=\"$candidate/modules\"\n\t\t\t\tbreak\n\t\t\tfi\n\t\tdone\n\tfi\n\nis when you go from 2 to 3. Two points to note are:\n\n - It would be easier to add the fourth one this way\n\n - The explicit \"break\" makes it clear that the paths are listed in\n decreasing order of precedence (i.e. /etc/httpd if exists makes\n /usr/lib/httpd ignored even if the latter exists); the original\n \"test -d X && M=X ; test -d Y && M=Y\" gives higher precedence to\n the later items but readers need to wonder if it is intended or\n the code is simply being sloppy.\n\nHope this helps.\n\n\n\n"},{"id":"354776","messageId":"xmqqftzpkesb.fsf@gitster-ct.c.googlers.com","threadId":"49059","inReplyTo":"xmqqk1p1kgwq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-07T22:11:48Z","receivedAt":"2018-08-07T22:11:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \tif test -z \"$module_path\"\n> \tthen\n> \t\tfor candidate in \\\n> \t\t\t/etc/httpd \\\n> \t\t\t/usr/lib/apache2 \\\n> \t\t\t/usr/lib/httpd \\\n\nI obviously missed semicolon here...\n\n> \t\tdo\n> \t\t\tif test -d \"$candidate/modules\"\n> \t\t\tthen\n> \t\t\t\tmodule_path=\"$candidate/modules\"\n> \t\t\t\tbreak\n> \t\t\tfi\n\nOne more thing to note is that the fourth candidate might not end\nwith \"/modules\" and force us to update these existing three to have\n\"/modules\" at the end and lose appending \"/modules\" from these two\nlines to compensate. That is sort of deliberate (i.e. as long as we\ncan share \"/modules\" as a common substring at the end, it is OK to\ntake advantage of that).\n\n> \t\tdone\n> \tfi\n>\n> is when you go from 2 to 3. Two points to note are:\n>\n> - It would be easier to add the fourth one this way\n>\n> - The explicit \"break\" makes it clear that the paths are listed in\n> decreasing order of precedence (i.e. /etc/httpd if exists makes\n> /usr/lib/httpd ignored even if the latter exists); the original\n> \"test -d X && M=X ; test -d Y && M=Y\" gives higher precedence to\n> the later items but readers need to wonder if it is intended or\n> the code is simply being sloppy.\n>\n> Hope this helps.\n"},{"id":"354791","messageId":"20180808084918.3424-1-skisela@redhat.com","threadId":"49059","inReplyTo":"xmqqftzpkesb.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","fromName":"Sebastian Kisela","fromEmail":"skisela@redhat.com","sentAt":"2018-08-08T08:49:17Z","receivedAt":"2018-08-08T08:51:10Z","isPatch":true,"sender":{"key":"skisela@redhat.com","avatar":"https://avatars.githubusercontent.com/u/26842433?v=4"},"body":"Junio thanks for the tip! Your suggestion definitely looks better.\n\nBTW. I apologize for polluting the git mailing list with the recent\nemail. I am still new to git-send-email.\n\n"},{"id":"354792","messageId":"20180808084918.3424-2-skisela@redhat.com","threadId":"49059","inReplyTo":"20180808084918.3424-1-skisela@redhat.com","subject":"[PATCH 1/2] git-instaweb: support Fedora/Red Hat apache module path","fromName":"Sebastian Kisela","fromEmail":"skisela@redhat.com","sentAt":"2018-08-08T08:49:18Z","receivedAt":"2018-08-08T08:51:11Z","isPatch":true,"sender":{"key":"skisela@redhat.com","avatar":"https://avatars.githubusercontent.com/u/26842433?v=4"},"body":"On Fedora-derived systems, the apache httpd package installs modules\nunder /usr/lib{,64}/httpd/modules, depending on whether the system is\n32- or 64-bit. A symlink from /etc/httpd/modules is created which\npoints to the proper module path. Use it to support apache on Fedora,\nCentOS, and Red Hat systems.\n\nWritten with assistance of Todd Zullinger and\nJunio C Hamano .\n\nSigned-off-by: Sebastian Kisela \n---\n git-instaweb.sh | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 47e38f34c..675add184 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -326,13 +326,17 @@ EOF\n }\n \n apache2_conf () {\n-\tif test -z \"$module_path\"\n-\tthen\n-\t\ttest -d \"/usr/lib/httpd/modules\" &&\n-\t\t\tmodule_path=\"/usr/lib/httpd/modules\"\n-\t\ttest -d \"/usr/lib/apache2/modules\" &&\n-\t\t\tmodule_path=\"/usr/lib/apache2/modules\"\n-\tfi\n+\tfor candidate in \\\n+\t\t/etc/httpd \\\n+\t\t/usr/lib/apache2 \\\n+\t\t/usr/lib/httpd ;\n+\tdo\n+\t\tif test -d \"$candidate/modules\"\n+\t\tthen\n+\t\t\tmodule_path=\"$candidate/modules\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone\n \tbind=\n \ttest x\"$local\" = xtrue && bind='127.0.0.1:'\n \techo 'text/css css' > \"$fqgitdir/mime.types\"\n-- \n2.14.4\n\n"}]}