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

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

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

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