All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Giulio Benetti <giulio.benetti@benettiengineering.com>
Cc: Bernd Kuhls <bernd@kuhls.net>,
	Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>,
	James Autry <jautry@tekvox.com>,
	Matthew Maron <matthewm@tekvox.com>,
	buildroot@buildroot.org, Jim Reinhart <jimr@tekvox.com>
Subject: Re: [Buildroot] [PATCH v3] package/apache: add option BR2_PACKAGE_APACHE_DAEMON
Date: Thu, 21 Sep 2023 22:45:03 +0200	[thread overview]
Message-ID: <20230921204503.GM512384@scaer> (raw)
In-Reply-To: <20230921193111.1136943-1-giulio.benetti@benettiengineering.com>

Giulio, All,

TBH, I am not too fond of this two-in-one package... :-(
However, it is not that ugly, so let's give it a look. See below...

On 2023-09-21 21:31 +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.
> 
> 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
> diff --git a/package/Config.in b/package/Config.in
> index e8dbadadf3..aa5e9b5a98 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -2278,7 +2278,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"

package/modsecurity2/Config.in also has a redundant dependency on
BR2_PACKAGE_APACHE, which is now even less relevant.

>  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

This new config... [0]

[--SNIP--]
> diff --git a/package/apache/apache.mk b/package/apache/apache.mk
> index 320a6ad20e..83df3854a3 100644
> --- a/package/apache/apache.mk
> +++ b/package/apache/apache.mk
[--SNIP--]
> @@ -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

[0] ... means that BR2_PACKAGE_PHP_SAPI_APACHE will have to be made
dependent on it rather than on plain BR2_PACKAGE_APACHE.

Beside modsecurity2 and php, there are no other package that refer to
apache;

    $ git grep -E '(select|depends on) BR2_PACKAGE_APACHE'
    package/modsecurity2/Config.in: depends on BR2_PACKAGE_APACHE
    package/php/Config.in:  depends on BR2_PACKAGE_APACHE

> +APACHE_CONF_OPTS += \
> +	--sysconfdir=/etc/apache2 \
>  	--enable-http \
>  	--enable-dbd \
>  	--enable-proxy \
> @@ -121,5 +125,19 @@ define APACHE_INSTALL_INIT_SYSTEMD
>  	$(INSTALL) -D -m 644 package/apache/apache.service \
>  		$(TARGET_DIR)/usr/lib/systemd/system/apache.service
>  endef
> +else
> +APACHE_CONF_OPTS += \
> +	--with-static-htdigest \
> +	--with-static-htpasswd

Why don't you --disable-http --disable-dbd and so on?

> +define APACHE_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/support htdigest htpasswd

If it is possible to properly disable everything else, then we would
not need custom commands, neither for build...

> +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

... nor for install.

And thus this would look less than two-packages-in-one, and more like
how we handle libcurl, so that would be even better.

Regards,
Yann E. MORIN.

> +endif
>  
>  $(eval $(autotools-package))
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2023-09-21 20:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-21 19:31 [Buildroot] [PATCH v3] package/apache: add option BR2_PACKAGE_APACHE_DAEMON Giulio Benetti
2023-09-21 20:45 ` Yann E. MORIN [this message]
2023-09-22 21:07   ` Giulio Benetti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230921204503.GM512384@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=bernd@kuhls.net \
    --cc=buildroot@buildroot.org \
    --cc=giulio.benetti+tekvox@benettiengineering.com \
    --cc=giulio.benetti@benettiengineering.com \
    --cc=jautry@tekvox.com \
    --cc=jimr@tekvox.com \
    --cc=matthewm@tekvox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.