[1/3] buildprocess: Add extra metadata to meta-* files

Message ID 20210423161534.32738-2-robin.roevens@disroot.org
State Superseded
Headers
Series Pakfile metadata enhancements |

Commit Message

Robin Roevens April 23, 2021, 4:15 p.m. UTC
  * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
* Add a Summary and InitScripts field to the meta-* addon files
  containing the value's of those macro's.
* Replace the INSTALL_INITSCRIPT makefile macro by a new
  INSTALL_INITSCRIPTS macro/method that will install all initscripts
  defined in the new INITSCRIPTS macro.
* Adapt libvirt and zabbix_agentd pak as examples of how to use this.

Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
---
 lfs/Config        | 8 ++++++--
 lfs/libvirt       | 6 ++++--
 lfs/zabbix_agentd | 5 ++++-
 src/pakfire/meta  | 2 ++
 4 files changed, 16 insertions(+), 5 deletions(-)
  

Comments

Jonatan Schlag May 12, 2021, 6:49 p.m. UTC | #1
Hi,

> Am 23.04.2021 um 18:16 schrieb Robin Roevens <robin.roevens@disroot.org>:
> 
> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
> * Add a Summary and InitScripts field to the meta-* addon files
>  containing the value's of those macro's.
> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>  INSTALL_INITSCRIPTS macro/method that will install all initscripts
>  defined in the new INITSCRIPTS macro.
> * Adapt libvirt and zabbix_agentd pak as examples of how to use this.
> 
> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> ---
> lfs/Config        | 8 ++++++--
> lfs/libvirt       | 6 ++++--
> lfs/zabbix_agentd | 5 ++++-
> src/pakfire/meta  | 2 ++
> 4 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/lfs/Config b/lfs/Config
> index eadbbc408..61d6f0c82 100644
> --- a/lfs/Config
> +++ b/lfs/Config
> @@ -299,15 +299,19 @@ define PAK
>    # Create meta file
>    sed \
>        -e "s/NAME/$(PROG)/g" \
> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>        -e "s/VER/$(VER)/g" \
>        -e "s/RELEASE/$(PAK_VER)/g" \
>        -e "s/DEPS/$(DEPS)/g" \
>        -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>      < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
> endef
> 
> -define INSTALL_INITSCRIPT
> -    install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)  /etc/rc.d/init.d/$(1)
> +define INSTALL_INITSCRIPTS
> +    for initscript in $(INITSCRIPTS); do \
> +        install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript  /etc/rc.d/init.d/$$initscript; \
> +    done
> endef
> 
> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
> diff --git a/lfs/libvirt b/lfs/libvirt
> index 28a95d317..be5d3db3a 100644
> --- a/lfs/libvirt
> +++ b/lfs/libvirt
> @@ -25,6 +25,7 @@
> include Config
> 
> VER        = 6.5.0
> +SUMMARY       = Server side daemon and supporting files for libvirt
> 
> THISAPP    = libvirt-$(VER)
> DL_FILE    = $(THISAPP).tar.xz
> @@ -37,6 +38,8 @@ PAK_VER    = 25
> 
> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> 
> +INITSCRIPTS= libvirtd virtlogd
> +
> ###############################################################################
> # Top-level Rules
> ###############################################################################
> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>    cd $(DIR_APP)/build_libvirt && make install
> 
>    #install initscripts
> -    $(call INSTALL_INITSCRIPT,libvirtd)
> -    $(call INSTALL_INITSCRIPT,virtlogd)
> +    @$(INSTALL_INITSCRIPTS)
>    mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
And here my approach maybe breaks :-). How could we handle this? It is not a daemon, more something like a startup/shutdown scripts for machines... (And should not appear in the service.cgi). So parsing the root file may yield wrong results and your way would be the better one. 

Jonatan
> 
>    # Backup
> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> index c69643a54..a72fe024b 100644
> --- a/lfs/zabbix_agentd
> +++ b/lfs/zabbix_agentd
> @@ -25,6 +25,7 @@
> include Config
> 
> VER        = 4.2.6
> +SUMMARY    = Zabbix Agent
> 
> THISAPP    = zabbix-$(VER)
> DL_FILE    = $(THISAPP).tar.gz
> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> PAK_VER    = 4
> DEPS       =
> 
> +INITSCRIPTS= zabbix_agentd
> +
> ###############################################################################
> # Top-level Rules
> ###############################################################################
> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>    chown zabbix.zabbix /var/run/zabbix
> 
>    # Install initscripts
> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> +    @$(INSTALL_INITSCRIPTS)
> 
>    # Install sudoers include file
>    install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> diff --git a/src/pakfire/meta b/src/pakfire/meta
> index d97b2a0fa..849b9cd6c 100644
> --- a/src/pakfire/meta
> +++ b/src/pakfire/meta
> @@ -1,6 +1,8 @@
> Name: NAME
> +Summary: SUMMARY
> ProgVersion: VER
> Release: RELEASE
> Size: SIZE
> Dependencies: DEPS
> File: NAME-VER-RELEASE.ipfire
> +InitScripts: INITSCRIPTS
> -- 
> 2.31.1
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
  
Michael Tremer May 14, 2021, 12:23 p.m. UTC | #2
Hello,

> On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
> * Add a Summary and InitScripts field to the meta-* addon files
>  containing the value's of those macro's.
> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>  INSTALL_INITSCRIPTS macro/method that will install all initscripts
>  defined in the new INITSCRIPTS macro.
> * Adapt libvirt and zabbix_agentd pak as examples of how to use this.
> 
> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> ---
> lfs/Config        | 8 ++++++--
> lfs/libvirt       | 6 ++++--
> lfs/zabbix_agentd | 5 ++++-
> src/pakfire/meta  | 2 ++
> 4 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/lfs/Config b/lfs/Config
> index eadbbc408..61d6f0c82 100644
> --- a/lfs/Config
> +++ b/lfs/Config
> @@ -299,15 +299,19 @@ define PAK
> 	# Create meta file
> 	sed \
> 		-e "s/NAME/$(PROG)/g" \
> +		-e "s/SUMMARY/$(SUMMARY)/g" \
> 		-e "s/VER/$(VER)/g" \
> 		-e "s/RELEASE/$(PAK_VER)/g" \
> 		-e "s/DEPS/$(DEPS)/g" \
> 		-e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
> +		-e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> 	  < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
> endef
> 
> -define INSTALL_INITSCRIPT
> -	install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)  /etc/rc.d/init.d/$(1)
> +define INSTALL_INITSCRIPTS
> +	for initscript in $(INITSCRIPTS); do \
> +		install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript  /etc/rc.d/init.d/$$initscript; \
> +	done
> endef

This won’t abort any more if there install command failed. The exit code is ignored and the loop will continue with its next iteration.

You could add “|| exit 1” after the install command. There should be plenty of examples in the code where this has been used.

> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
> diff --git a/lfs/libvirt b/lfs/libvirt
> index 28a95d317..be5d3db3a 100644
> --- a/lfs/libvirt
> +++ b/lfs/libvirt
> @@ -25,6 +25,7 @@
> include Config
> 
> VER        = 6.5.0
> +SUMMARY	   = Server side daemon and supporting files for libvirt
> 
> THISAPP    = libvirt-$(VER)
> DL_FILE    = $(THISAPP).tar.xz
> @@ -37,6 +38,8 @@ PAK_VER    = 25
> 
> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> 
> +INITSCRIPTS= libvirtd virtlogd
> +
> ###############################################################################
> # Top-level Rules
> ###############################################################################
> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> 	cd $(DIR_APP)/build_libvirt && make install
> 
> 	#install initscripts
> -	$(call INSTALL_INITSCRIPT,libvirtd)
> -	$(call INSTALL_INITSCRIPT,virtlogd)
> +	@$(INSTALL_INITSCRIPTS)
> 	mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
> 
> 	# Backup
> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> index c69643a54..a72fe024b 100644
> --- a/lfs/zabbix_agentd
> +++ b/lfs/zabbix_agentd
> @@ -25,6 +25,7 @@
> include Config
> 
> VER        = 4.2.6
> +SUMMARY    = Zabbix Agent
> 
> THISAPP    = zabbix-$(VER)
> DL_FILE    = $(THISAPP).tar.gz
> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> PAK_VER    = 4
> DEPS       =
> 
> +INITSCRIPTS= zabbix_agentd
> +
> ###############################################################################
> # Top-level Rules
> ###############################################################################
> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> 	chown zabbix.zabbix /var/run/zabbix
> 
> 	# Install initscripts
> -	$(call INSTALL_INITSCRIPT,zabbix_agentd)
> +	@$(INSTALL_INITSCRIPTS)
> 
> 	# Install sudoers include file
> 	install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> diff --git a/src/pakfire/meta b/src/pakfire/meta
> index d97b2a0fa..849b9cd6c 100644
> --- a/src/pakfire/meta
> +++ b/src/pakfire/meta
> @@ -1,6 +1,8 @@
> Name: NAME
> +Summary: SUMMARY
> ProgVersion: VER
> Release: RELEASE
> Size: SIZE
> Dependencies: DEPS
> File: NAME-VER-RELEASE.ipfire
> +InitScripts: INITSCRIPTS
> -- 
> 2.31.1

Also, what happens with all the other files that used to call “INSTALL_INITSCRIPT”?

-Michael

> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
  
Michael Tremer May 14, 2021, 12:24 p.m. UTC | #3
Hello Jonatan,

Yes I get your point. Just because a package has more than one initscript does not mean they both should be listed in services.cgi.

It is a weird corner case, but how do we want to handle this?

-Michael

> On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag@ipfire.org> wrote:
> 
> Hi,
> 
>> Am 23.04.2021 um 18:16 schrieb Robin Roevens <robin.roevens@disroot.org>:
>> 
>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
>> * Add a Summary and InitScripts field to the meta-* addon files
>> containing the value's of those macro's.
>> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>> INSTALL_INITSCRIPTS macro/method that will install all initscripts
>> defined in the new INITSCRIPTS macro.
>> * Adapt libvirt and zabbix_agentd pak as examples of how to use this.
>> 
>> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
>> ---
>> lfs/Config        | 8 ++++++--
>> lfs/libvirt       | 6 ++++--
>> lfs/zabbix_agentd | 5 ++++-
>> src/pakfire/meta  | 2 ++
>> 4 files changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/lfs/Config b/lfs/Config
>> index eadbbc408..61d6f0c82 100644
>> --- a/lfs/Config
>> +++ b/lfs/Config
>> @@ -299,15 +299,19 @@ define PAK
>>   # Create meta file
>>   sed \
>>       -e "s/NAME/$(PROG)/g" \
>> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>>       -e "s/VER/$(VER)/g" \
>>       -e "s/RELEASE/$(PAK_VER)/g" \
>>       -e "s/DEPS/$(DEPS)/g" \
>>       -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
>> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>     < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
>> endef
>> 
>> -define INSTALL_INITSCRIPT
>> -    install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)  /etc/rc.d/init.d/$(1)
>> +define INSTALL_INITSCRIPTS
>> +    for initscript in $(INITSCRIPTS); do \
>> +        install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript  /etc/rc.d/init.d/$$initscript; \
>> +    done
>> endef
>> 
>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
>> diff --git a/lfs/libvirt b/lfs/libvirt
>> index 28a95d317..be5d3db3a 100644
>> --- a/lfs/libvirt
>> +++ b/lfs/libvirt
>> @@ -25,6 +25,7 @@
>> include Config
>> 
>> VER        = 6.5.0
>> +SUMMARY       = Server side daemon and supporting files for libvirt
>> 
>> THISAPP    = libvirt-$(VER)
>> DL_FILE    = $(THISAPP).tar.xz
>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>> 
>> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
>> 
>> +INITSCRIPTS= libvirtd virtlogd
>> +
>> ###############################################################################
>> # Top-level Rules
>> ###############################################################################
>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>   cd $(DIR_APP)/build_libvirt && make install
>> 
>>   #install initscripts
>> -    $(call INSTALL_INITSCRIPT,libvirtd)
>> -    $(call INSTALL_INITSCRIPT,virtlogd)
>> +    @$(INSTALL_INITSCRIPTS)
>>   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
> And here my approach maybe breaks :-). How could we handle this? It is not a daemon, more something like a startup/shutdown scripts for machines... (And should not appear in the service.cgi). So parsing the root file may yield wrong results and your way would be the better one. 
> 
> Jonatan
>> 
>>   # Backup
>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>> index c69643a54..a72fe024b 100644
>> --- a/lfs/zabbix_agentd
>> +++ b/lfs/zabbix_agentd
>> @@ -25,6 +25,7 @@
>> include Config
>> 
>> VER        = 4.2.6
>> +SUMMARY    = Zabbix Agent
>> 
>> THISAPP    = zabbix-$(VER)
>> DL_FILE    = $(THISAPP).tar.gz
>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>> PAK_VER    = 4
>> DEPS       =
>> 
>> +INITSCRIPTS= zabbix_agentd
>> +
>> ###############################################################################
>> # Top-level Rules
>> ###############################################################################
>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>   chown zabbix.zabbix /var/run/zabbix
>> 
>>   # Install initscripts
>> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
>> +    @$(INSTALL_INITSCRIPTS)
>> 
>>   # Install sudoers include file
>>   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>> index d97b2a0fa..849b9cd6c 100644
>> --- a/src/pakfire/meta
>> +++ b/src/pakfire/meta
>> @@ -1,6 +1,8 @@
>> Name: NAME
>> +Summary: SUMMARY
>> ProgVersion: VER
>> Release: RELEASE
>> Size: SIZE
>> Dependencies: DEPS
>> File: NAME-VER-RELEASE.ipfire
>> +InitScripts: INITSCRIPTS
>> -- 
>> 2.31.1
>> 
>> 
>> -- 
>> Dit bericht is gescanned op virussen en andere gevaarlijke
>> inhoud door MailScanner en lijkt schoon te zijn.
  
Robin Roevens May 14, 2021, 8:07 p.m. UTC | #4
Hi

Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
> Hello Jonatan,
> 
> Yes I get your point. Just because a package has more than one
> initscript does not mean they both should be listed in services.cgi.
> 
> It is a weird corner case, but how do we want to handle this?
> 
> -Michael

Maybe a slight renaming of my proposed new variable INITSCRIPTS to
SERVICES in the LFS files would make it a bit more clear that this
concerns only service-initscripts?

The INSTALL_INITSCRIPTS macro could then be renamed to
INSTALL_SERVICE_INITSCRIPTS.

If we then also keep the original INSTALL_INITSCRIPT macro, that macro
can  then be used to install non-service initscripts if required. Or
even to install all required initscripts in the occasion that you want
to skip some service-initscripts defined in SERVICES because they are
included in the source, like the situation Jonatan pointed out (in this
case the SERVICES variable is filled with the sole purpose to serve as
metadata).

So this would then result in:
---
define INSTALL_INITSCRIPT
   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
/etc/rc.d/init.d/$(1)
endef

define INSTALL_SERVICE_INITSCRIPTS
   for service in $(SERVICES); do \
      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
   done
endef
---

Optionally we could also add again a variable INITSCRIPTS with the
purpose to list non-service initscripts. so
SERVICES - for service-initscripts
INITSCRIPTS - for non-service-initscripts

and in the meta-* files:
---
Name: NAME
Summary: SUMMARY
ProgVersion: VER
Release: RELEASE
Size: SIZE
Dependencies: DEPS
File: NAME-VER-RELEASE.ipfire
Services: SERVICES
InitScripts: INITSCRIPTS
---

Maybe that info could also come in handy sooner or later ?

