| Message ID | 20230921203237.1249291-1-giulio.benetti@benettiengineering.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v4] package/apache: add option BR2_PACKAGE_APACHE_DAEMON | expand |
Giulio, All, You sent your v4 while I was writing my review of v3, so you could not have adressed my comments. Concurrency in computers is really hard, but is even harder with humans! ;-) This is however a good opportunity to add what I forgot... On 2023-09-21 22:32 +0200, Giulio Benetti spake thusly: > From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com> > > With option BR2_PACKAGE_APACHE_DAEMON disabled only htdigest and htpasswd > are built and installed. By default BR2_PACKAGE_APACHE_DAEMON is enabled > and entire apache daemon is built. This is useful for Mongoose credentials > handling. Your commit log must not describe the commit; it must explain it. Start with the problem you have, and then explain how you fixed it. E.g.: package/apache: add option to disable server Other packages (e.g. mongoose) can use htdigest and htpasswd, but those are only available with apache. We don't want to build the whole apache server just for those tools, so we add an option to disable the server; it is enabled by default for legacy purposes (so that existing (def)configs still work). (adapt the following:) However, there is not way to tell the apache buildsystem to only build those two tools, so we have to provide custom build and install commands; they are statically linked against the apache internal helper libs, so we have nothing to install besides those two executables. About that last part: if --disable-http et al. really do the job, then it is moot, of course: adapt it appropriately. Regards, Yann E. MORIN. > Cc: Jim Reinhart <jimr@tekvox.com> > Cc: James Autry <jautry@tekvox.com> > Cc: Matthew Maron <matthewm@tekvox.com> > Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com> > --- > V1->V2: > * Hide "External Apache modules" if BR2_PACKAGE_APACHE_UTILS_ONLY is enabled > V2->V3: > as suggested by Arnout: > * change negative option BR2_PACKAGE_APACHE_UTILS_ONLY to BR2_PACKAGE_APACHE_DAEMON > * set a common APACHE_CONF_OPTS and only add specific options for > BR2_PACKAGE_APACHE_DAEMON enabled or not > V3->V4: > * drop --with-static-* options as suggested by Arnout > --- > package/Config.in | 2 +- > package/apache/Config.in | 9 +++++++++ > package/apache/apache.mk | 20 +++++++++++++++++--- > 3 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/package/Config.in b/package/Config.in > index cc99be39fb..cd7fe056b8 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -2279,7 +2279,7 @@ menu "Networking applications" > source "package/alfred/Config.in" > source "package/aoetools/Config.in" > source "package/apache/Config.in" > -if BR2_PACKAGE_APACHE > +if BR2_PACKAGE_APACHE_DAEMON > menu "External Apache modules" > source "package/modsecurity2/Config.in" > endmenu > diff --git a/package/apache/Config.in b/package/apache/Config.in > index 270296bce4..5e9e4c5f9d 100644 > --- a/package/apache/Config.in > +++ b/package/apache/Config.in > @@ -17,6 +17,14 @@ config BR2_PACKAGE_APACHE > > if BR2_PACKAGE_APACHE > > +config BR2_PACKAGE_APACHE_DAEMON > + bool "apache-daemon" > + default y > + help > + Provide entire Apache daemon, otherwise only htdigest and htpasswd > + will be built and installed. > + > +if BR2_PACKAGE_APACHE_DAEMON > choice > prompt "Multi-Processing Module (MPM)" > default BR2_PACKAGE_APACHE_MPM_WORKER > @@ -40,6 +48,7 @@ config BR2_PACKAGE_APACHE_MPM_WORKER > Implements a hybrid multi-threaded multi-process web server > > endchoice > +endif > > endif > > diff --git a/package/apache/apache.mk b/package/apache/apache.mk > index 320a6ad20e..994842b455 100644 > --- a/package/apache/apache.mk > +++ b/package/apache/apache.mk > @@ -12,8 +12,6 @@ APACHE_LICENSE_FILES = LICENSE > APACHE_CPE_ID_VENDOR = apache > APACHE_CPE_ID_PRODUCT = http_server > APACHE_SELINUX_MODULES = apache > -# Needed for mod_php > -APACHE_INSTALL_STAGING = YES > # We have a patch touching configure.in and Makefile.in, > # so we need to autoreconf: > APACHE_AUTORECONF = YES > @@ -32,10 +30,16 @@ APACHE_MPM = worker > endif > > APACHE_CONF_OPTS = \ > - --sysconfdir=/etc/apache2 \ > --with-apr=$(STAGING_DIR)/usr \ > --with-apr-util=$(STAGING_DIR)/usr \ > --with-pcre=$(STAGING_DIR)/usr/bin/pcre2-config \ > + > +ifeq ($(BR2_PACKAGE_APACHE_DAEMON),y) > +# Needed for mod_php > +APACHE_INSTALL_STAGING = YES > + > +APACHE_CONF_OPTS += \ > + --sysconfdir=/etc/apache2 \ > --enable-http \ > --enable-dbd \ > --enable-proxy \ > @@ -121,5 +125,15 @@ define APACHE_INSTALL_INIT_SYSTEMD > $(INSTALL) -D -m 644 package/apache/apache.service \ > $(TARGET_DIR)/usr/lib/systemd/system/apache.service > endef > +else > +define APACHE_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/support htdigest htpasswd > +endef > + > +define APACHE_INSTALL_TARGET_CMDS > + $(INSTALL) -m 0755 -D $(@D)/support/htdigest $(TARGET_DIR)/usr/bin/htdigest > + $(INSTALL) -m 0755 -D $(@D)/support/htpasswd $(TARGET_DIR)/usr/bin/htpasswd > +endef > +endif > > $(eval $(autotools-package)) > -- > 2.34.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Yann, All, On 21/09/23 22:56, Yann E. MORIN wrote: > Giulio, All, > > You sent your v4 while I was writing my review of v3, so you could not > have adressed my comments. Concurrency in computers is really hard, but > is even harder with humans! ;-) > > This is however a good opportunity to add what I forgot... > > On 2023-09-21 22:32 +0200, Giulio Benetti spake thusly: >> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com> >> >> With option BR2_PACKAGE_APACHE_DAEMON disabled only htdigest and htpasswd >> are built and installed. By default BR2_PACKAGE_APACHE_DAEMON is enabled >> and entire apache daemon is built. This is useful for Mongoose credentials >> handling. > > Your commit log must not describe the commit; it must explain it. Start > with the problem you have, and then explain how you fixed it. E.g.: > > package/apache: add option to disable server > > Other packages (e.g. mongoose) can use htdigest and htpasswd, but > those are only available with apache. > > We don't want to build the whole apache server just for those tools, > so we add an option to disable the server; it is enabled by default > for legacy purposes (so that existing (def)configs still work). > > (adapt the following:) > > However, there is not way to tell the apache buildsystem to only > build those two tools, so we have to provide custom build and > install commands; they are statically linked against the apache > internal helper libs, so we have nothing to install besides those > two executables. > > About that last part: if --disable-http et al. really do the job, then > it is moot, of course: adapt it appropriately. Thanks a lot for commit log suggestion! Going to send V5(and wait a bit :-)) Best regards
diff --git a/package/Config.in b/package/Config.in index cc99be39fb..cd7fe056b8 100644 --- a/package/Config.in +++ b/package/Config.in @@ -2279,7 +2279,7 @@ menu "Networking applications" source "package/alfred/Config.in" source "package/aoetools/Config.in" source "package/apache/Config.in" -if BR2_PACKAGE_APACHE +if BR2_PACKAGE_APACHE_DAEMON menu "External Apache modules" source "package/modsecurity2/Config.in" endmenu diff --git a/package/apache/Config.in b/package/apache/Config.in index 270296bce4..5e9e4c5f9d 100644 --- a/package/apache/Config.in +++ b/package/apache/Config.in @@ -17,6 +17,14 @@ config BR2_PACKAGE_APACHE if BR2_PACKAGE_APACHE +config BR2_PACKAGE_APACHE_DAEMON + bool "apache-daemon" + default y + help + Provide entire Apache daemon, otherwise only htdigest and htpasswd + will be built and installed. + +if BR2_PACKAGE_APACHE_DAEMON choice prompt "Multi-Processing Module (MPM)" default BR2_PACKAGE_APACHE_MPM_WORKER @@ -40,6 +48,7 @@ config BR2_PACKAGE_APACHE_MPM_WORKER Implements a hybrid multi-threaded multi-process web server endchoice +endif endif diff --git a/package/apache/apache.mk b/package/apache/apache.mk index 320a6ad20e..994842b455 100644 --- a/package/apache/apache.mk +++ b/package/apache/apache.mk @@ -12,8 +12,6 @@ APACHE_LICENSE_FILES = LICENSE APACHE_CPE_ID_VENDOR = apache APACHE_CPE_ID_PRODUCT = http_server APACHE_SELINUX_MODULES = apache -# Needed for mod_php -APACHE_INSTALL_STAGING = YES # We have a patch touching configure.in and Makefile.in, # so we need to autoreconf: APACHE_AUTORECONF = YES @@ -32,10 +30,16 @@ APACHE_MPM = worker endif APACHE_CONF_OPTS = \ - --sysconfdir=/etc/apache2 \ --with-apr=$(STAGING_DIR)/usr \ --with-apr-util=$(STAGING_DIR)/usr \ --with-pcre=$(STAGING_DIR)/usr/bin/pcre2-config \ + +ifeq ($(BR2_PACKAGE_APACHE_DAEMON),y) +# Needed for mod_php +APACHE_INSTALL_STAGING = YES + +APACHE_CONF_OPTS += \ + --sysconfdir=/etc/apache2 \ --enable-http \ --enable-dbd \ --enable-proxy \ @@ -121,5 +125,15 @@ define APACHE_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 package/apache/apache.service \ $(TARGET_DIR)/usr/lib/systemd/system/apache.service endef +else +define APACHE_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/support htdigest htpasswd +endef + +define APACHE_INSTALL_TARGET_CMDS + $(INSTALL) -m 0755 -D $(@D)/support/htdigest $(TARGET_DIR)/usr/bin/htdigest + $(INSTALL) -m 0755 -D $(@D)/support/htpasswd $(TARGET_DIR)/usr/bin/htpasswd +endef +endif $(eval $(autotools-package))