Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path
From: Jeff King <hidden>
Date: 2023-11-08 16:54:28
On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:
While it is possible to specify these paths via `LIB_HTTPD_PATH` and
`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer
to figure out how to set those up. And in fact we can do better by
dynamically detecting both httpd and its module path at runtime:
- The httpd binary can be located via PATH.
- The module directory can (in many cases) be derived via the
`HTTPD_ROOT` compile-time variable.
Amend the code to do so. The new runtime-detected paths will only be
used in case none of the hardcoded paths are usable.If these improve detection on your platform, I think that is a good thing and they are worth doing. But as a generic mechanism, I have two comments:
-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' "$(command -v httpd)" do if test -x "$DEFAULT_HTTPD_PATH" then
The binary goes under the name "httpd", but also "apache2". But the PATH search only looks for "httpd". Should we check "command -v apache2" as well? This also means we may run "test -x" on an empty string, but that is probably OK in practice (we could guard it with "test -n", though).
+if test -x "$DEFAULT_HTTPD_PATH"
+then
+ DETECTED_HTTPD_ROOT="$("$DEFAULT_HTTPD_PATH" -V | sed -n 's/^ -D HTTPD_ROOT="\(.*\)"$/\1/p')"
+fiI was really pleased to see this and hoped it could replace the kitchen-sink list of paths in the hunk below. But sadly I think it depends on having a configured apache setup. On my Debian system, for example, I have the "apache2-bin" package installed but not "apache" (because only the latter actually sets up a system apache daemon, which I don't want). And thus there is no config: $ /usr/sbin/apache2 -D HTTPD_ROOT apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory So without a system config file to act as a template for our custom config, I think we are stuck with guessing where the installer might have put them. -Peff