In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
back to INSTALL_INITSCRIPTS, now accepting a parameter which is a list
of initscripts to install, so one could call:
---
$(call INSTALL_INITSCRIPTS,$(SERVICES))
$(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
---
to install all initscripts.

Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS which
would then install both SERVICES and INITSCRIPTS (or even replace the
above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a macro to
install all initscripts (SERVICES and INITSCRIPTS) and a macro to
install a single initscript in case some of the ones listed in SERVICES
and/or INITSCRIPTS need to be skipped..

But I think the INSTALL_INITSCRIPTS which takes a list of services to
install as parameter is probably the most generic one, giving you the
possibility to install all SERVICES in one go, and then individually
install only those INITSCRIPTS not included in the source if
INITSCRIPTS contains such initscript(s) or vice versa.

Robin

> 
> > On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag@ipfire.org>
> > wrote:
> > 
> > Hi,
> > 
> > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > robin.roevens@disroot.org>:
> > > 
> > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
> > > * Add a Summary and InitScripts field to the meta-* addon files
> > > containing the value's of those macro's.
> > > * Replace the INSTALL_INITSCRIPT makefile macro by a new
> > > INSTALL_INITSCRIPTS macro/method that will install all initscripts
> > > defined in the new INITSCRIPTS macro.
> > > * Adapt libvirt and zabbix_agentd pak as examples of how to use
> > > this.
> > > 
> > > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > > ---
> > > lfs/Config        | 8 ++++++--
> > > lfs/libvirt       | 6 ++++--
> > > lfs/zabbix_agentd | 5 ++++-
> > > src/pakfire/meta  | 2 ++
> > > 4 files changed, 16 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/lfs/Config b/lfs/Config
> > > index eadbbc408..61d6f0c82 100644
> > > --- a/lfs/Config
> > > +++ b/lfs/Config
> > > @@ -299,15 +299,19 @@ define PAK
> > >   # Create meta file
> > >   sed \
> > >       -e "s/NAME/$(PROG)/g" \
> > > +        -e "s/SUMMARY/$(SUMMARY)/g" \
> > >       -e "s/VER/$(VER)/g" \
> > >       -e "s/RELEASE/$(PAK_VER)/g" \
> > >       -e "s/DEPS/$(DEPS)/g" \
> > >       -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-
> > > $(VER)-$(PAK_VER).ipfire)/g" \
> > > +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> > >     < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
> > > endef
> > > 
> > > -define INSTALL_INITSCRIPT
> > > -    install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > /etc/rc.d/init.d/$(1)
> > > +define INSTALL_INITSCRIPTS
> > > +    for initscript in $(INITSCRIPTS); do \
> > > +        install -m 754 -v
> > > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > > /etc/rc.d/init.d/$$initscript; \
> > > +    done
> > > endef
> > > 
> > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
> > > diff --git a/lfs/libvirt b/lfs/libvirt
> > > index 28a95d317..be5d3db3a 100644
> > > --- a/lfs/libvirt
> > > +++ b/lfs/libvirt
> > > @@ -25,6 +25,7 @@
> > > include Config
> > > 
> > > VER        = 6.5.0
> > > +SUMMARY       = Server side daemon and supporting files for
> > > libvirt
> > > 
> > > THISAPP    = libvirt-$(VER)
> > > DL_FILE    = $(THISAPP).tar.xz
> > > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > > 
> > > DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> > > 
> > > +INITSCRIPTS= libvirtd virtlogd
> > > +
> > > ###################################################################
> > > ############
> > > # Top-level Rules
> > > ###################################################################
> > > ############
> > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > > %,$(DIR_DL)/%,$(objects))
> > >   cd $(DIR_APP)/build_libvirt && make install
> > > 
> > >   #install initscripts
> > > -    $(call INSTALL_INITSCRIPT,libvirtd)
> > > -    $(call INSTALL_INITSCRIPT,virtlogd)
> > > +    @$(INSTALL_INITSCRIPTS)
> > >   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
> > And here my approach maybe breaks :-). How could we handle this? It
> > is not a daemon, more something like a startup/shutdown scripts for
> > machines... (And should not appear in the service.cgi). So parsing
> > the root file may yield wrong results and your way would be the
> > better one. 
> > 
> > Jonatan
> > > 
> > >   # Backup
> > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > index c69643a54..a72fe024b 100644
> > > --- a/lfs/zabbix_agentd
> > > +++ b/lfs/zabbix_agentd
> > > @@ -25,6 +25,7 @@
> > > include Config
> > > 
> > > VER        = 4.2.6
> > > +SUMMARY    = Zabbix Agent
> > > 
> > > THISAPP    = zabbix-$(VER)
> > > DL_FILE    = $(THISAPP).tar.gz
> > > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > > PAK_VER    = 4
> > > DEPS       =
> > > 
> > > +INITSCRIPTS= zabbix_agentd
> > > +
> > > ###################################################################
> > > ############
> > > # Top-level Rules
> > > ###################################################################
> > > ############
> > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > > %,$(DIR_DL)/%,$(objects))
> > >   chown zabbix.zabbix /var/run/zabbix
> > > 
> > >   # Install initscripts
> > > -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > > +    @$(INSTALL_INITSCRIPTS)
> > > 
> > >   # Install sudoers include file
> > >   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > > index d97b2a0fa..849b9cd6c 100644
> > > --- a/src/pakfire/meta
> > > +++ b/src/pakfire/meta
> > > @@ -1,6 +1,8 @@
> > > Name: NAME
> > > +Summary: SUMMARY
> > > ProgVersion: VER
> > > Release: RELEASE
> > > Size: SIZE
> > > Dependencies: DEPS
> > > File: NAME-VER-RELEASE.ipfire
> > > +InitScripts: INITSCRIPTS
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > > -- 
> > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > inhoud door MailScanner en lijkt schoon te zijn.
> 
>
  
Robin Roevens May 14, 2021, 8:16 p.m. UTC | #5
Hi

Michael Tremer schreef op vr 14-05-2021 om 13:23 [+0100]:
> Hello,
> 
> > On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens@disroot.org>
> > wrote:
> > 
> > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
> > * Add a Summary and InitScripts field to the meta-* addon files
> >  containing the value's of those macro's.
> > * Replace the INSTALL_INITSCRIPT makefile macro by a new
> >  INSTALL_INITSCRIPTS macro/method that will install all initscripts
> >  defined in the new INITSCRIPTS macro.
> > * Adapt libvirt and zabbix_agentd pak as examples of how to use
> > this.
> > 
> > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > ---
> > lfs/Config        | 8 ++++++--
> > lfs/libvirt       | 6 ++++--
> > lfs/zabbix_agentd | 5 ++++-
> > src/pakfire/meta  | 2 ++
> > 4 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lfs/Config b/lfs/Config
> > index eadbbc408..61d6f0c82 100644
> > --- a/lfs/Config
> > +++ b/lfs/Config
> > @@ -299,15 +299,19 @@ define PAK
> >         # Create meta file
> >         sed \
> >                 -e "s/NAME/$(PROG)/g" \
> > +               -e "s/SUMMARY/$(SUMMARY)/g" \
> >                 -e "s/VER/$(VER)/g" \
> >                 -e "s/RELEASE/$(PAK_VER)/g" \
> >                 -e "s/DEPS/$(DEPS)/g" \
> >                 -e "s/SIZE/$$(stat --format=%s
> > /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
> > +               -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> >           < /usr/src/src/pakfire/meta > /install/packages/meta-
> > $(PROG)
> > endef
> > 
> > -define INSTALL_INITSCRIPT
> > -       install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > /etc/rc.d/init.d/$(1)
> > +define INSTALL_INITSCRIPTS
> > +       for initscript in $(INITSCRIPTS); do \
> > +               install -m 754 -v
> > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > /etc/rc.d/init.d/$$initscript; \
> > +       done
> > endef
> 
> This won’t abort any more if there install command failed. The exit
> code is ignored and the loop will continue with its next iteration.
> 
> You could add “|| exit 1” after the install command. There should be
> plenty of examples in the code where this has been used.
> 

Good point. will add that.

> > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
> > diff --git a/lfs/libvirt b/lfs/libvirt
> > index 28a95d317..be5d3db3a 100644
> > --- a/lfs/libvirt
> > +++ b/lfs/libvirt
> > @@ -25,6 +25,7 @@
> > include Config
> > 
> > VER        = 6.5.0
> > +SUMMARY           = Server side daemon and supporting files for
> > libvirt
> > 
> > THISAPP    = libvirt-$(VER)
> > DL_FILE    = $(THISAPP).tar.xz
> > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > 
> > DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> > 
> > +INITSCRIPTS= libvirtd virtlogd
> > +
> > ###################################################################
> > ############
> > # Top-level Rules
> > ###################################################################
> > ############
> > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > %,$(DIR_DL)/%,$(objects))
> >         cd $(DIR_APP)/build_libvirt && make install
> > 
> >         #install initscripts
> > -       $(call INSTALL_INITSCRIPT,libvirtd)
> > -       $(call INSTALL_INITSCRIPT,virtlogd)
> > +       @$(INSTALL_INITSCRIPTS)
> >         mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
> > guests
> > 
> >         # Backup
> > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > index c69643a54..a72fe024b 100644
> > --- a/lfs/zabbix_agentd
> > +++ b/lfs/zabbix_agentd
> > @@ -25,6 +25,7 @@
> > include Config
> > 
> > VER        = 4.2.6
> > +SUMMARY    = Zabbix Agent
> > 
> > THISAPP    = zabbix-$(VER)
> > DL_FILE    = $(THISAPP).tar.gz
> > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > PAK_VER    = 4
> > DEPS       =
> > 
> > +INITSCRIPTS= zabbix_agentd
> > +
> > ###################################################################
> > ############
> > # Top-level Rules
> > ###################################################################
> > ############
> > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > %,$(DIR_DL)/%,$(objects))
> >         chown zabbix.zabbix /var/run/zabbix
> > 
> >         # Install initscripts
> > -       $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > +       @$(INSTALL_INITSCRIPTS)
> > 
> >         # Install sudoers include file
> >         install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > index d97b2a0fa..849b9cd6c 100644
> > --- a/src/pakfire/meta
> > +++ b/src/pakfire/meta
> > @@ -1,6 +1,8 @@
> > Name: NAME
> > +Summary: SUMMARY
> > ProgVersion: VER
> > Release: RELEASE
> > Size: SIZE
> > Dependencies: DEPS
> > File: NAME-VER-RELEASE.ipfire
> > +InitScripts: INITSCRIPTS
> > -- 
> > 2.31.1
> 
> Also, what happens with all the other files that used to call
> “INSTALL_INITSCRIPT”?
> 

What other files call INSTALL_INITSCRIPT ? Is it not only called from
the lfs files?
The intention is of course, in the final patch-set to adapt all lfs
files to use the INITSCRIPTS variable and call INSTALL_INITSCIPTS
instead of individual calls to INSTALL_INITSCRIPT. But I did not yet
want to do this, since my idea could also be shot down or drastically
adapted, making that work obsolete. (already proven by my new
SERVICES/INITSCRIPTS proposal in my previous mail). SO I currently only
included 2 examples (zabbix_agentd and libvirt) of the changes to the
lfs files involved.
If that is what you meant ?

Regards
Robin

> -Michael
> 
> > -- 
> > Dit bericht is gescanned op virussen en andere gevaarlijke
> > inhoud door MailScanner en lijkt schoon te zijn.
> > 
> 
>
  
Michael Tremer May 18, 2021, 11:12 a.m. UTC | #6
Hello,

> On 14 May 2021, at 21:16, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi
> 
> Michael Tremer schreef op vr 14-05-2021 om 13:23 [+0100]:
>> Hello,
>> 
>>> On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens@disroot.org>
>>> wrote:
>>> 
>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
>>> * Add a Summary and InitScripts field to the meta-* addon files
>>>  containing the value's of those macro's.
>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>>>  INSTALL_INITSCRIPTS macro/method that will install all initscripts
>>>  defined in the new INITSCRIPTS macro.
>>> * Adapt libvirt and zabbix_agentd pak as examples of how to use
>>> this.
>>> 
>>> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
>>> ---
>>> lfs/Config        | 8 ++++++--
>>> lfs/libvirt       | 6 ++++--
>>> lfs/zabbix_agentd | 5 ++++-
>>> src/pakfire/meta  | 2 ++
>>> 4 files changed, 16 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/lfs/Config b/lfs/Config
>>> index eadbbc408..61d6f0c82 100644
>>> --- a/lfs/Config
>>> +++ b/lfs/Config
>>> @@ -299,15 +299,19 @@ define PAK
>>>         # Create meta file
>>>         sed \
>>>                 -e "s/NAME/$(PROG)/g" \
>>> +               -e "s/SUMMARY/$(SUMMARY)/g" \
>>>                 -e "s/VER/$(VER)/g" \
>>>                 -e "s/RELEASE/$(PAK_VER)/g" \
>>>                 -e "s/DEPS/$(DEPS)/g" \
>>>                 -e "s/SIZE/$$(stat --format=%s
>>> /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
>>> +               -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>>           < /usr/src/src/pakfire/meta > /install/packages/meta-
>>> $(PROG)
>>> endef
>>> 
>>> -define INSTALL_INITSCRIPT
>>> -       install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
>>> /etc/rc.d/init.d/$(1)
>>> +define INSTALL_INITSCRIPTS
>>> +       for initscript in $(INITSCRIPTS); do \
>>> +               install -m 754 -v
>>> $(DIR_SRC)/src/initscripts/packages/$$initscript 
>>> /etc/rc.d/init.d/$$initscript; \
>>> +       done
>>> endef
>> 
>> This won’t abort any more if there install command failed. The exit
>> code is ignored and the loop will continue with its next iteration.
>> 
>> You could add “|| exit 1” after the install command. There should be
>> plenty of examples in the code where this has been used.
>> 
> 
> Good point. will add that.
> 
>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
>>> diff --git a/lfs/libvirt b/lfs/libvirt
>>> index 28a95d317..be5d3db3a 100644
>>> --- a/lfs/libvirt
>>> +++ b/lfs/libvirt
>>> @@ -25,6 +25,7 @@
>>> include Config
>>> 
>>> VER        = 6.5.0
>>> +SUMMARY           = Server side daemon and supporting files for
>>> libvirt
>>> 
>>> THISAPP    = libvirt-$(VER)
>>> DL_FILE    = $(THISAPP).tar.xz
>>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>>> 
>>> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
>>> 
>>> +INITSCRIPTS= libvirtd virtlogd
>>> +
>>> ###################################################################
>>> ############
>>> # Top-level Rules
>>> ###################################################################
>>> ############
>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
>>> %,$(DIR_DL)/%,$(objects))
>>>         cd $(DIR_APP)/build_libvirt && make install
>>> 
>>>         #install initscripts
>>> -       $(call INSTALL_INITSCRIPT,libvirtd)
>>> -       $(call INSTALL_INITSCRIPT,virtlogd)
>>> +       @$(INSTALL_INITSCRIPTS)
>>>         mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
>>> guests
>>> 
>>>         # Backup
>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>>> index c69643a54..a72fe024b 100644
>>> --- a/lfs/zabbix_agentd
>>> +++ b/lfs/zabbix_agentd
>>> @@ -25,6 +25,7 @@
>>> include Config
>>> 
>>> VER        = 4.2.6
>>> +SUMMARY    = Zabbix Agent
>>> 
>>> THISAPP    = zabbix-$(VER)
>>> DL_FILE    = $(THISAPP).tar.gz
>>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>>> PAK_VER    = 4
>>> DEPS       =
>>> 
>>> +INITSCRIPTS= zabbix_agentd
>>> +
>>> ###################################################################
>>> ############
>>> # Top-level Rules
>>> ###################################################################
>>> ############
>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
>>> %,$(DIR_DL)/%,$(objects))
>>>         chown zabbix.zabbix /var/run/zabbix
>>> 
>>>         # Install initscripts
>>> -       $(call INSTALL_INITSCRIPT,zabbix_agentd)
>>> +       @$(INSTALL_INITSCRIPTS)
>>> 
>>>         # Install sudoers include file
>>>         install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
>>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>>> index d97b2a0fa..849b9cd6c 100644
>>> --- a/src/pakfire/meta
>>> +++ b/src/pakfire/meta
>>> @@ -1,6 +1,8 @@
>>> Name: NAME
>>> +Summary: SUMMARY
>>> ProgVersion: VER
>>> Release: RELEASE
>>> Size: SIZE
>>> Dependencies: DEPS
>>> File: NAME-VER-RELEASE.ipfire
>>> +InitScripts: INITSCRIPTS
>>> -- 
>>> 2.31.1
>> 
>> Also, what happens with all the other files that used to call
>> “INSTALL_INITSCRIPT”?
>> 
> 
> What other files call INSTALL_INITSCRIPT ? Is it not only called from
> the lfs files?
> The intention is of course, in the final patch-set to adapt all lfs
> files to use the INITSCRIPTS variable and call INSTALL_INITSCIPTS
> instead of individual calls to INSTALL_INITSCRIPT. But I did not yet
> want to do this, since my idea could also be shot down or drastically
> adapted, making that work obsolete. (already proven by my new
> SERVICES/INITSCRIPTS proposal in my previous mail). SO I currently only
> included 2 examples (zabbix_agentd and libvirt) of the changes to the
> lfs files involved.
> If that is what you meant ?

