Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]

On Wed, Nov 08, 2023 at 11:54:26AM -0500, Jeff King wrote:
> 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?

Doesn't hurt to do it.

> 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).

Fair, will do.

> > +if test -x "$DEFAULT_HTTPD_PATH"
> > +then
> > +	DETECTED_HTTPD_ROOT="$("$DEFAULT_HTTPD_PATH" -V | sed -n 's/^ -D HTTPD_ROOT="\(.*\)"$/\1/p')"
> > +fi
> 
> I 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.

Indeed. Also, paths like `/usr/libexec/apache2` wouldn't be possible to
be discovered via `${HTTPD_ROOT}/modules` anyway. It's kind of a shame
that there is no way (or at least no way I know of) to portably discover
the default modules directory used by httpd.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]