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

[Robin Roevens <robin.roevens@disroot.org>]

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

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(a)ipfire.org>
> > wrote:
> > 
> > Hi,
> > 
> > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > robin.roevens(a)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(a)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.