Okay, that wasn’t clear to me, but this is what I was referring to.

INSTALL_INITSCRIPTS is called 56 times and you want to change it, all those files would need to be changed, too.

However, let’s sort out Jonatan’s point first and see what a good solution would be.

Best,
-Michael

> 
> Regards
> Robin
> 
>> -Michael
>> 
>>> -- 
>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>> inhoud door MailScanner en lijkt schoon te zijn.
>>> 
>> 
>> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
  
Michael Tremer May 18, 2021, 3:09 p.m. UTC | #7
> On 14 May 2021, at 21:07, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi
> 
> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
>> Hello Jonatan,
>> 
>> Yes I get your point. Just because a package has more than one
>> initscript does not mean they both should be listed in services.cgi.
>> 
>> It is a weird corner case, but how do we want to handle this?
>> 
>> -Michael
> 
> Maybe a slight renaming of my proposed new variable INITSCRIPTS to
> SERVICES in the LFS files would make it a bit more clear that this
> concerns only service-initscripts?
> 
> The INSTALL_INITSCRIPTS macro could then be renamed to
> INSTALL_SERVICE_INITSCRIPTS.
> 
> If we then also keep the original INSTALL_INITSCRIPT macro, that macro
> can  then be used to install non-service initscripts if required. Or
> even to install all required initscripts in the occasion that you want
> to skip some service-initscripts defined in SERVICES because they are
> included in the source, like the situation Jonatan pointed out (in this
> case the SERVICES variable is filled with the sole purpose to serve as
> metadata).
> 
> So this would then result in:
> ---
> define INSTALL_INITSCRIPT
>   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> /etc/rc.d/init.d/$(1)
> endef
> 
> define INSTALL_SERVICE_INITSCRIPTS
>   for service in $(SERVICES); do \
>      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
>   done
> endef
> ---
> 
> Optionally we could also add again a variable INITSCRIPTS with the
> purpose to list non-service initscripts. so
> SERVICES - for service-initscripts
> INITSCRIPTS - for non-service-initscripts
> 
> and in the meta-* files:
> ---
> Name: NAME
> Summary: SUMMARY
> ProgVersion: VER
> Release: RELEASE
> Size: SIZE
> Dependencies: DEPS
> File: NAME-VER-RELEASE.ipfire
> Services: SERVICES
> InitScripts: INITSCRIPTS
> ---
> 
> Maybe that info could also come in handy sooner or later ?

This sounds rather complicated to me.

What are our objectives here? Does Zabbix want/need to start/stop services? And if so, why?

> In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
> back to INSTALL_INITSCRIPTS, now accepting a parameter which is a list
> of initscripts to install, so one could call:
> ---
> $(call INSTALL_INITSCRIPTS,$(SERVICES))
> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
> ---
> to install all initscripts.
> 
> Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS which
> would then install both SERVICES and INITSCRIPTS (or even replace the
> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a macro to
> install all initscripts (SERVICES and INITSCRIPTS) and a macro to
> install a single initscript in case some of the ones listed in SERVICES
> and/or INITSCRIPTS need to be skipped..
> 
> But I think the INSTALL_INITSCRIPTS which takes a list of services to
> install as parameter is probably the most generic one, giving you the
> possibility to install all SERVICES in one go, and then individually
> install only those INITSCRIPTS not included in the source if
> INITSCRIPTS contains such initscript(s) or vice versa.
> 
> Robin
> 
>> 
>>> On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag@ipfire.org>
>>> wrote:
>>> 
>>> Hi,
>>> 
>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens <
>>>> robin.roevens@disroot.org>:
>>>> 
>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles.
>>>> * Add a Summary and InitScripts field to the meta-* addon files
>>>> containing the value's of those macro's.
>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>>>> INSTALL_INITSCRIPTS macro/method that will install all initscripts
>>>> defined in the new INITSCRIPTS macro.
>>>> * Adapt libvirt and zabbix_agentd pak as examples of how to use
>>>> this.
>>>> 
>>>> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
>>>> ---
>>>> lfs/Config        | 8 ++++++--
>>>> lfs/libvirt       | 6 ++++--
>>>> lfs/zabbix_agentd | 5 ++++-
>>>> src/pakfire/meta  | 2 ++
>>>> 4 files changed, 16 insertions(+), 5 deletions(-)
>>>> 
>>>> diff --git a/lfs/Config b/lfs/Config
>>>> index eadbbc408..61d6f0c82 100644
>>>> --- a/lfs/Config
>>>> +++ b/lfs/Config
>>>> @@ -299,15 +299,19 @@ define PAK
>>>>   # Create meta file
>>>>   sed \
>>>>       -e "s/NAME/$(PROG)/g" \
>>>> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>>>>       -e "s/VER/$(VER)/g" \
>>>>       -e "s/RELEASE/$(PAK_VER)/g" \
>>>>       -e "s/DEPS/$(DEPS)/g" \
>>>>       -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-
>>>> $(VER)-$(PAK_VER).ipfire)/g" \
>>>> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>>>     < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
>>>> endef
>>>> 
>>>> -define INSTALL_INITSCRIPT
>>>> -    install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>> /etc/rc.d/init.d/$(1)
>>>> +define INSTALL_INITSCRIPTS
>>>> +    for initscript in $(INITSCRIPTS); do \
>>>> +        install -m 754 -v
>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript 
>>>> /etc/rc.d/init.d/$$initscript; \
>>>> +    done
>>>> endef
>>>> 
>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
>>>> diff --git a/lfs/libvirt b/lfs/libvirt
>>>> index 28a95d317..be5d3db3a 100644
>>>> --- a/lfs/libvirt
>>>> +++ b/lfs/libvirt
>>>> @@ -25,6 +25,7 @@
>>>> include Config
>>>> 
>>>> VER        = 6.5.0
>>>> +SUMMARY       = Server side daemon and supporting files for
>>>> libvirt
>>>> 
>>>> THISAPP    = libvirt-$(VER)
>>>> DL_FILE    = $(THISAPP).tar.xz
>>>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>>>> 
>>>> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
>>>> 
>>>> +INITSCRIPTS= libvirtd virtlogd
>>>> +
>>>> ###################################################################
>>>> ############
>>>> # Top-level Rules
>>>> ###################################################################
>>>> ############
>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
>>>> %,$(DIR_DL)/%,$(objects))
>>>>   cd $(DIR_APP)/build_libvirt && make install
>>>> 
>>>>   #install initscripts
>>>> -    $(call INSTALL_INITSCRIPT,libvirtd)
>>>> -    $(call INSTALL_INITSCRIPT,virtlogd)
>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
>>> And here my approach maybe breaks :-). How could we handle this? It
>>> is not a daemon, more something like a startup/shutdown scripts for
>>> machines... (And should not appear in the service.cgi). So parsing
>>> the root file may yield wrong results and your way would be the
>>> better one. 
>>> 
>>> Jonatan
>>>> 
>>>>   # Backup
>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>>>> index c69643a54..a72fe024b 100644
>>>> --- a/lfs/zabbix_agentd
>>>> +++ b/lfs/zabbix_agentd
>>>> @@ -25,6 +25,7 @@
>>>> include Config
>>>> 
>>>> VER        = 4.2.6
>>>> +SUMMARY    = Zabbix Agent
>>>> 
>>>> THISAPP    = zabbix-$(VER)
>>>> DL_FILE    = $(THISAPP).tar.gz
>>>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>>>> PAK_VER    = 4
>>>> DEPS       =
>>>> 
>>>> +INITSCRIPTS= zabbix_agentd
>>>> +
>>>> ###################################################################
>>>> ############
>>>> # Top-level Rules
>>>> ###################################################################
>>>> ############
>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
>>>> %,$(DIR_DL)/%,$(objects))
>>>>   chown zabbix.zabbix /var/run/zabbix
>>>> 
>>>>   # Install initscripts
>>>> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
>>>> +    @$(INSTALL_INITSCRIPTS)
>>>> 
>>>>   # Install sudoers include file
>>>>   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>>>> index d97b2a0fa..849b9cd6c 100644
>>>> --- a/src/pakfire/meta
>>>> +++ b/src/pakfire/meta
>>>> @@ -1,6 +1,8 @@
>>>> Name: NAME
>>>> +Summary: SUMMARY
>>>> ProgVersion: VER
>>>> Release: RELEASE
>>>> Size: SIZE
>>>> Dependencies: DEPS
>>>> File: NAME-VER-RELEASE.ipfire
>>>> +InitScripts: INITSCRIPTS
>>>> -- 
>>>> 2.31.1
>>>> 
>>>> 
>>>> -- 
>>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>>> inhoud door MailScanner en lijkt schoon te zijn.
>> 
>> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
  
Robin Roevens May 24, 2021, 8:23 p.m. UTC | #8
Hi Michael

Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
> 
> 
> > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens@disroot.org>
> > wrote:
> > 
> > Hi
> > 
> > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
> > > Hello Jonatan,
> > > 
> > > Yes I get your point. Just because a package has more than one
> > > initscript does not mean they both should be listed in
> > > services.cgi.
> > > 
> > > It is a weird corner case, but how do we want to handle this?
> > > 
> > > -Michael
> > 
> > Maybe a slight renaming of my proposed new variable INITSCRIPTS to
> > SERVICES in the LFS files would make it a bit more clear that this
> > concerns only service-initscripts?
> > 
> > The INSTALL_INITSCRIPTS macro could then be renamed to
> > INSTALL_SERVICE_INITSCRIPTS.
> > 
> > If we then also keep the original INSTALL_INITSCRIPT macro, that
> > macro
> > can  then be used to install non-service initscripts if required.
> > Or
> > even to install all required initscripts in the occasion that you
> > want
> > to skip some service-initscripts defined in SERVICES because they
> > are
> > included in the source, like the situation Jonatan pointed out (in
> > this
> > case the SERVICES variable is filled with the sole purpose to serve
> > as
> > metadata).
> > 
> > So this would then result in:
> > ---
> > define INSTALL_INITSCRIPT
> >   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > /etc/rc.d/init.d/$(1)
> > endef
> > 
> > define INSTALL_SERVICE_INITSCRIPTS
> >   for service in $(SERVICES); do \
> >      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
> >   done
> > endef
> > ---
> > 
> > Optionally we could also add again a variable INITSCRIPTS with the
> > purpose to list non-service initscripts. so
> > SERVICES - for service-initscripts
> > INITSCRIPTS - for non-service-initscripts
> > 
> > and in the meta-* files:
> > ---
> > Name: NAME
> > Summary: SUMMARY
> > ProgVersion: VER
> > Release: RELEASE
> > Size: SIZE
> > Dependencies: DEPS
> > File: NAME-VER-RELEASE.ipfire
> > Services: SERVICES
> > InitScripts: INITSCRIPTS
> > ---
> > 
> > Maybe that info could also come in handy sooner or later ?
> 
> This sounds rather complicated to me.
> 
> What are our objectives here? Does Zabbix want/need to start/stop
> services? And if so, why?

I was not directly thinking of Zabbix specifically here. The objective
is to have a central 'database' of metadata for the pak's that can come
in handy for current and possibly future functionality of IPFire. And a
single way of retrieving this information (both by using "pakfire info"
or directly accessing the pakfire library, in my proposal). 

Knowing which pak's provide which services can be used in service.cgi
(which currently does give users the possibility to start/stop services
indeed) and can be used for monitoring agents like Zabbix, but also
Nagios or possibly others to know which services to monitor.

This instead of my earlier proposal of re-using parts of the
services.cgi code in a separate script for zabbix_agentd, which you
pointed out that should be done cleaner using some central way of
storing/retrieving that information.

Zabbix does have functionality to implement starting/stopping those
services using a context menu in the Zabbix web GUI; and a user can
easily implement that on it's own (as this should be done mainly on
server-side anyway) if he wants to and has the information of which
services should run. This could be done by zabbix server logging into
the IPFire machine using ssh and using addonctrl or by the agent, which
would then need permission to addonctrl start/stop. But I think that is
something for the user to decide on his own as you pointed out
previously, giving zabbix agent root permission on addonctrl start/stop
is a possible extra security risk. So I don't think we should provide
that out of the box. (Maybe it could be a configurable 'setting' in the
IPFire webgui some time in the future, but not something to focus on
currently)

But with that said, I don't really understand your remark here as I
only propose a way for the LFS files to populate the "Services" meta-
data and I only threw in "Initscripts" for non-service initscripts,
just for completeness of the meta-data.
And to prevent duplication of the list of initscripts, I added a macro
to install those in one go instead of again 'defining' this list by
needing to call INSTALL_INITSCRIPT individually for each initscript
while we now have a list of them in a variable. And due to the remark
of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak
maintainer to use in corner cases.

But this has nothing to do with actually stopping/starting services
other that providing information about which services are installed
(and maybe could be stopped/started if wanted).

If it would make things more clear, I could work my last mail out in an
actual patch ?

Regards
Robin

> 
> > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
> > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a
> > list
> > of initscripts to install, so one could call:
> > ---
> > $(call INSTALL_INITSCRIPTS,$(SERVICES))
> > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
> > ---
> > to install all initscripts.
> > 
> > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS
> > which
> > would then install both SERVICES and INITSCRIPTS (or even replace
> > the
> > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a
> > macro to
> > install all initscripts (SERVICES and INITSCRIPTS) and a macro to
> > install a single initscript in case some of the ones listed in
> > SERVICES
> > and/or INITSCRIPTS need to be skipped..
> > 
> > But I think the INSTALL_INITSCRIPTS which takes a list of services
> > to
> > install as parameter is probably the most generic one, giving you
> > the
> > possibility to install all SERVICES in one go, and then
> > individually
> > install only those INITSCRIPTS not included in the source if
> > INITSCRIPTS contains such initscript(s) or vice versa.
> > 
> > Robin
> > 
> > > 
> > > > On 12 May 2021, at 19:49, Jonatan Schlag <
> > > > jonatan.schlag@ipfire.org>
> > > > wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > > > robin.roevens@disroot.org>:
> > > > > 
> > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
> > > > > makefiles.
> > > > > * Add a Summary and InitScripts field to the meta-* addon
> > > > > files
> > > > > containing the value's of those macro's.
> > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new
> > > > > INSTALL_INITSCRIPTS macro/method that will install all
> > > > > initscripts
> > > > > defined in the new INITSCRIPTS macro.
> > > > > * Adapt libvirt and zabbix_agentd pak as examples of how to
> > > > > use
> > > > > this.
> > > > > 
> > > > > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > > > > ---
> > > > > lfs/Config        | 8 ++++++--
> > > > > lfs/libvirt       | 6 ++++--
> > > > > lfs/zabbix_agentd | 5 ++++-
> > > > > src/pakfire/meta  | 2 ++
> > > > > 4 files changed, 16 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/lfs/Config b/lfs/Config
> > > > > index eadbbc408..61d6f0c82 100644
> > > > > --- a/lfs/Config
> > > > > +++ b/lfs/Config
> > > > > @@ -299,15 +299,19 @@ define PAK
> > > > >   # Create meta file
> > > > >   sed \
> > > > >       -e "s/NAME/$(PROG)/g" \
> > > > > +        -e "s/SUMMARY/$(SUMMARY)/g" \
> > > > >       -e "s/VER/$(VER)/g" \
> > > > >       -e "s/RELEASE/$(PAK_VER)/g" \
> > > > >       -e "s/DEPS/$(DEPS)/g" \
> > > > >       -e "s/SIZE/$$(stat --format=%s
> > > > > /install/packages/$(PROG)-
> > > > > $(VER)-$(PAK_VER).ipfire)/g" \
> > > > > +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> > > > >     < /usr/src/src/pakfire/meta > /install/packages/meta-
> > > > > $(PROG)
> > > > > endef
> > > > > 
> > > > > -define INSTALL_INITSCRIPT
> > > > > -    install -m 754 -v
> > > > > $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > > > /etc/rc.d/init.d/$(1)
> > > > > +define INSTALL_INITSCRIPTS
> > > > > +    for initscript in $(INITSCRIPTS); do \
> > > > > +        install -m 754 -v
> > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > > > > /etc/rc.d/init.d/$$initscript; \
> > > > > +    done
> > > > > endef
> > > > > 
> > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
> > > > > riscv64)"
> > > > > diff --git a/lfs/libvirt b/lfs/libvirt
> > > > > index 28a95d317..be5d3db3a 100644
> > > > > --- a/lfs/libvirt
> > > > > +++ b/lfs/libvirt
> > > > > @@ -25,6 +25,7 @@
> > > > > include Config
> > > > > 
> > > > > VER        = 6.5.0
> > > > > +SUMMARY       = Server side daemon and supporting files for
> > > > > libvirt
> > > > > 
> > > > > THISAPP    = libvirt-$(VER)
> > > > > DL_FILE    = $(THISAPP).tar.xz
> > > > > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > > > > 
> > > > > DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> > > > > 
> > > > > +INITSCRIPTS= libvirtd virtlogd
> > > > > +
> > > > > #############################################################
> > > > > ######
> > > > > ############
> > > > > # Top-level Rules
> > > > > #############################################################
> > > > > ######
> > > > > ############
> > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > > > > %,$(DIR_DL)/%,$(objects))
> > > > >   cd $(DIR_APP)/build_libvirt && make install
> > > > > 
> > > > >   #install initscripts
> > > > > -    $(call INSTALL_INITSCRIPT,libvirtd)
> > > > > -    $(call INSTALL_INITSCRIPT,virtlogd)
> > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > >   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
> > > > > guests
> > > > And here my approach maybe breaks :-). How could we handle
> > > > this? It
> > > > is not a daemon, more something like a startup/shutdown scripts
> > > > for
> > > > machines... (And should not appear in the service.cgi). So
> > > > parsing
> > > > the root file may yield wrong results and your way would be the
> > > > better one. 
> > > > 
> > > > Jonatan
> > > > > 
> > > > >   # Backup
> > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > > > index c69643a54..a72fe024b 100644
> > > > > --- a/lfs/zabbix_agentd
> > > > > +++ b/lfs/zabbix_agentd
> > > > > @@ -25,6 +25,7 @@
> > > > > include Config
> > > > > 
> > > > > VER        = 4.2.6
> > > > > +SUMMARY    = Zabbix Agent
> > > > > 
> > > > > THISAPP    = zabbix-$(VER)
> > > > > DL_FILE    = $(THISAPP).tar.gz
> > > > > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > > > > PAK_VER    = 4
> > > > > DEPS       =
> > > > > 
> > > > > +INITSCRIPTS= zabbix_agentd
> > > > > +
> > > > > #############################################################
> > > > > ######
> > > > > ############
> > > > > # Top-level Rules
> > > > > #############################################################
> > > > > ######
> > > > > ############
> > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > > > > %,$(DIR_DL)/%,$(objects))
> > > > >   chown zabbix.zabbix /var/run/zabbix
> > > > > 
> > > > >   # Install initscripts
> > > > > -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > 
> > > > >   # Install sudoers include file
> > > > >   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > > > > index d97b2a0fa..849b9cd6c 100644
> > > > > --- a/src/pakfire/meta
> > > > > +++ b/src/pakfire/meta
> > > > > @@ -1,6 +1,8 @@
> > > > > Name: NAME
> > > > > +Summary: SUMMARY
> > > > > ProgVersion: VER
> > > > > Release: RELEASE
> > > > > Size: SIZE
> > > > > Dependencies: DEPS
> > > > > File: NAME-VER-RELEASE.ipfire
> > > > > +InitScripts: INITSCRIPTS
> > > > > -- 
> > > > > 2.31.1
> > > > > 
> > > > > 
> > > > > -- 
> > > > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > 
> > > 
> > 
> > 
> > -- 
> > Dit bericht is gescanned op virussen en andere gevaarlijke
> > inhoud door MailScanner en lijkt schoon te zijn.
> 
>
  
Robin Roevens June 6, 2021, 6:41 p.m. UTC | #9
Hi Michael

I know/see a lot of activity here, so I assume you are always quite
busy and may have missed my last reaction?

Should I proceed with my proposition to add
Summary: SUMMARY
Services: SERVICES
to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro

Should I also add 
InitScripts: INITSCRIPTS
for non-service initscripts.. just for completion of metadata ?
And in that case a more generic INSTALL_INITSCRIPTS macro accepting a
list of initscripts to install? (see mail history below for explanation
of my intentions)

Or should we find other methods to have this info ready for
services.cgi and/or monitoring agents ?

Regards
Robin

Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]:
> Hi Michael
> 
> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
> > 
> > 
> > > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens@disroot.org>
> > > wrote:
> > > 
> > > Hi
> > > 
> > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
> > > > Hello Jonatan,
> > > > 
> > > > Yes I get your point. Just because a package has more than one
> > > > initscript does not mean they both should be listed in
> > > > services.cgi.
> > > > 
> > > > It is a weird corner case, but how do we want to handle this?
> > > > 
> > > > -Michael
> > > 
> > > Maybe a slight renaming of my proposed new variable INITSCRIPTS to
> > > SERVICES in the LFS files would make it a bit more clear that this
> > > concerns only service-initscripts?
> > > 
> > > The INSTALL_INITSCRIPTS macro could then be renamed to
> > > INSTALL_SERVICE_INITSCRIPTS.
> > > 
> > > If we then also keep the original INSTALL_INITSCRIPT macro, that
> > > macro
> > > can  then be used to install non-service initscripts if required.
> > > Or
> > > even to install all required initscripts in the occasion that you
> > > want
> > > to skip some service-initscripts defined in SERVICES because they
> > > are
> > > included in the source, like the situation Jonatan pointed out (in
> > > this
> > > case the SERVICES variable is filled with the sole purpose to serve
> > > as
> > > metadata).
> > > 
> > > So this would then result in:
> > > ---
> > > define INSTALL_INITSCRIPT
> > >   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > /etc/rc.d/init.d/$(1)
> > > endef
> > > 
> > > define INSTALL_SERVICE_INITSCRIPTS
> > >   for service in $(SERVICES); do \
> > >      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
> > >   done
> > > endef
> > > ---
> > > 
> > > Optionally we could also add again a variable INITSCRIPTS with the
> > > purpose to list non-service initscripts. so
> > > SERVICES - for service-initscripts
> > > INITSCRIPTS - for non-service-initscripts
> > > 
> > > and in the meta-* files:
> > > ---
> > > Name: NAME
> > > Summary: SUMMARY
> > > ProgVersion: VER
> > > Release: RELEASE
> > > Size: SIZE
> > > Dependencies: DEPS
> > > File: NAME-VER-RELEASE.ipfire
> > > Services: SERVICES
> > > InitScripts: INITSCRIPTS
> > > ---
> > > 
> > > Maybe that info could also come in handy sooner or later ?
> > 
> > This sounds rather complicated to me.
> > 
> > What are our objectives here? Does Zabbix want/need to start/stop
> > services? And if so, why?
> 
> I was not directly thinking of Zabbix specifically here. The objective
> is to have a central 'database' of metadata for the pak's that can come
> in handy for current and possibly future functionality of IPFire. And a
> single way of retrieving this information (both by using "pakfire info"
> or directly accessing the pakfire library, in my proposal). 
> 
> Knowing which pak's provide which services can be used in service.cgi
> (which currently does give users the possibility to start/stop services
> indeed) and can be used for monitoring agents like Zabbix, but also
> Nagios or possibly others to know which services to monitor.
> 
> This instead of my earlier proposal of re-using parts of the
> services.cgi code in a separate script for zabbix_agentd, which you
> pointed out that should be done cleaner using some central way of
> storing/retrieving that information.
> 
> Zabbix does have functionality to implement starting/stopping those
> services using a context menu in the Zabbix web GUI; and a user can
> easily implement that on it's own (as this should be done mainly on
> server-side anyway) if he wants to and has the information of which
> services should run. This could be done by zabbix server logging into
> the IPFire machine using ssh and using addonctrl or by the agent, which
> would then need permission to addonctrl start/stop. But I think that is
> something for the user to decide on his own as you pointed out
> previously, giving zabbix agent root permission on addonctrl start/stop
> is a possible extra security risk. So I don't think we should provide
> that out of the box. (Maybe it could be a configurable 'setting' in the
> IPFire webgui some time in the future, but not something to focus on
> currently)
> 
> But with that said, I don't really understand your remark here as I
> only propose a way for the LFS files to populate the "Services" meta-
> data and I only threw in "Initscripts" for non-service initscripts,
> just for completeness of the meta-data.
> And to prevent duplication of the list of initscripts, I added a macro
> to install those in one go instead of again 'defining' this list by
> needing to call INSTALL_INITSCRIPT individually for each initscript
> while we now have a list of them in a variable. And due to the remark
> of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak
> maintainer to use in corner cases.
> 
> But this has nothing to do with actually stopping/starting services
> other that providing information about which services are installed
> (and maybe could be stopped/started if wanted).
> 
> If it would make things more clear, I could work my last mail out in an
> actual patch ?
> 
> Regards
> Robin
> 
> > 
> > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
> > > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a
> > > list
> > > of initscripts to install, so one could call:
> > > ---
> > > $(call INSTALL_INITSCRIPTS,$(SERVICES))
> > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
> > > ---
> > > to install all initscripts.
> > > 
> > > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS
> > > which
> > > would then install both SERVICES and INITSCRIPTS (or even replace
> > > the
> > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a
> > > macro to
> > > install all initscripts (SERVICES and INITSCRIPTS) and a macro to
> > > install a single initscript in case some of the ones listed in
> > > SERVICES
> > > and/or INITSCRIPTS need to be skipped..
> > > 
> > > But I think the INSTALL_INITSCRIPTS which takes a list of services
> > > to
> > > install as parameter is probably the most generic one, giving you
> > > the
> > > possibility to install all SERVICES in one go, and then
> > > individually
> > > install only those INITSCRIPTS not included in the source if
> > > INITSCRIPTS contains such initscript(s) or vice versa.
> > > 
> > > Robin
> > > 
> > > > 
> > > > > On 12 May 2021, at 19:49, Jonatan Schlag <
> > > > > jonatan.schlag@ipfire.org>
> > > > > wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > > > > robin.roevens@disroot.org>:
> > > > > > 
> > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
> > > > > > makefiles.
> > > > > > * Add a Summary and InitScripts field to the meta-* addon
> > > > > > files
> > > > > > containing the value's of those macro's.
> > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new
> > > > > > INSTALL_INITSCRIPTS macro/method that will install all
> > > > > > initscripts
> > > > > > defined in the new INITSCRIPTS macro.
> > > > > > * Adapt libvirt and zabbix_agentd pak as examples of how to
> > > > > > use
> > > > > > this.
> > > > > > 
> > > > > > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > > > > > ---
> > > > > > lfs/Config        | 8 ++++++--
> > > > > > lfs/libvirt       | 6 ++++--
> > > > > > lfs/zabbix_agentd | 5 ++++-
> > > > > > src/pakfire/meta  | 2 ++
> > > > > > 4 files changed, 16 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/lfs/Config b/lfs/Config
> > > > > > index eadbbc408..61d6f0c82 100644
> > > > > > --- a/lfs/Config
> > > > > > +++ b/lfs/Config
> > > > > > @@ -299,15 +299,19 @@ define PAK
> > > > > >   # Create meta file
> > > > > >   sed \
> > > > > >       -e "s/NAME/$(PROG)/g" \
> > > > > > +        -e "s/SUMMARY/$(SUMMARY)/g" \
> > > > > >       -e "s/VER/$(VER)/g" \
> > > > > >       -e "s/RELEASE/$(PAK_VER)/g" \
> > > > > >       -e "s/DEPS/$(DEPS)/g" \
> > > > > >       -e "s/SIZE/$$(stat --format=%s
> > > > > > /install/packages/$(PROG)-
> > > > > > $(VER)-$(PAK_VER).ipfire)/g" \
> > > > > > +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> > > > > >     < /usr/src/src/pakfire/meta > /install/packages/meta-
> > > > > > $(PROG)
> > > > > > endef
> > > > > > 
> > > > > > -define INSTALL_INITSCRIPT
> > > > > > -    install -m 754 -v
> > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > > > > /etc/rc.d/init.d/$(1)
> > > > > > +define INSTALL_INITSCRIPTS
> > > > > > +    for initscript in $(INITSCRIPTS); do \
> > > > > > +        install -m 754 -v
> > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > > > > > /etc/rc.d/init.d/$$initscript; \
> > > > > > +    done
> > > > > > endef
> > > > > > 
> > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
> > > > > > riscv64)"
> > > > > > diff --git a/lfs/libvirt b/lfs/libvirt
> > > > > > index 28a95d317..be5d3db3a 100644
> > > > > > --- a/lfs/libvirt
> > > > > > +++ b/lfs/libvirt
> > > > > > @@ -25,6 +25,7 @@
> > > > > > include Config
> > > > > > 
> > > > > > VER        = 6.5.0
> > > > > > +SUMMARY       = Server side daemon and supporting files for
> > > > > > libvirt
> > > > > > 
> > > > > > THISAPP    = libvirt-$(VER)
> > > > > > DL_FILE    = $(THISAPP).tar.xz
> > > > > > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > > > > > 
> > > > > > DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> > > > > > 
> > > > > > +INITSCRIPTS= libvirtd virtlogd
> > > > > > +
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > # Top-level Rules
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > >   cd $(DIR_APP)/build_libvirt && make install
> > > > > > 
> > > > > >   #install initscripts
> > > > > > -    $(call INSTALL_INITSCRIPT,libvirtd)
> > > > > > -    $(call INSTALL_INITSCRIPT,virtlogd)
> > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > >   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
> > > > > > guests
> > > > > And here my approach maybe breaks :-). How could we handle
> > > > > this? It
> > > > > is not a daemon, more something like a startup/shutdown scripts
> > > > > for
> > > > > machines... (And should not appear in the service.cgi). So
> > > > > parsing
> > > > > the root file may yield wrong results and your way would be the
> > > > > better one. 
> > > > > 
> > > > > Jonatan
> > > > > > 
> > > > > >   # Backup
> > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > > > > index c69643a54..a72fe024b 100644
> > > > > > --- a/lfs/zabbix_agentd
> > > > > > +++ b/lfs/zabbix_agentd
> > > > > > @@ -25,6 +25,7 @@
> > > > > > include Config
> > > > > > 
> > > > > > VER        = 4.2.6
> > > > > > +SUMMARY    = Zabbix Agent
> > > > > > 
> > > > > > THISAPP    = zabbix-$(VER)
> > > > > > DL_FILE    = $(THISAPP).tar.gz
> > > > > > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > > > > > PAK_VER    = 4
> > > > > > DEPS       =
> > > > > > 
> > > > > > +INITSCRIPTS= zabbix_agentd
> > > > > > +
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > # Top-level Rules
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > >   chown zabbix.zabbix /var/run/zabbix
> > > > > > 
> > > > > >   # Install initscripts
> > > > > > -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > > 
> > > > > >   # Install sudoers include file
> > > > > >   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > > > > > index d97b2a0fa..849b9cd6c 100644
> > > > > > --- a/src/pakfire/meta
> > > > > > +++ b/src/pakfire/meta
> > > > > > @@ -1,6 +1,8 @@
> > > > > > Name: NAME
> > > > > > +Summary: SUMMARY
> > > > > > ProgVersion: VER
> > > > > > Release: RELEASE
> > > > > > Size: SIZE
> > > > > > Dependencies: DEPS
> > > > > > File: NAME-VER-RELEASE.ipfire
> > > > > > +InitScripts: INITSCRIPTS
> > > > > > -- 
> > > > > > 2.31.1
> > > > > > 
> > > > > > 
> > > > > > -- 
> > > > > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > > 
> > > > 
> > > 
> > > 
> > > -- 
> > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > inhoud door MailScanner en lijkt schoon te zijn.
> > 
> > 
> 
>
  
Michael Tremer June 10, 2021, 11:24 a.m. UTC | #10
Hello,

> On 24 May 2021, at 21:23, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi Michael
> 
> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
>> 
>> 
>>> On 14 May 2021, at 21:07, Robin Roevens <robin.roevens@disroot.org>
>>> wrote:
>>> 
>>> Hi
>>> 
>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
>>>> Hello Jonatan,
>>>> 
>>>> Yes I get your point. Just because a package has more than one
>>>> initscript does not mean they both should be listed in
>>>> services.cgi.
>>>> 
>>>> It is a weird corner case, but how do we want to handle this?
>>>> 
>>>> -Michael
>>> 
>>> Maybe a slight renaming of my proposed new variable INITSCRIPTS to
>>> SERVICES in the LFS files would make it a bit more clear that this
>>> concerns only service-initscripts?
>>> 
>>> The INSTALL_INITSCRIPTS macro could then be renamed to
>>> INSTALL_SERVICE_INITSCRIPTS.
>>> 
>>> If we then also keep the original INSTALL_INITSCRIPT macro, that
>>> macro
>>> can  then be used to install non-service initscripts if required.
>>> Or
>>> even to install all required initscripts in the occasion that you
>>> want
>>> to skip some service-initscripts defined in SERVICES because they
>>> are
>>> included in the source, like the situation Jonatan pointed out (in
>>> this
>>> case the SERVICES variable is filled with the sole purpose to serve
>>> as
>>> metadata).
>>> 
>>> So this would then result in:
>>> ---
>>> define INSTALL_INITSCRIPT
>>>   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
>>> /etc/rc.d/init.d/$(1)
>>> endef
>>> 
>>> define INSTALL_SERVICE_INITSCRIPTS
>>>   for service in $(SERVICES); do \
>>>      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
>>>   done
>>> endef
>>> ---
>>> 
>>> Optionally we could also add again a variable INITSCRIPTS with the
>>> purpose to list non-service initscripts. so
>>> SERVICES - for service-initscripts
>>> INITSCRIPTS - for non-service-initscripts
>>> 
>>> and in the meta-* files:
>>> ---
>>> Name: NAME
>>> Summary: SUMMARY
>>> ProgVersion: VER
>>> Release: RELEASE
>>> Size: SIZE
>>> Dependencies: DEPS
>>> File: NAME-VER-RELEASE.ipfire
>>> Services: SERVICES
>>> InitScripts: INITSCRIPTS
>>> ---
>>> 
>>> Maybe that info could also come in handy sooner or later ?
>> 
>> This sounds rather complicated to me.
>> 
>> What are our objectives here? Does Zabbix want/need to start/stop
>> services? And if so, why?
> 
> I was not directly thinking of Zabbix specifically here. The objective
> is to have a central 'database' of metadata for the pak's that can come
> in handy for current and possibly future functionality of IPFire. And a
> single way of retrieving this information (both by using "pakfire info"
> or directly accessing the pakfire library, in my proposal). 

I understood that, but my question remains. Why does Zabbix need to start/stop individual services on the firewall?

I just want to understand the use-case so I can help to build a better solution.

> Knowing which pak's provide which services can be used in service.cgi
> (which currently does give users the possibility to start/stop services
> indeed) and can be used for monitoring agents like Zabbix, but also
> Nagios or possibly others to know which services to monitor.
> 
> This instead of my earlier proposal of re-using parts of the
> services.cgi code in a separate script for zabbix_agentd, which you
> pointed out that should be done cleaner using some central way of
> storing/retrieving that information.
> 
> Zabbix does have functionality to implement starting/stopping those
> services using a context menu in the Zabbix web GUI; and a user can
> easily implement that on it's own (as this should be done mainly on
> server-side anyway) if he wants to and has the information of which
> services should run. This could be done by zabbix server logging into
> the IPFire machine using ssh and using addonctrl or by the agent, which
> would then need permission to addonctrl start/stop. But I think that is
> something for the user to decide on his own as you pointed out
> previously, giving zabbix agent root permission on addonctrl start/stop
> is a possible extra security risk. So I don't think we should provide
> that out of the box. (Maybe it could be a configurable 'setting' in the
> IPFire webgui some time in the future, but not something to focus on
> currently)
> 
> But with that said, I don't really understand your remark here as I
> only propose a way for the LFS files to populate the "Services" meta-
> data and I only threw in "Initscripts" for non-service initscripts,
> just for completeness of the meta-data.
> And to prevent duplication of the list of initscripts, I added a macro
> to install those in one go instead of again 'defining' this list by
> needing to call INSTALL_INITSCRIPT individually for each initscript
> while we now have a list of them in a variable. And due to the remark
> of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak
> maintainer to use in corner cases.
> 
> But this has nothing to do with actually stopping/starting services
> other that providing information about which services are installed
> (and maybe could be stopped/started if wanted).

Noted.

> If it would make things more clear, I could work my last mail out in an
> actual patch ?

To remind me again, what does that entail?

-Michael

> Regards
> Robin
> 
>> 
>>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
>>> back to INSTALL_INITSCRIPTS, now accepting a parameter which is a
>>> list
>>> of initscripts to install, so one could call:
>>> ---
>>> $(call INSTALL_INITSCRIPTS,$(SERVICES))
>>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
>>> ---
>>> to install all initscripts.
>>> 
>>> Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS
>>> which
>>> would then install both SERVICES and INITSCRIPTS (or even replace
>>> the
>>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a
>>> macro to
>>> install all initscripts (SERVICES and INITSCRIPTS) and a macro to
>>> install a single initscript in case some of the ones listed in
>>> SERVICES
>>> and/or INITSCRIPTS need to be skipped..
>>> 
>>> But I think the INSTALL_INITSCRIPTS which takes a list of services
>>> to
>>> install as parameter is probably the most generic one, giving you
>>> the
>>> possibility to install all SERVICES in one go, and then
>>> individually
>>> install only those INITSCRIPTS not included in the source if
>>> INITSCRIPTS contains such initscript(s) or vice versa.
>>> 
>>> Robin
>>> 
>>>> 
>>>>> On 12 May 2021, at 19:49, Jonatan Schlag <
>>>>> jonatan.schlag@ipfire.org>
>>>>> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens <
>>>>>> robin.roevens@disroot.org>:
>>>>>> 
>>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
>>>>>> makefiles.
>>>>>> * Add a Summary and InitScripts field to the meta-* addon
>>>>>> files
>>>>>> containing the value's of those macro's.
>>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>>>>>> INSTALL_INITSCRIPTS macro/method that will install all
>>>>>> initscripts
>>>>>> defined in the new INITSCRIPTS macro.
>>>>>> * Adapt libvirt and zabbix_agentd pak as examples of how to
>>>>>> use
>>>>>> this.
>>>>>> 
>>>>>> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
>>>>>> ---
>>>>>> lfs/Config        | 8 ++++++--
>>>>>> lfs/libvirt       | 6 ++++--
>>>>>> lfs/zabbix_agentd | 5 ++++-
>>>>>> src/pakfire/meta  | 2 ++
>>>>>> 4 files changed, 16 insertions(+), 5 deletions(-)
>>>>>> 
>>>>>> diff --git a/lfs/Config b/lfs/Config
>>>>>> index eadbbc408..61d6f0c82 100644
>>>>>> --- a/lfs/Config
>>>>>> +++ b/lfs/Config
>>>>>> @@ -299,15 +299,19 @@ define PAK
>>>>>>   # Create meta file
>>>>>>   sed \
>>>>>>       -e "s/NAME/$(PROG)/g" \
>>>>>> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>>>>>>       -e "s/VER/$(VER)/g" \
>>>>>>       -e "s/RELEASE/$(PAK_VER)/g" \
>>>>>>       -e "s/DEPS/$(DEPS)/g" \
>>>>>>       -e "s/SIZE/$$(stat --format=%s
>>>>>> /install/packages/$(PROG)-
>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \
>>>>>> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>>>>>     < /usr/src/src/pakfire/meta > /install/packages/meta-
>>>>>> $(PROG)
>>>>>> endef
>>>>>> 
>>>>>> -define INSTALL_INITSCRIPT
>>>>>> -    install -m 754 -v
>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>>>> /etc/rc.d/init.d/$(1)
>>>>>> +define INSTALL_INITSCRIPTS
>>>>>> +    for initscript in $(INITSCRIPTS); do \
>>>>>> +        install -m 754 -v
>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript 
>>>>>> /etc/rc.d/init.d/$$initscript; \
>>>>>> +    done
>>>>>> endef
>>>>>> 
>>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
>>>>>> riscv64)"
>>>>>> diff --git a/lfs/libvirt b/lfs/libvirt
>>>>>> index 28a95d317..be5d3db3a 100644
>>>>>> --- a/lfs/libvirt
>>>>>> +++ b/lfs/libvirt
>>>>>> @@ -25,6 +25,7 @@
>>>>>> include Config
>>>>>> 
>>>>>> VER        = 6.5.0
>>>>>> +SUMMARY       = Server side daemon and supporting files for
>>>>>> libvirt
>>>>>> 
>>>>>> THISAPP    = libvirt-$(VER)
>>>>>> DL_FILE    = $(THISAPP).tar.xz
>>>>>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>>>>>> 
>>>>>> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
>>>>>> 
>>>>>> +INITSCRIPTS= libvirtd virtlogd
>>>>>> +
>>>>>> #############################################################
>>>>>> ######
>>>>>> ############
>>>>>> # Top-level Rules
>>>>>> #############################################################
>>>>>> ######
>>>>>> ############
>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>   cd $(DIR_APP)/build_libvirt && make install
>>>>>> 
>>>>>>   #install initscripts
>>>>>> -    $(call INSTALL_INITSCRIPT,libvirtd)
>>>>>> -    $(call INSTALL_INITSCRIPT,virtlogd)
>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>>   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
>>>>>> guests
>>>>> And here my approach maybe breaks :-). How could we handle
>>>>> this? It
>>>>> is not a daemon, more something like a startup/shutdown scripts
>>>>> for
>>>>> machines... (And should not appear in the service.cgi). So
>>>>> parsing
>>>>> the root file may yield wrong results and your way would be the
>>>>> better one. 
>>>>> 
>>>>> Jonatan
>>>>>> 
>>>>>>   # Backup
>>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>>>>>> index c69643a54..a72fe024b 100644
>>>>>> --- a/lfs/zabbix_agentd
>>>>>> +++ b/lfs/zabbix_agentd
>>>>>> @@ -25,6 +25,7 @@
>>>>>> include Config
>>>>>> 
>>>>>> VER        = 4.2.6
>>>>>> +SUMMARY    = Zabbix Agent
>>>>>> 
>>>>>> THISAPP    = zabbix-$(VER)
>>>>>> DL_FILE    = $(THISAPP).tar.gz
>>>>>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>>>>>> PAK_VER    = 4
>>>>>> DEPS       =
>>>>>> 
>>>>>> +INITSCRIPTS= zabbix_agentd
>>>>>> +
>>>>>> #############################################################
>>>>>> ######
>>>>>> ############
>>>>>> # Top-level Rules
>>>>>> #############################################################
>>>>>> ######
>>>>>> ############
>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>   chown zabbix.zabbix /var/run/zabbix
>>>>>> 
>>>>>>   # Install initscripts
>>>>>> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>> 
>>>>>>   # Install sudoers include file
>>>>>>   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
>>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>>>>>> index d97b2a0fa..849b9cd6c 100644
>>>>>> --- a/src/pakfire/meta
>>>>>> +++ b/src/pakfire/meta
>>>>>> @@ -1,6 +1,8 @@
>>>>>> Name: NAME
>>>>>> +Summary: SUMMARY
>>>>>> ProgVersion: VER
>>>>>> Release: RELEASE
>>>>>> Size: SIZE
>>>>>> Dependencies: DEPS
>>>>>> File: NAME-VER-RELEASE.ipfire
>>>>>> +InitScripts: INITSCRIPTS
>>>>>> -- 
>>>>>> 2.31.1
>>>>>> 
>>>>>> 
>>>>>> -- 
>>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>>>>> inhoud door MailScanner en lijkt schoon te zijn.
>>>> 
>>>> 
>>> 
>>> 
>>> -- 
>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>> inhoud door MailScanner en lijkt schoon te zijn.
>> 
>> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
  
Michael Tremer June 10, 2021, 11:27 a.m. UTC | #11
Hello,

> On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi Michael
> 
> I know/see a lot of activity here, so I assume you are always quite
> busy and may have missed my last reaction?

Yes, sorry, I missed this, or didn’t know what to say. Feel free to remind me if you didn’t hear from me within a couple of days.

> Should I proceed with my proposition to add
> Summary: SUMMARY
> Services: SERVICES
> to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro

I would say yes. Do we have any unanswered questions left?

Is Jonatan happy?

> Should I also add 
> InitScripts: INITSCRIPTS
> for non-service initscripts.. just for completion of metadata ?

Hmm, I would say no. If there is no point to it, then leave it.

We can always add it later if we decide that we want it later.

> And in that case a more generic INSTALL_INITSCRIPTS macro accepting a
> list of initscripts to install? (see mail history below for explanation
> of my intentions)
> 
> Or should we find other methods to have this info ready for
> services.cgi and/or monitoring agents ?

No, I believe this is a good solution. We can then have a function that queries the package database and shows all services correctly.

That sounds clean to me and is re-usable and extensible.

-Michael

> 
> Regards
> Robin
> 
> Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]:
>> Hi Michael
>> 
>> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
>>> 
>>> 
>>>> On 14 May 2021, at 21:07, Robin Roevens <robin.roevens@disroot.org>
>>>> wrote:
>>>> 
>>>> Hi
>>>> 
>>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
>>>>> Hello Jonatan,
>>>>> 
>>>>> Yes I get your point. Just because a package has more than one
>>>>> initscript does not mean they both should be listed in
>>>>> services.cgi.
>>>>> 
>>>>> It is a weird corner case, but how do we want to handle this?
>>>>> 
>>>>> -Michael
>>>> 
>>>> Maybe a slight renaming of my proposed new variable INITSCRIPTS to
>>>> SERVICES in the LFS files would make it a bit more clear that this
>>>> concerns only service-initscripts?
>>>> 
>>>> The INSTALL_INITSCRIPTS macro could then be renamed to
>>>> INSTALL_SERVICE_INITSCRIPTS.
>>>> 
>>>> If we then also keep the original INSTALL_INITSCRIPT macro, that
>>>> macro
>>>> can  then be used to install non-service initscripts if required.
>>>> Or
>>>> even to install all required initscripts in the occasion that you
>>>> want
>>>> to skip some service-initscripts defined in SERVICES because they
>>>> are
>>>> included in the source, like the situation Jonatan pointed out (in
>>>> this
>>>> case the SERVICES variable is filled with the sole purpose to serve
>>>> as
>>>> metadata).
>>>> 
>>>> So this would then result in:
>>>> ---
>>>> define INSTALL_INITSCRIPT
>>>>   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>> /etc/rc.d/init.d/$(1)
>>>> endef
>>>> 
>>>> define INSTALL_SERVICE_INITSCRIPTS
>>>>   for service in $(SERVICES); do \
>>>>      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
>>>>   done
>>>> endef
>>>> ---
>>>> 
>>>> Optionally we could also add again a variable INITSCRIPTS with the
>>>> purpose to list non-service initscripts. so
>>>> SERVICES - for service-initscripts
>>>> INITSCRIPTS - for non-service-initscripts
>>>> 
>>>> and in the meta-* files:
>>>> ---
>>>> Name: NAME
>>>> Summary: SUMMARY
>>>> ProgVersion: VER
>>>> Release: RELEASE
>>>> Size: SIZE
>>>> Dependencies: DEPS
>>>> File: NAME-VER-RELEASE.ipfire
>>>> Services: SERVICES
>>>> InitScripts: INITSCRIPTS
>>>> ---
>>>> 
>>>> Maybe that info could also come in handy sooner or later ?
>>> 
>>> This sounds rather complicated to me.
>>> 
>>> What are our objectives here? Does Zabbix want/need to start/stop
>>> services? And if so, why?
>> 
>> I was not directly thinking of Zabbix specifically here. The objective
>> is to have a central 'database' of metadata for the pak's that can come
>> in handy for current and possibly future functionality of IPFire. And a
>> single way of retrieving this information (both by using "pakfire info"
>> or directly accessing the pakfire library, in my proposal). 
>> 
>> Knowing which pak's provide which services can be used in service.cgi
>> (which currently does give users the possibility to start/stop services
>> indeed) and can be used for monitoring agents like Zabbix, but also
>> Nagios or possibly others to know which services to monitor.
>> 
>> This instead of my earlier proposal of re-using parts of the
>> services.cgi code in a separate script for zabbix_agentd, which you
>> pointed out that should be done cleaner using some central way of
>> storing/retrieving that information.
>> 
>> Zabbix does have functionality to implement starting/stopping those
>> services using a context menu in the Zabbix web GUI; and a user can
>> easily implement that on it's own (as this should be done mainly on
>> server-side anyway) if he wants to and has the information of which
>> services should run. This could be done by zabbix server logging into
>> the IPFire machine using ssh and using addonctrl or by the agent, which
>> would then need permission to addonctrl start/stop. But I think that is
>> something for the user to decide on his own as you pointed out
>> previously, giving zabbix agent root permission on addonctrl start/stop
>> is a possible extra security risk. So I don't think we should provide
>> that out of the box. (Maybe it could be a configurable 'setting' in the
>> IPFire webgui some time in the future, but not something to focus on
>> currently)
>> 
>> But with that said, I don't really understand your remark here as I
>> only propose a way for the LFS files to populate the "Services" meta-
>> data and I only threw in "Initscripts" for non-service initscripts,
>> just for completeness of the meta-data.
>> And to prevent duplication of the list of initscripts, I added a macro
>> to install those in one go instead of again 'defining' this list by
>> needing to call INSTALL_INITSCRIPT individually for each initscript
>> while we now have a list of them in a variable. And due to the remark
>> of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak
>> maintainer to use in corner cases.
>> 
>> But this has nothing to do with actually stopping/starting services
>> other that providing information about which services are installed
>> (and maybe could be stopped/started if wanted).
>> 
>> If it would make things more clear, I could work my last mail out in an
>> actual patch ?
>> 
>> Regards
>> Robin
>> 
>>> 
>>>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
>>>> back to INSTALL_INITSCRIPTS, now accepting a parameter which is a
>>>> list
>>>> of initscripts to install, so one could call:
>>>> ---
>>>> $(call INSTALL_INITSCRIPTS,$(SERVICES))
>>>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
>>>> ---
>>>> to install all initscripts.
>>>> 
>>>> Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS
>>>> which
>>>> would then install both SERVICES and INITSCRIPTS (or even replace
>>>> the
>>>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a
>>>> macro to
>>>> install all initscripts (SERVICES and INITSCRIPTS) and a macro to
>>>> install a single initscript in case some of the ones listed in
>>>> SERVICES
>>>> and/or INITSCRIPTS need to be skipped..
>>>> 
>>>> But I think the INSTALL_INITSCRIPTS which takes a list of services
>>>> to
>>>> install as parameter is probably the most generic one, giving you
>>>> the
>>>> possibility to install all SERVICES in one go, and then
>>>> individually
>>>> install only those INITSCRIPTS not included in the source if
>>>> INITSCRIPTS contains such initscript(s) or vice versa.
>>>> 
>>>> Robin
>>>> 
>>>>> 
>>>>>> On 12 May 2021, at 19:49, Jonatan Schlag <
>>>>>> jonatan.schlag@ipfire.org>
>>>>>> wrote:
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens <
>>>>>>> robin.roevens@disroot.org>:
>>>>>>> 
>>>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
>>>>>>> makefiles.
>>>>>>> * Add a Summary and InitScripts field to the meta-* addon
>>>>>>> files
>>>>>>> containing the value's of those macro's.
>>>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new
>>>>>>> INSTALL_INITSCRIPTS macro/method that will install all
>>>>>>> initscripts
>>>>>>> defined in the new INITSCRIPTS macro.
>>>>>>> * Adapt libvirt and zabbix_agentd pak as examples of how to
>>>>>>> use
>>>>>>> this.
>>>>>>> 
>>>>>>> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
>>>>>>> ---
>>>>>>> lfs/Config        | 8 ++++++--
>>>>>>> lfs/libvirt       | 6 ++++--
>>>>>>> lfs/zabbix_agentd | 5 ++++-
>>>>>>> src/pakfire/meta  | 2 ++
>>>>>>> 4 files changed, 16 insertions(+), 5 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/lfs/Config b/lfs/Config
>>>>>>> index eadbbc408..61d6f0c82 100644
>>>>>>> --- a/lfs/Config
>>>>>>> +++ b/lfs/Config
>>>>>>> @@ -299,15 +299,19 @@ define PAK
>>>>>>>   # Create meta file
>>>>>>>   sed \
>>>>>>>       -e "s/NAME/$(PROG)/g" \
>>>>>>> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>>>>>>>       -e "s/VER/$(VER)/g" \
>>>>>>>       -e "s/RELEASE/$(PAK_VER)/g" \
>>>>>>>       -e "s/DEPS/$(DEPS)/g" \
>>>>>>>       -e "s/SIZE/$$(stat --format=%s
>>>>>>> /install/packages/$(PROG)-
>>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \
>>>>>>> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>>>>>>     < /usr/src/src/pakfire/meta > /install/packages/meta-
>>>>>>> $(PROG)
>>>>>>> endef
>>>>>>> 
>>>>>>> -define INSTALL_INITSCRIPT
>>>>>>> -    install -m 754 -v
>>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>>>>> /etc/rc.d/init.d/$(1)
>>>>>>> +define INSTALL_INITSCRIPTS
>>>>>>> +    for initscript in $(INITSCRIPTS); do \
>>>>>>> +        install -m 754 -v
>>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript 
>>>>>>> /etc/rc.d/init.d/$$initscript; \
>>>>>>> +    done
>>>>>>> endef
>>>>>>> 
>>>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
>>>>>>> riscv64)"
>>>>>>> diff --git a/lfs/libvirt b/lfs/libvirt
>>>>>>> index 28a95d317..be5d3db3a 100644
>>>>>>> --- a/lfs/libvirt
>>>>>>> +++ b/lfs/libvirt
>>>>>>> @@ -25,6 +25,7 @@
>>>>>>> include Config
>>>>>>> 
>>>>>>> VER        = 6.5.0
>>>>>>> +SUMMARY       = Server side daemon and supporting files for
>>>>>>> libvirt
>>>>>>> 
>>>>>>> THISAPP    = libvirt-$(VER)
>>>>>>> DL_FILE    = $(THISAPP).tar.xz
>>>>>>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>>>>>>> 
>>>>>>> DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
>>>>>>> 
>>>>>>> +INITSCRIPTS= libvirtd virtlogd
>>>>>>> +
>>>>>>> #############################################################
>>>>>>> ######
>>>>>>> ############
>>>>>>> # Top-level Rules
>>>>>>> #############################################################
>>>>>>> ######
>>>>>>> ############
>>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
>>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>>   cd $(DIR_APP)/build_libvirt && make install
>>>>>>> 
>>>>>>>   #install initscripts
>>>>>>> -    $(call INSTALL_INITSCRIPT,libvirtd)
>>>>>>> -    $(call INSTALL_INITSCRIPT,virtlogd)
>>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>>>   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
>>>>>>> guests
>>>>>> And here my approach maybe breaks :-). How could we handle
>>>>>> this? It
>>>>>> is not a daemon, more something like a startup/shutdown scripts
>>>>>> for
>>>>>> machines... (And should not appear in the service.cgi). So
>>>>>> parsing
>>>>>> the root file may yield wrong results and your way would be the
>>>>>> better one. 
>>>>>> 
>>>>>> Jonatan
>>>>>>> 
>>>>>>>   # Backup
>>>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>>>>>>> index c69643a54..a72fe024b 100644
>>>>>>> --- a/lfs/zabbix_agentd
>>>>>>> +++ b/lfs/zabbix_agentd
>>>>>>> @@ -25,6 +25,7 @@
>>>>>>> include Config
>>>>>>> 
>>>>>>> VER        = 4.2.6
>>>>>>> +SUMMARY    = Zabbix Agent
>>>>>>> 
>>>>>>> THISAPP    = zabbix-$(VER)
>>>>>>> DL_FILE    = $(THISAPP).tar.gz
>>>>>>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>>>>>>> PAK_VER    = 4
>>>>>>> DEPS       =
>>>>>>> 
>>>>>>> +INITSCRIPTS= zabbix_agentd
>>>>>>> +
>>>>>>> #############################################################
>>>>>>> ######
>>>>>>> ############
>>>>>>> # Top-level Rules
>>>>>>> #############################################################
>>>>>>> ######
>>>>>>> ############
>>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
>>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>>   chown zabbix.zabbix /var/run/zabbix
>>>>>>> 
>>>>>>>   # Install initscripts
>>>>>>> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
>>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>>> 
>>>>>>>   # Install sudoers include file
>>>>>>>   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
>>>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>>>>>>> index d97b2a0fa..849b9cd6c 100644
>>>>>>> --- a/src/pakfire/meta
>>>>>>> +++ b/src/pakfire/meta
>>>>>>> @@ -1,6 +1,8 @@
>>>>>>> Name: NAME
>>>>>>> +Summary: SUMMARY
>>>>>>> ProgVersion: VER
>>>>>>> Release: RELEASE
>>>>>>> Size: SIZE
>>>>>>> Dependencies: DEPS
>>>>>>> File: NAME-VER-RELEASE.ipfire
>>>>>>> +InitScripts: INITSCRIPTS
>>>>>>> -- 
>>>>>>> 2.31.1
>>>>>>> 
>>>>>>> 
>>>>>>> -- 
>>>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>>>>>> inhoud door MailScanner en lijkt schoon te zijn.
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> -- 
>>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>>> inhoud door MailScanner en lijkt schoon te zijn.
>>> 
>>> 
>> 
>> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
  
Robin Roevens June 17, 2021, 10:28 p.m. UTC | #12
Hi

Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]:
> Hello,
> 
> > On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens@disroot.org>
> > wrote:
> > 
> > Hi Michael
> > 
> > I know/see a lot of activity here, so I assume you are always quite
> > busy and may have missed my last reaction?
> 
> Yes, sorry, I missed this, or didn’t know what to say. Feel free to
> remind me if you didn’t hear from me within a couple of days.
> 
> > Should I proceed with my proposition to add
> > Summary: SUMMARY
> > Services: SERVICES
> > to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro
> 
> I would say yes. Do we have any unanswered questions left?
Not for this patch-set at the moment, I think..

> 
> Is Jonatan happy?
I don't know.. @Jonatan, are you happy? :-)

> 
> > Should I also add 
> > InitScripts: INITSCRIPTS
> > for non-service initscripts.. just for completion of metadata ?
> 
> Hmm, I would say no. If there is no point to it, then leave it.
> 
> We can always add it later if we decide that we want it later.
In the light of possibly adding this later, I opt for the more generic
INSTALL_INITSCRIPTS macro accepting a list of initscripts to install.
Most common way of calling it from the lfs files would then be:

$(call INSTALL_INITSCRIPTS,$(SERVICES))

> 
> > And in that case a more generic INSTALL_INITSCRIPTS macro accepting
> > a
> > list of initscripts to install? (see mail history below for
> > explanation
> > of my intentions)
> > 
> > Or should we find other methods to have this info ready for
> > services.cgi and/or monitoring agents ?
> 
> No, I believe this is a good solution. We can then have a function
> that queries the package database and shows all services correctly.
> 
> That sounds clean to me and is re-usable and extensible.
I think so too.

I started working on changing all pak-lfs files with the additional
fields. When finished and tested, how should I post this change ?
As there are about 200+ lfs files that wil be modified, mostly just
adding a SUMMARY and SERVICES field. Should I post those changes as one
big patch? I assume no-one will be happy if I start posting 200+
patches into the mailing list?

I could try splitting up into patches containing only non-service paks,
and another containing all paks installing initscripts, and then maybe
some separate patches for lfs files that contain some corner cases
should I encounter such paks ?

This of course next to patches to add the INSTALL_INITSCRIPTS macro and
pakfire library change to read/use this additional metadata.

Regards

Robin

> 
> -Michael
> 
> > 
> > Regards
> > Robin
> > 
> > Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]:
> > > Hi Michael
> > > 
> > > Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
> > > > 
> > > > 
> > > > > On 14 May 2021, at 21:07, Robin Roevens <   
> > > > > robin.roevens@disroot.org>
> > > > > wrote:
> > > > > 
> > > > > Hi
> > > > > 
> > > > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
> > > > > > Hello Jonatan,
> > > > > > 
> > > > > > Yes I get your point. Just because a package has more than
> > > > > > one
> > > > > > initscript does not mean they both should be listed in
> > > > > > services.cgi.
> > > > > > 
> > > > > > It is a weird corner case, but how do we want to handle
> > > > > > this?
> > > > > > 
> > > > > > -Michael
> > > > > 
> > > > > Maybe a slight renaming of my proposed new variable
> > > > > INITSCRIPTS to
> > > > > SERVICES in the LFS files would make it a bit more clear that
> > > > > this
> > > > > concerns only service-initscripts?
> > > > > 
> > > > > The INSTALL_INITSCRIPTS macro could then be renamed to
> > > > > INSTALL_SERVICE_INITSCRIPTS.
> > > > > 
> > > > > If we then also keep the original INSTALL_INITSCRIPT macro,
> > > > > that
> > > > > macro
> > > > > can  then be used to install non-service initscripts if
> > > > > required.
> > > > > Or
> > > > > even to install all required initscripts in the occasion that
> > > > > you
> > > > > want
> > > > > to skip some service-initscripts defined in SERVICES because
> > > > > they
> > > > > are
> > > > > included in the source, like the situation Jonatan pointed
> > > > > out (in
> > > > > this
> > > > > case the SERVICES variable is filled with the sole purpose to
> > > > > serve
> > > > > as
> > > > > metadata).
> > > > > 
> > > > > So this would then result in:
> > > > > ---
> > > > > define INSTALL_INITSCRIPT
> > > > >   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > > > /etc/rc.d/init.d/$(1)
> > > > > endef
> > > > > 
> > > > > define INSTALL_SERVICE_INITSCRIPTS
> > > > >   for service in $(SERVICES); do \
> > > > >      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
> > > > >   done
> > > > > endef
> > > > > ---
> > > > > 
> > > > > Optionally we could also add again a variable INITSCRIPTS
> > > > > with the
> > > > > purpose to list non-service initscripts. so
> > > > > SERVICES - for service-initscripts
> > > > > INITSCRIPTS - for non-service-initscripts
> > > > > 
> > > > > and in the meta-* files:
> > > > > ---
> > > > > Name: NAME
> > > > > Summary: SUMMARY
> > > > > ProgVersion: VER
> > > > > Release: RELEASE
> > > > > Size: SIZE
> > > > > Dependencies: DEPS
> > > > > File: NAME-VER-RELEASE.ipfire
> > > > > Services: SERVICES
> > > > > InitScripts: INITSCRIPTS
> > > > > ---
> > > > > 
> > > > > Maybe that info could also come in handy sooner or later ?
> > > > 
> > > > This sounds rather complicated to me.
> > > > 
> > > > What are our objectives here? Does Zabbix want/need to
> > > > start/stop
> > > > services? And if so, why?
> > > 
> > > I was not directly thinking of Zabbix specifically here. The
> > > objective
> > > is to have a central 'database' of metadata for the pak's that
> > > can come
> > > in handy for current and possibly future functionality of IPFire.
> > > And a
> > > single way of retrieving this information (both by using "pakfire
> > > info"
> > > or directly accessing the pakfire library, in my proposal). 
> > > 
> > > Knowing which pak's provide which services can be used in
> > > service.cgi
> > > (which currently does give users the possibility to start/stop
> > > services
> > > indeed) and can be used for monitoring agents like Zabbix, but
> > > also
> > > Nagios or possibly others to know which services to monitor.
> > > 
> > > This instead of my earlier proposal of re-using parts of the
> > > services.cgi code in a separate script for zabbix_agentd, which
> > > you
> > > pointed out that should be done cleaner using some central way of
> > > storing/retrieving that information.
> > > 
> > > Zabbix does have functionality to implement starting/stopping
> > > those
> > > services using a context menu in the Zabbix web GUI; and a user
> > > can
> > > easily implement that on it's own (as this should be done mainly
> > > on
> > > server-side anyway) if he wants to and has the information of
> > > which
> > > services should run. This could be done by zabbix server logging
> > > into
> > > the IPFire machine using ssh and using addonctrl or by the agent,
> > > which
> > > would then need permission to addonctrl start/stop. But I think
> > > that is
> > > something for the user to decide on his own as you pointed out
> > > previously, giving zabbix agent root permission on addonctrl
> > > start/stop
> > > is a possible extra security risk. So I don't think we should
> > > provide
> > > that out of the box. (Maybe it could be a configurable 'setting'
> > > in the
> > > IPFire webgui some time in the future, but not something to focus
> > > on
> > > currently)
> > > 
> > > But with that said, I don't really understand your remark here as
> > > I
> > > only propose a way for the LFS files to populate the "Services"
> > > meta-
> > > data and I only threw in "Initscripts" for non-service
> > > initscripts,
> > > just for completeness of the meta-data.
> > > And to prevent duplication of the list of initscripts, I added a
> > > macro
> > > to install those in one go instead of again 'defining' this list
> > > by
> > > needing to call INSTALL_INITSCRIPT individually for each
> > > initscript
> > > while we now have a list of them in a variable. And due to the
> > > remark
> > > of Jonatan I now also kept the original INSTALL_INITSCRIPT for
> > > the pak
> > > maintainer to use in corner cases.
> > > 
> > > But this has nothing to do with actually stopping/starting
> > > services
> > > other that providing information about which services are
> > > installed
> > > (and maybe could be stopped/started if wanted).
> > > 
> > > If it would make things more clear, I could work my last mail out
> > > in an
> > > actual patch ?
> > > 
> > > Regards
> > > Robin
> > > 
> > > > 
> > > > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be
> > > > > renamed
> > > > > back to INSTALL_INITSCRIPTS, now accepting a parameter which
> > > > > is a
> > > > > list
> > > > > of initscripts to install, so one could call:
> > > > > ---
> > > > > $(call INSTALL_INITSCRIPTS,$(SERVICES))
> > > > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
> > > > > ---
> > > > > to install all initscripts.
> > > > > 
> > > > > Or even provide again an additional macro
> > > > > INSTALL_ALL_INITSCRIPTS
> > > > > which
> > > > > would then install both SERVICES and INITSCRIPTS (or even
> > > > > replace
> > > > > the
> > > > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in
> > > > > a
> > > > > macro to
> > > > > install all initscripts (SERVICES and INITSCRIPTS) and a
> > > > > macro to
> > > > > install a single initscript in case some of the ones listed
> > > > > in
> > > > > SERVICES
> > > > > and/or INITSCRIPTS need to be skipped..
> > > > > 
> > > > > But I think the INSTALL_INITSCRIPTS which takes a list of
> > > > > services
> > > > > to
> > > > > install as parameter is probably the most generic one, giving
> > > > > you
> > > > > the
> > > > > possibility to install all SERVICES in one go, and then
> > > > > individually
> > > > > install only those INITSCRIPTS not included in the source if
> > > > > INITSCRIPTS contains such initscript(s) or vice versa.
> > > > > 
> > > > > Robin
> > > > > 
> > > > > > 
> > > > > > > On 12 May 2021, at 19:49, Jonatan Schlag <
> > > > > > > jonatan.schlag@ipfire.org>
> > > > > > > wrote:
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > > > > > > robin.roevens@disroot.org>:
> > > > > > > > 
> > > > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
> > > > > > > > makefiles.
> > > > > > > > * Add a Summary and InitScripts field to the meta-*
> > > > > > > > addon
> > > > > > > > files
> > > > > > > > containing the value's of those macro's.
> > > > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a
> > > > > > > > new
> > > > > > > > INSTALL_INITSCRIPTS macro/method that will install all
> > > > > > > > initscripts
> > > > > > > > defined in the new INITSCRIPTS macro.
> > > > > > > > * Adapt libvirt and zabbix_agentd pak as examples of
> > > > > > > > how to
> > > > > > > > use
> > > > > > > > this.
> > > > > > > > 
> > > > > > > > Signed-off-by: Robin Roevens
> > > > > > > > <robin.roevens@disroot.org>
> > > > > > > > ---
> > > > > > > > lfs/Config        | 8 ++++++--
> > > > > > > > lfs/libvirt       | 6 ++++--
> > > > > > > > lfs/zabbix_agentd | 5 ++++-
> > > > > > > > src/pakfire/meta  | 2 ++
> > > > > > > > 4 files changed, 16 insertions(+), 5 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/lfs/Config b/lfs/Config
> > > > > > > > index eadbbc408..61d6f0c82 100644
> > > > > > > > --- a/lfs/Config
> > > > > > > > +++ b/lfs/Config
> > > > > > > > @@ -299,15 +299,19 @@ define PAK
> > > > > > > >   # Create meta file
> > > > > > > >   sed \
> > > > > > > >       -e "s/NAME/$(PROG)/g" \
> > > > > > > > +        -e "s/SUMMARY/$(SUMMARY)/g" \
> > > > > > > >       -e "s/VER/$(VER)/g" \
> > > > > > > >       -e "s/RELEASE/$(PAK_VER)/g" \
> > > > > > > >       -e "s/DEPS/$(DEPS)/g" \
> > > > > > > >       -e "s/SIZE/$$(stat --format=%s
> > > > > > > > /install/packages/$(PROG)-
> > > > > > > > $(VER)-$(PAK_VER).ipfire)/g" \
> > > > > > > > +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> > > > > > > >     < /usr/src/src/pakfire/meta >
> > > > > > > > /install/packages/meta-
> > > > > > > > $(PROG)
> > > > > > > > endef
> > > > > > > > 
> > > > > > > > -define INSTALL_INITSCRIPT
> > > > > > > > -    install -m 754 -v
> > > > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > > > > > > /etc/rc.d/init.d/$(1)
> > > > > > > > +define INSTALL_INITSCRIPTS
> > > > > > > > +    for initscript in $(INITSCRIPTS); do \
> > > > > > > > +        install -m 754 -v
> > > > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > > > > > > > /etc/rc.d/init.d/$$initscript; \
> > > > > > > > +    done
> > > > > > > > endef
> > > > > > > > 
> > > > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
> > > > > > > > riscv64)"
> > > > > > > > diff --git a/lfs/libvirt b/lfs/libvirt
> > > > > > > > index 28a95d317..be5d3db3a 100644
> > > > > > > > --- a/lfs/libvirt
> > > > > > > > +++ b/lfs/libvirt
> > > > > > > > @@ -25,6 +25,7 @@
> > > > > > > > include Config
> > > > > > > > 
> > > > > > > > VER        = 6.5.0
> > > > > > > > +SUMMARY       = Server side daemon and supporting
> > > > > > > > files for
> > > > > > > > libvirt
> > > > > > > > 
> > > > > > > > THISAPP    = libvirt-$(VER)
> > > > > > > > DL_FILE    = $(THISAPP).tar.xz
> > > > > > > > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > > > > > > > 
> > > > > > > > DEPS       = ebtables libpciaccess libtirpc libyajl
> > > > > > > > ncat qemu
> > > > > > > > 
> > > > > > > > +INITSCRIPTS= libvirtd virtlogd
> > > > > > > > +
> > > > > > > > #######################################################
> > > > > > > > ######
> > > > > > > > ######
> > > > > > > > ############
> > > > > > > > # Top-level Rules
> > > > > > > > #######################################################
> > > > > > > > ######
> > > > > > > > ######
> > > > > > > > ############
> > > > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > > > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > > > >   cd $(DIR_APP)/build_libvirt && make install
> > > > > > > > 
> > > > > > > >   #install initscripts
> > > > > > > > -    $(call INSTALL_INITSCRIPT,libvirtd)
> > > > > > > > -    $(call INSTALL_INITSCRIPT,virtlogd)
> > > > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > > > >   mv /usr/libexec/libvirt-guests.sh
> > > > > > > > /etc/rc.d/init.d/libvirt-
> > > > > > > > guests
> > > > > > > And here my approach maybe breaks :-). How could we
> > > > > > > handle
> > > > > > > this? It
> > > > > > > is not a daemon, more something like a startup/shutdown
> > > > > > > scripts
> > > > > > > for
> > > > > > > machines... (And should not appear in the service.cgi).
> > > > > > > So
> > > > > > > parsing
> > > > > > > the root file may yield wrong results and your way would
> > > > > > > be the
> > > > > > > better one. 
> > > > > > > 
> > > > > > > Jonatan
> > > > > > > > 
> > > > > > > >   # Backup
> > > > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > > > > > > index c69643a54..a72fe024b 100644
> > > > > > > > --- a/lfs/zabbix_agentd
> > > > > > > > +++ b/lfs/zabbix_agentd
> > > > > > > > @@ -25,6 +25,7 @@
> > > > > > > > include Config
> > > > > > > > 
> > > > > > > > VER        = 4.2.6
> > > > > > > > +SUMMARY    = Zabbix Agent
> > > > > > > > 
> > > > > > > > THISAPP    = zabbix-$(VER)
> > > > > > > > DL_FILE    = $(THISAPP).tar.gz
> > > > > > > > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > > > > > > > PAK_VER    = 4
> > > > > > > > DEPS       =
> > > > > > > > 
> > > > > > > > +INITSCRIPTS= zabbix_agentd
> > > > > > > > +
> > > > > > > > #######################################################
> > > > > > > > ######
> > > > > > > > ######
> > > > > > > > ############
> > > > > > > > # Top-level Rules
> > > > > > > > #######################################################
> > > > > > > > ######
> > > > > > > > ######
> > > > > > > > ############
> > > > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > > > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > > > >   chown zabbix.zabbix /var/run/zabbix
> > > > > > > > 
> > > > > > > >   # Install initscripts
> > > > > > > > -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > > > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > > > > 
> > > > > > > >   # Install sudoers include file
> > > > > > > >   install -v -m 644
> > > > > > > > $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > > > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > > > > > > > index d97b2a0fa..849b9cd6c 100644
> > > > > > > > --- a/src/pakfire/meta
> > > > > > > > +++ b/src/pakfire/meta
> > > > > > > > @@ -1,6 +1,8 @@
> > > > > > > > Name: NAME
> > > > > > > > +Summary: SUMMARY
> > > > > > > > ProgVersion: VER
> > > > > > > > Release: RELEASE
> > > > > > > > Size: SIZE
> > > > > > > > Dependencies: DEPS
> > > > > > > > File: NAME-VER-RELEASE.ipfire
> > > > > > > > +InitScripts: INITSCRIPTS
> > > > > > > > -- 
> > > > > > > > 2.31.1
> > > > > > > > 
> > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Dit bericht is gescanned op virussen en andere
> > > > > > > > gevaarlijke
> > > > > > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > > -- 
> > > > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> > -- 
> > Dit bericht is gescanned op virussen en andere gevaarlijke
> > inhoud door MailScanner en lijkt schoon te zijn.
> > 
> 
>
  
Michael Tremer June 18, 2021, 8:32 a.m. UTC | #13
Hello,

> On 17 Jun 2021, at 23:28, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi
> 
> Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]:
>> Hello,
>> 
>>> On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens@disroot.org>
>>> wrote:
>>> 
>>> Hi Michael
>>> 
>>> I know/see a lot of activity here, so I assume you are always quite
>>> busy and may have missed my last reaction?
>> 
>> Yes, sorry, I missed this, or didn’t know what to say. Feel free to
>> remind me if you didn’t hear from me within a couple of days.
>> 
>>> Should I proceed with my proposition to add
>>> Summary: SUMMARY
>>> Services: SERVICES
>>> to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro
>> 
>> I would say yes. Do we have any unanswered questions left?
> Not for this patch-set at the moment, I think..
> 
>> 
>> Is Jonatan happy?
> I don't know.. @Jonatan, are you happy? :-)

Seems to be on holiday...

> 
>> 
>>> Should I also add 
>>> InitScripts: INITSCRIPTS
>>> for non-service initscripts.. just for completion of metadata ?
>> 
>> Hmm, I would say no. If there is no point to it, then leave it.
>> 
>> We can always add it later if we decide that we want it later.
> In the light of possibly adding this later, I opt for the more generic
> INSTALL_INITSCRIPTS macro accepting a list of initscripts to install.
> Most common way of calling it from the lfs files would then be:
> 
> $(call INSTALL_INITSCRIPTS,$(SERVICES))
> 
>> 
>>> And in that case a more generic INSTALL_INITSCRIPTS macro accepting
>>> a
>>> list of initscripts to install? (see mail history below for
>>> explanation
>>> of my intentions)
>>> 
>>> Or should we find other methods to have this info ready for
>>> services.cgi and/or monitoring agents ?
>> 
>> No, I believe this is a good solution. We can then have a function
>> that queries the package database and shows all services correctly.
>> 
>> That sounds clean to me and is re-usable and extensible.
> I think so too.
> 
> I started working on changing all pak-lfs files with the additional
> fields. When finished and tested, how should I post this change ?
> As there are about 200+ lfs files that wil be modified, mostly just
> adding a SUMMARY and SERVICES field. Should I post those changes as one
> big patch? I assume no-one will be happy if I start posting 200+
> patches into the mailing list?

Would actually that many files be affected?

Changing the macro can be one patch for all lfs files. It does not have to be one patch per lfs file. Grouping logical changes is what we want to do. Nothing more, nothing less.

> I could try splitting up into patches containing only non-service paks,
> and another containing all paks installing initscripts, and then maybe
> some separate patches for lfs files that contain some corner cases
> should I encounter such paks ?
> 
> This of course next to patches to add the INSTALL_INITSCRIPTS macro and
> pakfire library change to read/use this additional metadata.

-Michael

> 
> Regards
> 
> Robin
> 
>> 
>> -Michael
>> 
>>> 
>>> Regards
>>> Robin
>>> 
>>> Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]:
>>>> Hi Michael
>>>> 
>>>> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
>>>>> 
>>>>> 
>>>>>> On 14 May 2021, at 21:07, Robin Roevens <   
>>>>>> robin.roevens@disroot.org>
>>>>>> wrote:
>>>>>> 
>>>>>> Hi
>>>>>> 
>>>>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
>>>>>>> Hello Jonatan,
>>>>>>> 
>>>>>>> Yes I get your point. Just because a package has more than
>>>>>>> one
>>>>>>> initscript does not mean they both should be listed in
>>>>>>> services.cgi.
>>>>>>> 
>>>>>>> It is a weird corner case, but how do we want to handle
>>>>>>> this?
>>>>>>> 
>>>>>>> -Michael
>>>>>> 
>>>>>> Maybe a slight renaming of my proposed new variable
>>>>>> INITSCRIPTS to
>>>>>> SERVICES in the LFS files would make it a bit more clear that
>>>>>> this
>>>>>> concerns only service-initscripts?
>>>>>> 
>>>>>> The INSTALL_INITSCRIPTS macro could then be renamed to
>>>>>> INSTALL_SERVICE_INITSCRIPTS.
>>>>>> 
>>>>>> If we then also keep the original INSTALL_INITSCRIPT macro,
>>>>>> that
>>>>>> macro
>>>>>> can  then be used to install non-service initscripts if
>>>>>> required.
>>>>>> Or
>>>>>> even to install all required initscripts in the occasion that
>>>>>> you
>>>>>> want
>>>>>> to skip some service-initscripts defined in SERVICES because
>>>>>> they
>>>>>> are
>>>>>> included in the source, like the situation Jonatan pointed
>>>>>> out (in
>>>>>> this
>>>>>> case the SERVICES variable is filled with the sole purpose to
>>>>>> serve
>>>>>> as
>>>>>> metadata).
>>>>>> 
>>>>>> So this would then result in:
>>>>>> ---
>>>>>> define INSTALL_INITSCRIPT
>>>>>>   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>>>> /etc/rc.d/init.d/$(1)
>>>>>> endef
>>>>>> 
>>>>>> define INSTALL_SERVICE_INITSCRIPTS
>>>>>>   for service in $(SERVICES); do \
>>>>>>      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
>>>>>>   done
>>>>>> endef
>>>>>> ---
>>>>>> 
>>>>>> Optionally we could also add again a variable INITSCRIPTS
>>>>>> with the
>>>>>> purpose to list non-service initscripts. so
>>>>>> SERVICES - for service-initscripts
>>>>>> INITSCRIPTS - for non-service-initscripts
>>>>>> 
>>>>>> and in the meta-* files:
>>>>>> ---
>>>>>> Name: NAME
>>>>>> Summary: SUMMARY
>>>>>> ProgVersion: VER
>>>>>> Release: RELEASE
>>>>>> Size: SIZE
>>>>>> Dependencies: DEPS
>>>>>> File: NAME-VER-RELEASE.ipfire
>>>>>> Services: SERVICES
>>>>>> InitScripts: INITSCRIPTS
>>>>>> ---
>>>>>> 
>>>>>> Maybe that info could also come in handy sooner or later ?
>>>>> 
>>>>> This sounds rather complicated to me.
>>>>> 
>>>>> What are our objectives here? Does Zabbix want/need to
>>>>> start/stop
>>>>> services? And if so, why?
>>>> 
>>>> I was not directly thinking of Zabbix specifically here. The
>>>> objective
>>>> is to have a central 'database' of metadata for the pak's that
>>>> can come
>>>> in handy for current and possibly future functionality of IPFire.
>>>> And a
>>>> single way of retrieving this information (both by using "pakfire
>>>> info"
>>>> or directly accessing the pakfire library, in my proposal). 
>>>> 
>>>> Knowing which pak's provide which services can be used in
>>>> service.cgi
>>>> (which currently does give users the possibility to start/stop
>>>> services
>>>> indeed) and can be used for monitoring agents like Zabbix, but
>>>> also
>>>> Nagios or possibly others to know which services to monitor.
>>>> 
>>>> This instead of my earlier proposal of re-using parts of the
>>>> services.cgi code in a separate script for zabbix_agentd, which
>>>> you
>>>> pointed out that should be done cleaner using some central way of
>>>> storing/retrieving that information.
>>>> 
>>>> Zabbix does have functionality to implement starting/stopping
>>>> those
>>>> services using a context menu in the Zabbix web GUI; and a user
>>>> can
>>>> easily implement that on it's own (as this should be done mainly
>>>> on
>>>> server-side anyway) if he wants to and has the information of
>>>> which
>>>> services should run. This could be done by zabbix server logging
>>>> into
>>>> the IPFire machine using ssh and using addonctrl or by the agent,
>>>> which
>>>> would then need permission to addonctrl start/stop. But I think
>>>> that is
>>>> something for the user to decide on his own as you pointed out
>>>> previously, giving zabbix agent root permission on addonctrl
>>>> start/stop
>>>> is a possible extra security risk. So I don't think we should
>>>> provide
>>>> that out of the box. (Maybe it could be a configurable 'setting'
>>>> in the
>>>> IPFire webgui some time in the future, but not something to focus
>>>> on
>>>> currently)
>>>> 
>>>> But with that said, I don't really understand your remark here as
>>>> I
>>>> only propose a way for the LFS files to populate the "Services"
>>>> meta-
>>>> data and I only threw in "Initscripts" for non-service
>>>> initscripts,
>>>> just for completeness of the meta-data.
>>>> And to prevent duplication of the list of initscripts, I added a
>>>> macro
>>>> to install those in one go instead of again 'defining' this list
>>>> by
>>>> needing to call INSTALL_INITSCRIPT individually for each
>>>> initscript
>>>> while we now have a list of them in a variable. And due to the
>>>> remark
>>>> of Jonatan I now also kept the original INSTALL_INITSCRIPT for
>>>> the pak
>>>> maintainer to use in corner cases.
>>>> 
>>>> But this has nothing to do with actually stopping/starting
>>>> services
>>>> other that providing information about which services are
>>>> installed
>>>> (and maybe could be stopped/started if wanted).
>>>> 
>>>> If it would make things more clear, I could work my last mail out
>>>> in an
>>>> actual patch ?
>>>> 
>>>> Regards
>>>> Robin
>>>> 
>>>>> 
>>>>>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be
>>>>>> renamed
>>>>>> back to INSTALL_INITSCRIPTS, now accepting a parameter which
>>>>>> is a
>>>>>> list
>>>>>> of initscripts to install, so one could call:
>>>>>> ---
>>>>>> $(call INSTALL_INITSCRIPTS,$(SERVICES))
>>>>>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
>>>>>> ---
>>>>>> to install all initscripts.
>>>>>> 
>>>>>> Or even provide again an additional macro
>>>>>> INSTALL_ALL_INITSCRIPTS
>>>>>> which
>>>>>> would then install both SERVICES and INITSCRIPTS (or even
>>>>>> replace
>>>>>> the
>>>>>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in
>>>>>> a
>>>>>> macro to
>>>>>> install all initscripts (SERVICES and INITSCRIPTS) and a
>>>>>> macro to
>>>>>> install a single initscript in case some of the ones listed
>>>>>> in
>>>>>> SERVICES
>>>>>> and/or INITSCRIPTS need to be skipped..
>>>>>> 
>>>>>> But I think the INSTALL_INITSCRIPTS which takes a list of
>>>>>> services
>>>>>> to
>>>>>> install as parameter is probably the most generic one, giving
>>>>>> you
>>>>>> the
>>>>>> possibility to install all SERVICES in one go, and then
>>>>>> individually
>>>>>> install only those INITSCRIPTS not included in the source if
>>>>>> INITSCRIPTS contains such initscript(s) or vice versa.
>>>>>> 
>>>>>> Robin
>>>>>> 
>>>>>>> 
>>>>>>>> On 12 May 2021, at 19:49, Jonatan Schlag <
>>>>>>>> jonatan.schlag@ipfire.org>
>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens <
>>>>>>>>> robin.roevens@disroot.org>:
>>>>>>>>> 
>>>>>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
>>>>>>>>> makefiles.
>>>>>>>>> * Add a Summary and InitScripts field to the meta-*
>>>>>>>>> addon
>>>>>>>>> files
>>>>>>>>> containing the value's of those macro's.
>>>>>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a
>>>>>>>>> new
>>>>>>>>> INSTALL_INITSCRIPTS macro/method that will install all
>>>>>>>>> initscripts
>>>>>>>>> defined in the new INITSCRIPTS macro.
>>>>>>>>> * Adapt libvirt and zabbix_agentd pak as examples of
>>>>>>>>> how to
>>>>>>>>> use
>>>>>>>>> this.
>>>>>>>>> 
>>>>>>>>> Signed-off-by: Robin Roevens
>>>>>>>>> <robin.roevens@disroot.org>
>>>>>>>>> ---
>>>>>>>>> lfs/Config        | 8 ++++++--
>>>>>>>>> lfs/libvirt       | 6 ++++--
>>>>>>>>> lfs/zabbix_agentd | 5 ++++-
>>>>>>>>> src/pakfire/meta  | 2 ++
>>>>>>>>> 4 files changed, 16 insertions(+), 5 deletions(-)
>>>>>>>>> 
>>>>>>>>> diff --git a/lfs/Config b/lfs/Config
>>>>>>>>> index eadbbc408..61d6f0c82 100644
>>>>>>>>> --- a/lfs/Config
>>>>>>>>> +++ b/lfs/Config
>>>>>>>>> @@ -299,15 +299,19 @@ define PAK
>>>>>>>>>   # Create meta file
>>>>>>>>>   sed \
>>>>>>>>>       -e "s/NAME/$(PROG)/g" \
>>>>>>>>> +        -e "s/SUMMARY/$(SUMMARY)/g" \
>>>>>>>>>       -e "s/VER/$(VER)/g" \
>>>>>>>>>       -e "s/RELEASE/$(PAK_VER)/g" \
>>>>>>>>>       -e "s/DEPS/$(DEPS)/g" \
>>>>>>>>>       -e "s/SIZE/$$(stat --format=%s
>>>>>>>>> /install/packages/$(PROG)-
>>>>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \
>>>>>>>>> +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
>>>>>>>>>     < /usr/src/src/pakfire/meta >
>>>>>>>>> /install/packages/meta-
>>>>>>>>> $(PROG)
>>>>>>>>> endef
>>>>>>>>> 
>>>>>>>>> -define INSTALL_INITSCRIPT
>>>>>>>>> -    install -m 754 -v
>>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) 
>>>>>>>>> /etc/rc.d/init.d/$(1)
>>>>>>>>> +define INSTALL_INITSCRIPTS
>>>>>>>>> +    for initscript in $(INITSCRIPTS); do \
>>>>>>>>> +        install -m 754 -v
>>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript 
>>>>>>>>> /etc/rc.d/init.d/$$initscript; \
>>>>>>>>> +    done
>>>>>>>>> endef
>>>>>>>>> 
>>>>>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
>>>>>>>>> riscv64)"
>>>>>>>>> diff --git a/lfs/libvirt b/lfs/libvirt
>>>>>>>>> index 28a95d317..be5d3db3a 100644
>>>>>>>>> --- a/lfs/libvirt
>>>>>>>>> +++ b/lfs/libvirt
>>>>>>>>> @@ -25,6 +25,7 @@
>>>>>>>>> include Config
>>>>>>>>> 
>>>>>>>>> VER        = 6.5.0
>>>>>>>>> +SUMMARY       = Server side daemon and supporting
>>>>>>>>> files for
>>>>>>>>> libvirt
>>>>>>>>> 
>>>>>>>>> THISAPP    = libvirt-$(VER)
>>>>>>>>> DL_FILE    = $(THISAPP).tar.xz
>>>>>>>>> @@ -37,6 +38,8 @@ PAK_VER    = 25
>>>>>>>>> 
>>>>>>>>> DEPS       = ebtables libpciaccess libtirpc libyajl
>>>>>>>>> ncat qemu
>>>>>>>>> 
>>>>>>>>> +INITSCRIPTS= libvirtd virtlogd
>>>>>>>>> +
>>>>>>>>> #######################################################
>>>>>>>>> ######
>>>>>>>>> ######
>>>>>>>>> ############
>>>>>>>>> # Top-level Rules
>>>>>>>>> #######################################################
>>>>>>>>> ######
>>>>>>>>> ######
>>>>>>>>> ############
>>>>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
>>>>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>>>>   cd $(DIR_APP)/build_libvirt && make install
>>>>>>>>> 
>>>>>>>>>   #install initscripts
>>>>>>>>> -    $(call INSTALL_INITSCRIPT,libvirtd)
>>>>>>>>> -    $(call INSTALL_INITSCRIPT,virtlogd)
>>>>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>>>>>   mv /usr/libexec/libvirt-guests.sh
>>>>>>>>> /etc/rc.d/init.d/libvirt-
>>>>>>>>> guests
>>>>>>>> And here my approach maybe breaks :-). How could we
>>>>>>>> handle
>>>>>>>> this? It
>>>>>>>> is not a daemon, more something like a startup/shutdown
>>>>>>>> scripts
>>>>>>>> for
>>>>>>>> machines... (And should not appear in the service.cgi).
>>>>>>>> So
>>>>>>>> parsing
>>>>>>>> the root file may yield wrong results and your way would
>>>>>>>> be the
>>>>>>>> better one. 
>>>>>>>> 
>>>>>>>> Jonatan
>>>>>>>>> 
>>>>>>>>>   # Backup
>>>>>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
>>>>>>>>> index c69643a54..a72fe024b 100644
>>>>>>>>> --- a/lfs/zabbix_agentd
>>>>>>>>> +++ b/lfs/zabbix_agentd
>>>>>>>>> @@ -25,6 +25,7 @@
>>>>>>>>> include Config
>>>>>>>>> 
>>>>>>>>> VER        = 4.2.6
>>>>>>>>> +SUMMARY    = Zabbix Agent
>>>>>>>>> 
>>>>>>>>> THISAPP    = zabbix-$(VER)
>>>>>>>>> DL_FILE    = $(THISAPP).tar.gz
>>>>>>>>> @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
>>>>>>>>> PAK_VER    = 4
>>>>>>>>> DEPS       =
>>>>>>>>> 
>>>>>>>>> +INITSCRIPTS= zabbix_agentd
>>>>>>>>> +
>>>>>>>>> #######################################################
>>>>>>>>> ######
>>>>>>>>> ######
>>>>>>>>> ############
>>>>>>>>> # Top-level Rules
>>>>>>>>> #######################################################
>>>>>>>>> ######
>>>>>>>>> ######
>>>>>>>>> ############
>>>>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
>>>>>>>>> %,$(DIR_DL)/%,$(objects))
>>>>>>>>>   chown zabbix.zabbix /var/run/zabbix
>>>>>>>>> 
>>>>>>>>>   # Install initscripts
>>>>>>>>> -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
>>>>>>>>> +    @$(INSTALL_INITSCRIPTS)
>>>>>>>>> 
>>>>>>>>>   # Install sudoers include file
>>>>>>>>>   install -v -m 644
>>>>>>>>> $(DIR_SRC)/config/zabbix_agentd/sudoers \
>>>>>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta
>>>>>>>>> index d97b2a0fa..849b9cd6c 100644
>>>>>>>>> --- a/src/pakfire/meta
>>>>>>>>> +++ b/src/pakfire/meta
>>>>>>>>> @@ -1,6 +1,8 @@
>>>>>>>>> Name: NAME
>>>>>>>>> +Summary: SUMMARY
>>>>>>>>> ProgVersion: VER
>>>>>>>>> Release: RELEASE
>>>>>>>>> Size: SIZE
>>>>>>>>> Dependencies: DEPS
>>>>>>>>> File: NAME-VER-RELEASE.ipfire
>>>>>>>>> +InitScripts: INITSCRIPTS
>>>>>>>>> -- 
>>>>>>>>> 2.31.1
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> -- 
>>>>>>>>> Dit bericht is gescanned op virussen en andere
>>>>>>>>> gevaarlijke
>>>>>>>>> inhoud door MailScanner en lijkt schoon te zijn.
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> -- 
>>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>>>>> inhoud door MailScanner en lijkt schoon te zijn.
>>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> -- 
>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>> inhoud door MailScanner en lijkt schoon te zijn.
>>> 
>> 
>> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
  

Patch

diff --git a/lfs/Config b/lfs/Config
index eadbbc408..61d6f0c82 100644
--- a/lfs/Config
+++ b/lfs/Config
@@ -299,15 +299,19 @@  define PAK
 	# Create meta file
 	sed \
 		-e "s/NAME/$(PROG)/g" \
+		-e "s/SUMMARY/$(SUMMARY)/g" \
 		-e "s/VER/$(VER)/g" \
 		-e "s/RELEASE/$(PAK_VER)/g" \
 		-e "s/DEPS/$(DEPS)/g" \
 		-e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \
+		-e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
 	  < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG)
 endef
 
-define INSTALL_INITSCRIPT
-	install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)  /etc/rc.d/init.d/$(1)
+define INSTALL_INITSCRIPTS
+	for initscript in $(INITSCRIPTS); do \
+		install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript  /etc/rc.d/init.d/$$initscript; \
+	done
 endef
 
 ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)"
diff --git a/lfs/libvirt b/lfs/libvirt
index 28a95d317..be5d3db3a 100644
--- a/lfs/libvirt
+++ b/lfs/libvirt
@@ -25,6 +25,7 @@ 
 include Config
 
 VER        = 6.5.0
+SUMMARY	   = Server side daemon and supporting files for libvirt
 
 THISAPP    = libvirt-$(VER)
 DL_FILE    = $(THISAPP).tar.xz
@@ -37,6 +38,8 @@  PAK_VER    = 25
 
 DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
 
+INITSCRIPTS= libvirtd virtlogd
+
 ###############################################################################
 # Top-level Rules
 ###############################################################################
@@ -121,8 +124,7 @@  $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
 	cd $(DIR_APP)/build_libvirt && make install
 
 	#install initscripts
-	$(call INSTALL_INITSCRIPT,libvirtd)
-	$(call INSTALL_INITSCRIPT,virtlogd)
+	@$(INSTALL_INITSCRIPTS)
 	mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests
 
 	# Backup
diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
index c69643a54..a72fe024b 100644
--- a/lfs/zabbix_agentd
+++ b/lfs/zabbix_agentd
@@ -25,6 +25,7 @@ 
 include Config
 
 VER        = 4.2.6
+SUMMARY    = Zabbix Agent
 
 THISAPP    = zabbix-$(VER)
 DL_FILE    = $(THISAPP).tar.gz
@@ -35,6 +36,8 @@  PROG       = zabbix_agentd
 PAK_VER    = 4
 DEPS       =
 
+INITSCRIPTS= zabbix_agentd
+
 ###############################################################################
 # Top-level Rules
 ###############################################################################
@@ -106,7 +109,7 @@  $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
 	chown zabbix.zabbix /var/run/zabbix
 
 	# Install initscripts
-	$(call INSTALL_INITSCRIPT,zabbix_agentd)
+	@$(INSTALL_INITSCRIPTS)
 
 	# Install sudoers include file
 	install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
diff --git a/src/pakfire/meta b/src/pakfire/meta
index d97b2a0fa..849b9cd6c 100644
--- a/src/pakfire/meta
+++ b/src/pakfire/meta
@@ -1,6 +1,8 @@ 
 Name: NAME
+Summary: SUMMARY
 ProgVersion: VER
 Release: RELEASE
 Size: SIZE
 Dependencies: DEPS
 File: NAME-VER-RELEASE.ipfire
+InitScripts: INITSCRIPTS